[CRIU] [PATCH 1/2] [v3] mount: don't overmount a mount if it should be bind-mounted somewhere

[Pavel Emelyanov]

On 05/12/2016 02:18 AM, Andrew Vagin wrote:
> On Wed, May 11, 2016 at 03:57:02PM +0300, Pavel Emelyanov wrote:
>> On 05/11/2016 03:38 AM, Andrey Vagin wrote:
>>> From: Andrew Vagin <avagin at virtuozzo.com>
>>>
>>> It's impossiable to make a bind-mount if a source is overmounted.
>>>
>>> v2: make a bind-mount from an underlying mount via a file descriptor
>>> v3: add a separate buffer to generate path to a file descriptor
>>>
>>> Reported-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>> Signed-off-by: Andrew Vagin <avagin at virtuozzo.com>
>>> ---
>>>  criu/include/mount.h |  1 +
>>>  criu/mount.c         | 30 ++++++++++++++++++++++++++----
>>>  2 files changed, 27 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/criu/include/mount.h b/criu/include/mount.h
>>> index 59e7f9a..c7992ac 100644
>>> --- a/criu/include/mount.h
>>> +++ b/criu/include/mount.h
>>> @@ -49,6 +49,7 @@ struct mount_info {
>>>  	 */
>>>  	char			*mountpoint;
>>>  	char			*ns_mountpoint;
>>> +	int			fd;
>>>  	unsigned		flags;
>>>  	unsigned		sb_flags;
>>>  	int			master_id;
>>> diff --git a/criu/mount.c b/criu/mount.c
>>> index 71c417a..6896aa4 100644
>>> --- a/criu/mount.c
>>> +++ b/criu/mount.c
>>> @@ -2428,6 +2428,7 @@ static int umount_clean_path()
>>>  
>>>  static int do_bind_mount(struct mount_info *mi)
>>>  {
>>> +	char mnt_fd_path[] = "/proc/self/fd/1234567890";
>>
>> We have a magic constant PSFDS for such cases :)
>>
>>>  	char *root, *cut_root, rpath[PATH_MAX];
>>>  	unsigned long mflags;
>>>  	int exit_code = -1;
>>> @@ -2468,11 +2469,16 @@ static int do_bind_mount(struct mount_info *mi)
>>>  	 */
>>>  	mi->private = mi->bind->private;
>>>  
>>> -	if (list_empty(&mi->bind->children))
>>> -		mnt_path = mi->bind->mountpoint;
>>> -	else {
>>> +	mnt_path = mi->bind->mountpoint;
>>> +	if (mi->bind->fd >= 0) {
>>> +		snprintf(mnt_fd_path, sizeof(mnt_fd_path),
>>> +					"/proc/self/fd/%d", mi->bind->fd);
>>> +		mnt_path = mnt_fd_path;
>>
>> This snprintf can be done under the check for children below.
> 
> fd is create only if a mount is overmounted (one of children has the
> same mount point). The next patch will add one more condition to the
> check for children below.

This one:

if (!list_empty(&mi->bind->children) && cut_root[0] != 0) {

?

Wait a second, cur_root will be then attached to either /proc/pid/fd/abc
or to what this /proc/pid/fd/abc got bind-mounted to. So what's the point
in additional bind-mount of /proc/pid/fd? Why not go and do final bind
mount of /proc/pid/fd/abc/cut/root/path ?

I guess I'm totally confused by do_bind_mount(). Would you write in some
pseudo-code what's going on there and why? Thanks :)

-- Pavel