[PATCH] bna: prevent bad user input in bnad_debugfs_write_regrd()

[PATCH] bna: prevent bad user input in bnad_debugfs_write_regrd()

[Miaoqian Lin]

A malicious user could pass an arbitrarily bad value
to memdup_user_nul(), potentially causing kernel crash.

This follows the same pattern as commit ee76746387f6
("netdevsim: prevent bad user input in nsim_dev_health_break_write()")
and commit 7ef4c19d245f
("smackfs: restrict bytes count in smackfs write functions")

Found via static analysis and code review.

Fixes: d0e6a8064c42 ("bna: use memdup_user to copy userspace buffers")
Cc: stable@vger.kernel.org
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
 drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
index 8f0972e6737c..ad33ab1d266d 100644
--- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
+++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
@@ -311,6 +311,9 @@ bnad_debugfs_write_regrd(struct file *file, const char __user *buf,
 	unsigned long flags;
 	void *kern_buf;
 
+	if (nbytes == 0 || nbytes > PAGE_SIZE)
+		return -EINVAL;
+
 	/* Copy the user space buf */
 	kern_buf = memdup_user_nul(buf, nbytes);
 	if (IS_ERR(kern_buf))
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] bna: prevent bad user input in bnad_debugfs_write_regrd()

[Andrew Lunn]

On Thu, Oct 30, 2025 at 01:34:10PM +0800, Miaoqian Lin wrote:
> A malicious user could pass an arbitrarily bad value
> to memdup_user_nul(), potentially causing kernel crash.

How would it crash the kernel? I would expect memdup_user_nul() to
either succeed or fail and return a NULL.

However, adding a range check does make sense.

> This follows the same pattern as commit ee76746387f6
> ("netdevsim: prevent bad user input in nsim_dev_health_break_write()")
> and commit 7ef4c19d245f
> ("smackfs: restrict bytes count in smackfs write functions")
> 
> Found via static analysis and code review.
> 
> Fixes: d0e6a8064c42 ("bna: use memdup_user to copy userspace buffers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
>  drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> index 8f0972e6737c..ad33ab1d266d 100644
> --- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> +++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> @@ -311,6 +311,9 @@ bnad_debugfs_write_regrd(struct file *file, const char __user *buf,
>  	unsigned long flags;
>  	void *kern_buf;
>  
> +	if (nbytes == 0 || nbytes > PAGE_SIZE)
> +		return -EINVAL;
> +
>  	/* Copy the user space buf */
>  	kern_buf = memdup_user_nul(buf, nbytes);
>  	if (IS_ERR(kern_buf))

Look at what it does next:

rc = sscanf(kern_buf, "%x:%x", &addr, &len);

What is the maximum length of "%x:%x" ? A lot less than PAGE_SIZE. So
you can make the range check much smaller.

Also, what about bnad_debugfs_write_regwr()? If you find a bug, look
around, the same bug might be repeated close by. You might also want
to look at your static analysis tool and find out why it did not
report that function.

    Andrew

---
pw-bot: cr