linux-kernel - Re: [PATCH 2/3] vfs: add propagate_mount_tree

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: <YtdqyjqrUaIivGUT@ZenIV>
Date:   Wed, 20 Jul 2022 03:39:06 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Ian Kent <raven@...maw.net>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        David Howells <dhowells@...hat.com>,
        Miklos Szeredi <miklos@...redi.hu>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] vfs: add propagate_mount_tree_busy() helper

On Wed, Jul 20, 2022 at 10:31:26AM +0800, Ian Kent wrote:
> 
> On 20/7/22 09:54, Al Viro wrote:
> > On Mon, Jul 11, 2022 at 11:37:46AM +0800, Ian Kent wrote:
> > 
> > > +static int do_mount_in_use_check(struct mount *mnt, int cnt)
> > > +{
> > > +	struct mount *topper;
> > > +
> > > +	/* Is there exactly one mount on the child that covers
> > > +	 * it completely?
> > > +	 */
> > > +	topper = find_topper(mnt);
> > > +	if (topper) {
> > > +		int topper_cnt = topper->mnt_mounts_cnt + 1;
> > > +
> > > +		/* Open file or pwd within singular mount? */
> > > +		if (do_refcount_check(topper, topper_cnt))
> > > +			return 1;
> > Whatever the hell for?  umount(2) will be able to slide the
> > underlying mount from under the topper, whatever the
> > refcount of topper might have been.
> 
> My thinking was that a process could have set a working
> 
> directory (or opened a descriptor) and some later change
> 
> to an autofs map resulted in it being mounted on. It's
> 
> irrelevant now with your suggested simpler approach, ;)

No, I mean why bother checking refcount of overmount in the first
place?  umount(2) will *not* consider it as -EBUSY.  On propagation
under the full overmount it will quietly remove the thing it's
overmounting.

If you have

overmount
victim
mountpoint

stacked like that, with overmount sitting directly on the root
subtree covered by the victim, the only things checked will be
	* victim itself is not busy
	* victim has nothing mounted deeper in it.
In that case it'll collapse to

overmount
mountpoint

and proceed to take the (now detached) victim out.