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

[Andrew Vagin]

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.

> 
> > +	}
> > +
> > +	if (!list_empty(&mi->bind->children)) {
> >  		/* mi->bind->mountpoint may be overmounted */
> > -		if (mount(mi->bind->mountpoint, mnt_clean_path, NULL, MS_BIND, NULL)) {
> > +		if (mount(mnt_path, mnt_clean_path, NULL, MS_BIND, NULL)) {
> >  			pr_perror("Unable to bind-mount %s to %s",
> >  					mi->bind->mountpoint, mnt_clean_path);
> >  		}
> 
> The else branch (no children at mi->bind) must have BUG_ON(mi->bind->fd >= 0).
> 
> > @@ -2627,6 +2633,12 @@ static int do_mount_root(struct mount_info *mi)
> >  	return fetch_rt_stat(mi, mi->mountpoint);
> >  }
> >  
> > +static int do_close_one(struct mount_info *mi)
> > +{
> > +	close_safe(&mi->fd);
> > +	return 0;
> > +}
> > +
> >  static int do_mount_one(struct mount_info *mi)
> >  {
> >  	int ret;
> > @@ -2639,6 +2651,14 @@ static int do_mount_one(struct mount_info *mi)
> >  		return 1;
> >  	}
> >  
> > +	if (mi->parent && !strcmp(mi->parent->mountpoint, mi->mountpoint)) {
> > +		mi->parent->fd = open(mi->parent->mountpoint, O_PATH);
> > +		if (mi->parent->fd < 0) {
> > +			pr_perror("Unable to open %s", mi->mountpoint);
> > +			return -1;
> > +		}
> > +	}
> > +
> >  	pr_debug("\tMounting %s @%s (%d)\n", mi->fstype->name, mi->mountpoint, mi->need_plugin);
> >  
> >  	if (!mi->parent) {
> > @@ -2761,6 +2781,7 @@ struct mount_info *mnt_entry_alloc()
> >  
> >  	new = xzalloc(sizeof(struct mount_info));
> >  	if (new) {
> > +		new->fd = -1;
> >  		INIT_LIST_HEAD(&new->children);
> >  		INIT_LIST_HEAD(&new->siblings);
> >  		INIT_LIST_HEAD(&new->mnt_slave_list);
> > @@ -3238,6 +3259,7 @@ static int populate_mnt_ns(void)
> >  		return -1;
> >  
> >  	ret = mnt_tree_for_each(pms, do_mount_one);
> > +	mnt_tree_for_each(pms, do_close_one);
> >  
> >  	if (umount_clean_path())
> >  		return -1;
> > 
>