Re: [PATCH 3/3] wifi: carl9170: NUL terminate string in debugfs

[Christian Lamparter <chunkeey@gmail.com>]

On 10/1/26 9:37 AM, Dan Carpenter wrote:
> The "buf" buffer comes from the user.  We use it to store a number or
> two so it doesn't need to be large.  It gets passed to sscanf() in the
> write functions such as carl9170_debugfs_erp_write().  Ensure that
> buffer is NUL terminated.
> 
> This is debugfs so it's root only.

Sure.
> 
> Fixes: 00c4da27a421 ("carl9170: firmware parser and debugfs code")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Christian Lamparter <chunkeey@gmail.com>

> ---
>   drivers/net/wireless/ath/carl9170/debug.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c
> index 0498df2a2160..bc6c8e0b1d16 100644
> --- a/drivers/net/wireless/ath/carl9170/debug.c
> +++ b/drivers/net/wireless/ath/carl9170/debug.c
> @@ -119,7 +119,7 @@ static ssize_t carl9170_debugfs_write(struct file *file,
>   	if (!count)
>   		return 0;
>   
> -	if (count > PAGE_SIZE)
> +	if (count >= PAGE_SIZE)
heh. It's unnecessary to change this for the max. two numbers we get.
But I'm curious if this is a change that an AI tool added?

But yeah, it's still fine.

>   		return -E2BIG;
>   
>   	ar = file->private_data;
> @@ -131,7 +131,7 @@ static ssize_t carl9170_debugfs_write(struct file *file,
>   	if (!dfops->write)
>   		return -ENOSYS;
>   
> -	buf = vmalloc(count);
> +	buf = vmalloc(count + 1);

I think there's also a vzalloc...

>   	if (!buf)
>   		return -ENOMEM;
>   
> @@ -139,6 +139,7 @@ static ssize_t carl9170_debugfs_write(struct file *file,
>   		err = -EFAULT;
>   		goto out_free;
>   	}
> +	buf[count] = '\0';
which would eliminate that. But yeah, this is fine as well.

>   
>   	if (mutex_trylock(&ar->mutex) == 0) {
>   		err = -EAGAIN;