linux-kernel - Re: [PATCH 1/5] fs/filesystems: Fix potential unsigned integer underflow in fs

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: <20250411-kaiman-bewahren-bef1f1baee8e@brauner>
Date: Fri, 11 Apr 2025 16:35:14 +0200
From: Christian Brauner <brauner@...nel.org>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>, 
	David Howells <dhowells@...hat.com>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH 1/5] fs/filesystems: Fix potential unsigned integer
 underflow in fs_name()

On Thu, Apr 10, 2025 at 07:45:27PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@...cinc.com>
> 
> fs_name() has @index as unsigned int, so there is underflow risk for
> operation '@...ex--'.
> 
> Fix by breaking the for loop when '@...ex == 0' which is also more proper
> than '@...ex <= 0' for unsigned integer comparison.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> ---

This is honestly not worth the effort thinking about.
I'm going to propose that we remove this system call or at least switch
the default to N. Nobody uses this anymore I'm pretty sure.

>  fs/filesystems.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/filesystems.c b/fs/filesystems.c
> index 58b9067b2391ce814e580709b518b405e0f9cb8a..95e5256821a53494d88f496193305a2e50e04444 100644
> --- a/fs/filesystems.c
> +++ b/fs/filesystems.c
> @@ -156,15 +156,19 @@ static int fs_index(const char __user * __name)
>  static int fs_name(unsigned int index, char __user * buf)
>  {
>  	struct file_system_type * tmp;
> -	int len, res;
> +	int len, res = -EINVAL;
>  
>  	read_lock(&file_systems_lock);
> -	for (tmp = file_systems; tmp; tmp = tmp->next, index--)
> -		if (index <= 0 && try_module_get(tmp->owner))
> +	for (tmp = file_systems; tmp; tmp = tmp->next, index--) {
> +		if (index == 0) {
> +			if (try_module_get(tmp->owner))
> +				res = 0;
>  			break;
> +		}
> +	}
>  	read_unlock(&file_systems_lock);
> -	if (!tmp)
> -		return -EINVAL;
> +	if (res)
> +		return res;
>  
>  	/* OK, we got the reference, so we can safely block */
>  	len = strlen(tmp->name) + 1;
> 
> -- 
> 2.34.1
>