linux-kernel - Re: [PATCH] misc: bcm-vk: validate entry_size before memcpy

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <2025122907-tamale-coveting-a44b@gregkh>
Date: Mon, 29 Dec 2025 11:34:05 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Guangshuo Li <lgs201920130244@...il.com>
Cc: Scott Branden <scott.branden@...adcom.com>,
	Arnd Bergmann <arnd@...db.de>, Olof Johansson <olof@...om.net>,
	Desmond Yan <desmond.yan@...adcom.com>,
	bcm-kernel-feedback-list@...adcom.com, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] misc: bcm-vk: validate entry_size before memcpy_fromio()

On Fri, Dec 19, 2025 at 10:11:57PM +0800, Guangshuo Li wrote:
> The driver trusts the 'num' and 'entry_size' fields read from BAR2 and
> uses them directly to compute the length for memcpy_fromio() without
> any bounds checking. If these fields get corrupted or otherwise contain
> invalid values, num * entry_size can exceed the size of
> proc_mon_info.entries and lead to a potential out-of-bounds write.

But we trust the hardware to get this right, is this suddenly a new
threat-model that you need to worry about for this type of device?  This
is a PCI device, so it is not normally "dynamic" for most types of
systems.

And is this the _only_ place that we trust the data from the device?  If
so, are all other data paths fixed up?

> Add validation for 'entry_size' by ensuring it is non-zero and that
> num * entry_size does not exceed the size of proc_mon_info.entries.
> 
> Fixes: ff428d052b3b ("misc: bcm-vk: add get_card_info, peerlog_info, and proc_mon_info")
> Cc: stable@...r.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@...il.com>
> ---
>  drivers/misc/bcm-vk/bcm_vk_dev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/misc/bcm-vk/bcm_vk_dev.c b/drivers/misc/bcm-vk/bcm_vk_dev.c
> index a16b99bdaa13..a4a74c10f02b 100644
> --- a/drivers/misc/bcm-vk/bcm_vk_dev.c
> +++ b/drivers/misc/bcm-vk/bcm_vk_dev.c
> @@ -439,6 +439,7 @@ static void bcm_vk_get_proc_mon_info(struct bcm_vk *vk)
>  	struct device *dev = &vk->pdev->dev;
>  	struct bcm_vk_proc_mon_info *mon = &vk->proc_mon_info;
>  	u32 num, entry_size, offset, buf_size;
> +	size_t max_bytes;
>  	u8 *dst;
>  
>  	/* calculate offset which is based on peerlog offset */
> @@ -458,6 +459,9 @@ static void bcm_vk_get_proc_mon_info(struct bcm_vk *vk)
>  			num, BCM_VK_PROC_MON_MAX);
>  		return;
>  	}
> +	if (!entry_size || (size_t)num > max_bytes / entry_size) {
> +		return;
> +	}

Any reason you didn't use checkpatch.pl on your submission?  Please
always do so.

And what tool found this issue?  That always must be documented.  And
how was this tested?

thanks,

greg k-h