netdev - Re: [PATCH] bna: prevent bad user input in bnad_debugfs_write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <75e4c931-3e17-430a-a902-60f9d8161bc3@lunn.ch>
Date: Thu, 30 Oct 2025 18:37:55 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Miaoqian Lin <linmq006@...il.com>
Cc: Rasesh Mody <rmody@...vell.com>,
	Sudarsana Kalluru <skalluru@...vell.com>,
	GR-Linux-NIC-Dev@...vell.com, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Ivan Vecera <ivecera@...hat.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] bna: prevent bad user input in bnad_debugfs_write_regrd()

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@...r.kernel.org
> Signed-off-by: Miaoqian Lin <linmq006@...il.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