Re: MNT_DETACH and mount namespace issue

[Ram Pai]

On Fri, Aug 01, 2014 at 12:17:13AM +0200, Richard Weinberger wrote:
> Am 30.07.2014 22:46, schrieb Richard Weinberger:
> > Am 30.07.2014 15:59, schrieb Richard Weinberger:
> >> If we use the plain list_empty() we might not see the
> >> hlist_del_init_rcu() and therefore miss one member of the
> >> list.
> >>
> >> It fixes the following issue:
> >> $ unshare -m /usr/bin/sleep 10000 &
> >> $ mkdir -p foo/proc
> >> $ mount -t proc none foo/proc
> >> $ mount -t binfmt_misc none foo/proc/sys/fs/binfmt_misc
> >> $ umount -l foo/proc
> >> $ rmdir foo/proc
> >> rmdir: failed to remove âfoo/procâ: Device or resource busy
> > 
> > Although my fix was wrong, the issue is real, it seems to exist for a very long
> > time. Just was able to reproduce it on 2.6.32.
> > Please note that you need a shared root subtree to trigger the issue.
> > i.e. mount --shared /
> > Maybe this is why nobody noticed it so far as only systemd distros
> > have the root subtree shared by default.
> > 
> > I hit the issue on openSUSE 13.1 where an application creates a chroot environment
> > and then lazy umounts /proc.
> > It happened on very few machines. An analysis showed that only boxes with an OpenVPN tunnel
> > were affected. This did not make any sense until I discovered that the OpenVPN systemd
> > service file has set "PrivateTmp=true". This setting creates
> > a mount namespace for the said service...
> > 
> > In __propagate_umount() the following piece of code is interesting:
> > 
> >  /*
> >  * umount the child only if the child has no
> >  * other children
> >  */
> > if (child && list_empty(&child->mnt_mounts)) {
> >         hlist_del_init_rcu(&child->mnt_hash);
> >         hlist_add_before_rcu(&child->mnt_hash, &mnt->mnt_hash);
> > }
> > 
> > child->mnt_mounts is non-empty for the "proc" although the "binfmt_misc"
> > subtree was removed.
> > I'm not sure whether this is only one more symptom or the main culprit.
> 
> CC'ing Ram Pai.
> 
> Ram, you are the author of the said code. Can you please explain why we need that
> list_empty() check?
> To my (limited) understanding of VFS, the following change should be fine to fix the issue:

We had made a rule then, that busy vfsmounts cannot be lazily unmounted
**implicitly**. Propagated unmounts are implicit unmounts, and if such
implicit vfsmounts have child-mounts than obviously they are busy, and
hence they cannot be lazy-unmounted implicitly.

the list_empty() is checking for no child-mounts on the vfsmount before
letting it unmount.

We did not want a bunch of mounts disappear without the users knowledge.
Hence we made the above rule.

Al Viro, will have more insights into this.

RP

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/