[PATCH] misc: bcm-vk: validate entry_size before memcpy_fromio()

[PATCH] misc: bcm-vk: validate entry_size before memcpy_fromio()

[Guangshuo Li]

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.

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@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.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;
+	}
 	mon->num = num;
 	mon->entry_size = entry_size;
 
-- 
2.43.0

Re: [PATCH] misc: bcm-vk: validate entry_size before memcpy_fromio()

[Scott Branden]

[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]

On Fri, Dec 19, 2025 at 6:12 AM Guangshuo Li <lgs201920130244@gmail.com> 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.
>
> 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@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
Acked-by: Scott Branden <scott.branden@broadcom.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;
> +       }
>         mon->num = num;
>         mon->entry_size = entry_size;
>
> --
> 2.43.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5473 bytes --]

Re: [PATCH] misc: bcm-vk: validate entry_size before memcpy_fromio()

[Greg Kroah-Hartman]

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@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.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