Re: [PATCH] ARM: dts: BCM5301X: R7000: add nvram node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing length validation in `brcm_nvram_parse` allows for out-of-bounds heap read/write and potential panics (preexisting).
--

commit fcc6c069f7250c796767f24fbe9f085f9b0e3334
Author: Rosen Penev <rosenp@gmail.com>

ARM: dts: BCM5301X: R7000: add nvram node

This commit adds the nvram node to the BCM4709 Netgear R7000 device tree
and configures it for gmac0.

> diff --git a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r7000.dts b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r7000.dts
> index 24ba8f8f9bf36..0f3a00bc38c14 100644
> --- a/arch/arm/boot/dts/broadcom/bcm4709-netgear-r7000.dts
> +++ b/arch/arm/boot/dts/broadcom/bcm4709-netgear-r7000.dts
> @@ -25,6 +25,15 @@ memory@0 {
>  		      <0x88000000 0x08000000>;
>  	};
>  
> +	nvram@1c080000 {
> +		compatible = "brcm,nvram";

[Severity: High]
This is a pre-existing issue, but adding this DTS node exposes the device to a
missing length validation in the brcm_nvram driver.

In drivers/nvmem/brcm_nvram.c:brcm_nvram_parse(), if the NVRAM flash is
completely erased or uninitialized (e.g. all 0xFF or 0x00),
brcm_nvram_copy_data() calculates priv->data_len as 0. This causes
devm_kzalloc() to return ZERO_SIZE_PTR.

Will the subsequent memcmp() dereference this invalid pointer and cause a
panic?

    if (memcmp(header->magic, NVRAM_MAGIC, 4)) {

Furthermore, if len is larger than priv->data_len, it is validated against
the mapped region size (priv->nvmem_size) instead of the allocated heap
size:

    if (len > priv->nvmem_size) {

Can this lead to an out-of-bounds heap read and write in
brcm_nvram_add_cells()?

    tmp = priv->data[len - 1];
    priv->data[len - 1] = '\0';

> +		reg = <0x1c080000 0x180000>;
> +
> +		et0macaddr: et0macaddr {
> +			#nvmem-cell-cells = <1>;
> +		};
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910001300.117062-1-rosenp@gmail.com?part=1