[PATCH 0/3] wifi: NULL terminate some user strings

[PATCH 0/3] wifi: NULL terminate some user strings

[Dan Carpenter]

These strings come from the user and they need to be NUL terminated.

Dan Carpenter (3):
  wifi: b43: don't pass unterminated string to sscanf()
  wifi: b43legacy: debugfs: NUL terminate string in debugfs
  wifi: carl9170: NUL terminate string in debugfs

 drivers/net/wireless/ath/carl9170/debug.c         | 5 +++--
 drivers/net/wireless/broadcom/b43/debugfs.c       | 2 +-
 drivers/net/wireless/broadcom/b43legacy/debugfs.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH 1/3] wifi: b43: don't pass unterminated string to sscanf()

[Dan Carpenter]

The "buf" buffer is a PAGE_SIZE and allocated with kzalloc().  We store
a string from the user in it and then pass it to sscanf() in the write
functions such as shm32write__write_file().  Only a few numbers are
stored in the buffer so it never comes close to filling up the whole
PAGE_SIZE.  Reserve the last character of the page for a NUL terminator.

This is debugfs so it's root only.

Fixes: 75388acd0cd8 ("[B43LEGACY]: add mac80211-based driver for legacy BCM43xx devices")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
 drivers/net/wireless/broadcom/b43/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/debugfs.c b/drivers/net/wireless/broadcom/b43/debugfs.c
index 31a1ff00c1a4..173ef5db90fc 100644
--- a/drivers/net/wireless/broadcom/b43/debugfs.c
+++ b/drivers/net/wireless/broadcom/b43/debugfs.c
@@ -557,7 +557,7 @@ static ssize_t b43_debugfs_write(struct file *file,
 
 	if (!count)
 		return 0;
-	if (count > PAGE_SIZE)
+	if (count >= PAGE_SIZE)
 		return -E2BIG;
 	dev = file->private_data;
 	if (!dev)
-- 
2.53.0

[PATCH 2/3] wifi: b43legacy: debugfs: NUL terminate string in debugfs

[Dan Carpenter]

The "buf" buffer is PAGE_SIZE and allocated with kzalloc().  We store a
string from the user in it and then pass it to sscanf() in the write
functions such as tsf_write_file().  The buffer is only used to store a
number so we never come close to filling up the whole PAGE_SIZE.  Reserve
the last character of the page for a NUL terminator.

This is debugfs so it's root only.

Fixes: 75388acd0cd8 ("[B43LEGACY]: add mac80211-based driver for legacy BCM43xx devices")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
 drivers/net/wireless/broadcom/b43legacy/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/debugfs.c b/drivers/net/wireless/broadcom/b43legacy/debugfs.c
index a04d90d7307c..7ce8bcf12f0a 100644
--- a/drivers/net/wireless/broadcom/b43legacy/debugfs.c
+++ b/drivers/net/wireless/broadcom/b43legacy/debugfs.c
@@ -259,7 +259,7 @@ static ssize_t b43legacy_debugfs_write(struct file *file,
 
 	if (!count)
 		return 0;
-	if (count > PAGE_SIZE)
+	if (count >= PAGE_SIZE)
 		return -E2BIG;
 	dev = file->private_data;
 	if (!dev)
-- 
2.53.0

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

[Dan Carpenter]

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.

Fixes: 00c4da27a421 ("carl9170: firmware parser and debugfs code")
Signed-off-by: Dan Carpenter <error27@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)
 		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);
 	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';
 
 	if (mutex_trylock(&ar->mutex) == 0) {
 		err = -EAGAIN;
-- 
2.53.0

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

[Christian Lamparter]

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;

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

[Dan Carpenter]

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

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

[Jeff Johnson]

On 10/1/2026 12: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.
> 
> Fixes: 00c4da27a421 ("carl9170: firmware parser and debugfs code")
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Johannes, since the other two in the series go through your tree, you might as
well take all three. I've assigned this one to you in patchwork.

Acked-by: Jeff Johnson <jjohnson@kernel.org>