linux-kernel - Re: [PATCH] mount: fix duplicate mounts using the new mount API

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: <20251031-gerufen-rotkohl-7d86b0c3dfe2@brauner>
Date: Fri, 31 Oct 2025 13:54:27 +0100
From: Christian Brauner <brauner@...nel.org>
To: GuangFei Luo <luogf2025@....com>
Cc: Al Viro <viro@...iv.linux.org.uk>, jack@...e.cz, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] mount: fix duplicate mounts using the new mount API

On Sat, Oct 25, 2025 at 02:02:51PM +0800, GuangFei Luo wrote:
> 
> 
> On 10/25/2025 11:36 AM, Al Viro wrote:
> > On Sat, Oct 25, 2025 at 10:49:34AM +0800, GuangFei Luo wrote:
> > 
> > > @@ -4427,6 +4427,7 @@ SYSCALL_DEFINE5(move_mount,
> > >   {
> > >   	struct path to_path __free(path_put) = {};
> > >   	struct path from_path __free(path_put) = {};
> > > +	struct path path __free(path_put) = {};
> > >   	struct filename *to_name __free(putname) = NULL;
> > >   	struct filename *from_name __free(putname) = NULL;
> > >   	unsigned int lflags, uflags;
> > > @@ -4472,6 +4473,14 @@ SYSCALL_DEFINE5(move_mount,
> > >   			return ret;
> > >   	}
> > > +	ret = user_path_at(AT_FDCWD, to_pathname, LOOKUP_FOLLOW, &path);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Refuse the same filesystem on the same mount point */
> > > +	if (path.mnt->mnt_sb == to_path.mnt->mnt_sb && path_mounted(&path))
> > > +		return -EBUSY;
> > Races galore:
> > 	* who said that string pointed to by to_pathname will remain
> > the same bothe for user_path_at() and getname_maybe_null()?
> > 	* assuming it is not changed, who said that it will resolve
> > to the same location the second time around?
> > 	* not a race but... the fact that to_dfd does not affect anything
> > in that check looks odd, doesn't it?  And if you try to pass it instead
> > of AT_FDCWD... who said that descriptor will correspond to the same
> > opened file for both?
> > 
> > Besides... assuming that nothing's changing under you, your test is basically
> > "we are not moving anything on top of existing mountpoint" - both path and
> > to_path come from resolving to_pathname, after all.  It doesn't depend upon
> > the thing you are asked to move over there - the check is done before you
> > even look at from_pathname.
> > 
> > What's more, you are breaking the case of mount --move, which had never had
> > that constraint of plain mount.  Same for mount --bind, for that matter.
> > 
> > I agree that it's a regression in mount(8) conversion to new API, but this
> > is not a fix.
> Thanks for the review. Perhaps fixing this in |move_mount| isn't the best
> approach, and I don’t have a good solution yet.

Sorry, no. This restriction never made any sense in the old mount api
and it certainly has no place in the new mount api. And it has been
_years_ since the new mount api was released. Any fix is likely to break
someone else that's already relying on that working.