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] [day] [month] [year] [list]

Message-ID: <788d8763-0c2c-458a-9b0b-a5634e50c029@163.com>
Date: Sat, 25 Oct 2025 14:02:51 +0800
From: GuangFei Luo <luogf2025@....com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: brauner@...nel.org, 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 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.