linux-kernel - Re: [PATCH 01/33] vfs: syscall: Add open_tree(2) to reference or clone a mount [ver #11]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180802212923.GA30522@ZenIV.linux.org.uk>
Date:   Thu, 2 Aug 2018 22:29:23 +0100
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Alan Jenkins <alan.christopher.jenkins@...il.com>
Cc:     David Howells <dhowells@...hat.com>, linux-api@...r.kernel.org,
        torvalds@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/33] vfs: syscall: Add open_tree(2) to reference or
 clone a mount [ver #11]

On Thu, Aug 02, 2018 at 06:31:06PM +0100, Alan Jenkins wrote:
> Hi
> 
> I found this interesting, though I don't entirely follow the kernel
> mount/unmount code.  I had one puzzle about the code, and two questions
> which I was largely able to answer.
> 
> On 01/08/18 16:24, David Howells wrote:
> > +void dissolve_on_fput(struct vfsmount *mnt)
> > +{
> > +	namespace_lock();
> > +	lock_mount_hash();
> > +	mntget(mnt);
> > +	umount_tree(real_mount(mnt), UMOUNT_SYNC);
> > +	unlock_mount_hash();
> > +	namespace_unlock();
> > +}
> 
> Can I ask why  UMOUNT_SYNC is used here?  I feel like I must have missed
> something, but doesn't it skip the IS_MNT_LOCKED() check in
> disconnect_mount() ?
> 
> UMOUNT_SYNC seems used for non-lazy unmounts, and in internal cleanups where
> userspace wouldn't be able to see.  But I think userspace can keep watching
> in this case, e.g. by `fd2 = openat(fd, ".", O_PATH)` (or `fd2 =
> open_tree(fd, ".", 0)` ?).  I would think this function should avoid using
> UMOUNT_SYNC, like lazy unmount avoids it.

FWIW, I suspect that UMOUNT_CONNECTED might be the right thing here...