Re: [PATCH 1/5] fs/filesystems: Fix potential unsigned integer underflow in fs_name()

[Christian Brauner <brauner@kernel.org>]

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