Re: [RFC] umount/__detach_mounts() race

[Al Viro <viro@zeniv.linux.org.uk>]

	BTW, while looking through the vicinity - I think this 
        if (!check_mnt(old) && old_path->dentry->d_op != &ns_dentry_operations)
			return mnt;
in __do_loopback() is too permissive.  I'd prefer to replace it with
        if (!check_mnt(old)) {
		// allow binding objects from internal instance of nsfs
		if (old->mnt_ns != MNT_NS_INTERAL ||
		    old_path->dentry->d_op != &ns_dentry_operations)
			return mnt;
	}

Suppose we'd bound an nsfs object e.g. on /tmp/foo.  Consider a race
between mount --bind /tmp/foo /tmp/bar and umount -l /tmp/foo.

do_loopback() resolves /tmp/foo.  We have dentry from nsfs and mount
that sits on /tmp/foo.  We'd resolved /tmp/bar.

In the meanwhile, umount has resolved /tmp/foo and unmounted it.
struct mount is still alive due to the reference held by do_loopback().

do_loopback() finally grabs namespace_sem.  It verifies that mountpoint
to be (/tmp/bar) is still OK (it is) and calls __do_loopback().  The
check in __do_loopback() passes - old is unmounted, but dentry is
nsfs one, so we proceed to clone old.

And that's where the things go wrong - we copy the flags, including
MNT_UMOUNT, to the new instance.  And proceed to attach it on /tmp/bar.

It's really not a good thing.  E.g. __detach_mnt() will assume that
reference to the parent mount of /tmp/bar is *not* held.  As the
matter of fact, it is, so we'll get a leak if the mountpoint (/tmp/bar,
that is) gets unlinked in another namespace.  That's not the only way
to get trouble - we are really not supposed to have MNT_UMOUNT mounts
attached to !MNT_UMOUNT ones.

Eric, do you see any problems with the change above?