Commit 570487d3 authored May 15, 2017 by Eric W. Biederman

mnt: In umount propagation reparent in a separate pass



It was observed that in some pathlogical cases that the current code
does not unmount everything it should.  After investigation it
was determined that the issue is that mnt_change_mntpoint can
can change which mounts are available to be unmounted during mount
propagation which is wrong.

The trivial reproducer is:
$ cat ./pathological.sh

mount -t tmpfs test-base /mnt
cd /mnt
mkdir 1 2 1/1
mount --bind 1 1
mount --make-shared 1
mount --bind 1 2
mount --bind 1/1 1/1
mount --bind 1/1 1/1
echo
grep test-base /proc/self/mountinfo
umount 1/1
echo
grep test-base /proc/self/mountinfo

$ unshare -Urm ./pathological.sh

The expected output looks like:
46 31 0:25 / /mnt rw,relatime - tmpfs test-base rw,uid=1000,gid=1000
47 46 0:25 /1 /mnt/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
48 46 0:25 /1 /mnt/2 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
49 54 0:25 /1/1 /mnt/1/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
50 53 0:25 /1/1 /mnt/2/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
51 49 0:25 /1/1 /mnt/1/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
54 47 0:25 /1/1 /mnt/1/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
53 48 0:25 /1/1 /mnt/2/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
52 50 0:25 /1/1 /mnt/2/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000

46 31 0:25 / /mnt rw,relatime - tmpfs test-base rw,uid=1000,gid=1000
47 46 0:25 /1 /mnt/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
48 46 0:25 /1 /mnt/2 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000

The output without the fix looks like:
46 31 0:25 / /mnt rw,relatime - tmpfs test-base rw,uid=1000,gid=1000
47 46 0:25 /1 /mnt/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
48 46 0:25 /1 /mnt/2 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
49 54 0:25 /1/1 /mnt/1/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
50 53 0:25 /1/1 /mnt/2/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
51 49 0:25 /1/1 /mnt/1/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
54 47 0:25 /1/1 /mnt/1/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
53 48 0:25 /1/1 /mnt/2/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
52 50 0:25 /1/1 /mnt/2/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000

46 31 0:25 / /mnt rw,relatime - tmpfs test-base rw,uid=1000,gid=1000
47 46 0:25 /1 /mnt/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
48 46 0:25 /1 /mnt/2 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000
52 48 0:25 /1/1 /mnt/2/1 rw,relatime shared:1 - tmpfs test-base rw,uid=1000,gid=1000

That last mount in the output was in the propgation tree to be unmounted but
was missed because the mnt_change_mountpoint changed it's parent before the walk
through the mount propagation tree observed it.

Cc: stable@vger.kernel.org
Fixes: 1064f874 ("mnt: Tuck mounts under others instead of creating shadow/side mounts.")
Acked-by: Andrei Vagin <avagin@virtuozzo.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

parent c70d9d80

fs/mount.h

+1 −0

Original line number	Diff line number	Diff line
		@@ -58,6 +58,7 @@ struct mount {
		struct mnt_namespace mnt_ns; / containing namespace */
		struct mountpoint mnt_mp; / where is it mounted */
		struct hlist_node mnt_mp_list; /* list mounts with the same mountpoint */
		struct list_head mnt_reparent; /* reparent list entry */
		#ifdef CONFIG_FSNOTIFY
		struct fsnotify_mark_connector __rcu *mnt_fsnotify_marks;
		__u32 mnt_fsnotify_mask;

fs/namespace.c

+1 −0

Original line number	Diff line number	Diff line
		@@ -236,6 +236,7 @@ static struct mount alloc_vfsmnt(const char name)
		INIT_LIST_HEAD(&mnt->mnt_slave_list);
		INIT_LIST_HEAD(&mnt->mnt_slave);
		INIT_HLIST_NODE(&mnt->mnt_mp_list);
		INIT_LIST_HEAD(&mnt->mnt_reparent);
		init_fs_pin(&mnt->mnt_umount, drop_mountpoint);
		}
		return mnt;

fs/pnode.c

+30 −5

Original line number	Diff line number	Diff line
		@@ -439,7 +439,7 @@ static void mark_umount_candidates(struct mount *mnt)
		* NOTE: unmounting 'mnt' naturally propagates to all other mounts its
		* parent propagates to.
		*/
		static void __propagate_umount(struct mount *mnt)
		static void __propagate_umount(struct mount mnt, struct list_head to_reparent)
		{
		struct mount *parent = mnt->mnt_parent;
		struct mount *m;
		@@ -464,17 +464,38 @@ static void __propagate_umount(struct mount *mnt)
		*/
		topper = find_topper(child);
		if (topper)
		mnt_change_mountpoint(child->mnt_parent, child->mnt_mp,
		topper);
		list_add_tail(&topper->mnt_reparent, to_reparent);

		if (list_empty(&child->mnt_mounts)) {
		if (topper \|\| list_empty(&child->mnt_mounts)) {
		list_del_init(&child->mnt_child);
		list_del_init(&child->mnt_reparent);
		child->mnt.mnt_flags \|= MNT_UMOUNT;
		list_move_tail(&child->mnt_list, &mnt->mnt_list);
		}
		}
		}

		static void reparent_mounts(struct list_head *to_reparent)
		{
		while (!list_empty(to_reparent)) {
		struct mount mnt, parent;
		struct mountpoint *mp;

		mnt = list_first_entry(to_reparent, struct mount, mnt_reparent);
		list_del_init(&mnt->mnt_reparent);

		/* Where should this mount be reparented to? */
		mp = mnt->mnt_mp;
		parent = mnt->mnt_parent;
		while (parent->mnt.mnt_flags & MNT_UMOUNT) {
		mp = parent->mnt_mp;
		parent = parent->mnt_parent;
		}

		mnt_change_mountpoint(parent, mp, mnt);
		}
		}

		/*
		* collect all mounts that receive propagation from the mount in @list,
		* and return these additional mounts in the same list.
		@@ -485,11 +506,15 @@ static void __propagate_umount(struct mount *mnt)
		int propagate_umount(struct list_head *list)
		{
		struct mount *mnt;
		LIST_HEAD(to_reparent);

		list_for_each_entry_reverse(mnt, list, mnt_list)
		mark_umount_candidates(mnt);

		list_for_each_entry(mnt, list, mnt_list)
		__propagate_umount(mnt);
		__propagate_umount(mnt, &to_reparent);

		reparent_mounts(&to_reparent);

		return 0;
		}

Admin message