[PATCH] mount: fix duplicate mounts using the new mount API

[PATCH] mount: fix duplicate mounts using the new mount API

[GuangFei Luo]

When using the new mount API, invoking the same mount command multiple
times such as:
mount /dev/mapper/mydevice /mnt/mydisk2
results in multiple identical mount entries being created:
/dev/mapper/mydevice on /mnt/mydisk2 type ext4 (rw,relatime)
/dev/mapper/mydevice on /mnt/mydisk2 type ext4 (rw,relatime)
/dev/mapper/mydevice on /mnt/mydisk2 type ext4 (rw,relatime)
...

Fixes: 2db154b3ea8e ("vfs: syscall: Add move_mount(2) to move mounts around")
Signed-off-by: GuangFei Luo <luogf2025@163.com>
Cc: <stable@vger.kernel.org> # 5.2+
---
 fs/namespace.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/namespace.c b/fs/namespace.c
index d82910f33dc4..9436471955ce 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -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;
+
 	uflags = 0;
 	if (flags & MOVE_MOUNT_F_EMPTY_PATH)
 		uflags = AT_EMPTY_PATH;
-- 
2.43.0

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

[Al Viro]

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.

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

[GuangFei Luo]

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.

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

[kernel test robot]

Hello,

kernel test robot noticed "xfstests.xfs.153.fail" on:

commit: ad04c4e00fd850d57b3a07e1920e4e44e6dc393b ("[PATCH] mount: fix duplicate mounts using the new mount API")
url: https://github.com/intel-lab-lkp/linux/commits/GuangFei-Luo/mount-fix-duplicate-mounts-using-the-new-mount-API/20251025-105257
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 566771afc7a81e343da9939f0bd848d3622e2501
patch link: https://lore.kernel.org/all/20251025024934.1350492-1-luogf2025@163.com/
patch subject: [PATCH] mount: fix duplicate mounts using the new mount API

in testcase: xfstests
version: xfstests-x86_64-2cba4b54-1_20251020
with following parameters:

	disk: 4HDD
	fs: xfs
	test: xfs-153



config: x86_64-rhel-9.4-func
compiler: gcc-14
test machine: 4 threads Intel(R) Xeon(R) CPU E3-1225 v5 @ 3.30GHz (Skylake) with 16G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202510291004.24ca6327-lkp@intel.com

2025-10-28 06:08:21 cd /lkp/benchmarks/xfstests
2025-10-28 06:08:23 export TEST_DIR=/fs/sda1
2025-10-28 06:08:23 export TEST_DEV=/dev/sda1
2025-10-28 06:08:23 export FSTYP=xfs
2025-10-28 06:08:23 export SCRATCH_MNT=/fs/scratch
2025-10-28 06:08:23 mkdir /fs/scratch -p
2025-10-28 06:08:23 export SCRATCH_DEV=/dev/sda4
2025-10-28 06:08:23 export SCRATCH_LOGDEV=/dev/sda2
2025-10-28 06:08:23 export SCRATCH_XFS_LIST_METADATA_FIELDS=u3.sfdir3.hdr.parent.i4
2025-10-28 06:08:23 export SCRATCH_XFS_LIST_FUZZ_VERBS=random
2025-10-28 06:08:23 echo xfs/153
2025-10-28 06:08:23 ./check xfs/153
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/x86_64 lkp-skl-d06 6.18.0-rc2-00237-gad04c4e00fd8 #1 SMP PREEMPT_DYNAMIC Tue Oct 28 13:58:10 CST 2025
MKFS_OPTIONS  -- -f /dev/sda4
MOUNT_OPTIONS -- /dev/sda4 /fs/scratch

xfs/153       [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//xfs/153.out.bad)
    --- tests/xfs/153.out	2025-10-20 16:48:15.000000000 +0000
    +++ /lkp/benchmarks/xfstests/results//xfs/153.out.bad	2025-10-28 06:08:30.851957601 +0000
    @@ -11,119 +11,6 @@
     [ROOT] 0 0 0 00 [--------] 3 0 0 00 [--------] 0 0 0 00 [--------]
     
     *** report initial settings
    -[ROOT] 0 0 0 00 [--------] 3 0 0 00 [--------] 0 0 0 00 [--------]
    -[NAME] 0 200 1000 00 [--------] 1 4 10 00 [--------] 0 0 0 00 [--------]
    -
    -*** push past the soft inode limit
    ...
    (Run 'diff -u /lkp/benchmarks/xfstests/tests/xfs/153.out /lkp/benchmarks/xfstests/results//xfs/153.out.bad'  to see the entire diff)
Ran: xfs/153
Failures: xfs/153
Failed 1 of 1 tests




The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20251029/202510291004.24ca6327-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Christian Brauner]

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.

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

[Al Viro]

On Fri, Oct 31, 2025 at 01:54:27PM +0100, Christian Brauner wrote:

> > > 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.

Not quite...  I agree that it makes little sense to do that on syscall level,
but conversion of mount(8) to new API is a different story - that's more recent
than the introduction of new API itself and it does create a regression on
the userland side.

IIRC, the original rationale had been "what if somebody keeps clicking on
something in some kind of filemangler inturdface and gets a pile of overmounts
there?", but however weak that might be, it is an established behaviour of
mount(2), with userland callers of mount(2) expecting that semantics.

Blind conversion to new API has changed userland behaviour.  I would argue
that it's a problem on the userland side, and the only question kernel-side
is whether there is something we could provide to simplify the life of those
who do such userland conversions.  A move_mount(2) flag, perhaps, defaulting
to what we have move_mount(2) do now?

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

[Christian Brauner]

On Fri, Oct 31, 2025 at 06:48:22PM +0000, Al Viro wrote:
> On Fri, Oct 31, 2025 at 01:54:27PM +0100, Christian Brauner wrote:
> 
> > > > 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.
> 
> Not quite...  I agree that it makes little sense to do that on syscall level,
> but conversion of mount(8) to new API is a different story - that's more recent
> than the introduction of new API itself and it does create a regression on
> the userland side.
> 
> IIRC, the original rationale had been "what if somebody keeps clicking on
> something in some kind of filemangler inturdface and gets a pile of overmounts
> there?", but however weak that might be, it is an established behaviour of
> mount(2), with userland callers of mount(2) expecting that semantics.
> 
> Blind conversion to new API has changed userland behaviour.  I would argue
> that it's a problem on the userland side, and the only question kernel-side
> is whether there is something we could provide to simplify the life of those
> who do such userland conversions.  A move_mount(2) flag, perhaps, defaulting
> to what we have move_mount(2) do now?

Maybe a flag but even then. I'm pretty sure that mount can just use
statmount() to figure out that someone is trying to mount the same fs
twice and simply abort. That should be close enough...