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

[Dan Carpenter <error27@gmail.com>]

On Thu, Oct 01, 2026 at 07:58:13PM +0200, Christian Lamparter wrote:
> 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?

No.  The other two patches used kzalloc() to allocate their buffers so
changing "> PAGE_SIZE" to ">= PAGE_SIZE" fixed the bug.  But as I was
writing this, I decided that there was no way I was going to let people
allocat PAGE_SIZE + 1 bytes...  :P  Plus, since I was adding a byte
later it kind of maintained the status quo to subtract one here.

I did use AI to write the Smatch check.  ;)

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

I was more considering changing this to kzalloc() but I decided that
was too much change.

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

Thanks!

regards,
dan carpenter