[CRIU] [PATCH v2] mount: don't overmount a mount if it should be bind-mounted somewhere

[Pavel Emelyanov]

On 05/03/2016 08:55 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

How about a test for this?

> 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         | 27 +++++++++++++++++++++++----
>  2 files changed, 24 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 0b1c2ba..6133a72 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -2447,11 +2447,15 @@ 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(rpath, sizeof(rpath), "/proc/self/fd/%d", mi->bind->fd);
> +		mnt_path = rpath;
> +	}
> +
> +	if (!list_empty(&mi->bind->children)) {

What if this if () is false, but mi->bind->fd >= 0 is true. We go ahead and corrupt
the rpath variable below.

>  		/* 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);
>  		}
> @@ -2606,6 +2610,11 @@ 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) {

Brace.

> +	close_safe(&mi->fd);
> +	return 0;
> +}
> +
>  static int do_mount_one(struct mount_info *mi)
>  {
>  	int ret;
> @@ -2618,6 +2627,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) {
> @@ -2740,6 +2757,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);
> @@ -3211,6 +3229,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;
>