Re: [PATCH 1/2] memory: brcmstb_dpfe: Fix out-of-bounds access due to DCPU offset

[Krzysztof Kozlowski <krzk@kernel.org>]

On 26/08/2026 22:14, Danesh Petigara wrote:
> From: Justin Chen <justin.chen@broadcom.com>
> 
> On API v1/v2 boards, the DCPU coprocessor can steer kernel readl_relaxed()
> and writel_relaxed() to any address within 256 MB of the ioremapped DPFE
> dmem or regs base. The DCPU firmware provides a 28-bit offset which the
> driver adds to the ioremap base without any bounds checking in
> get_msg_ptr().
> 
> This allows a compromised DCPU firmware to trick the host kernel into
> reading or writing arbitrary memory-mapped I/O registers in vmalloc
> space. When combined with a root-writable sysfs file like dpfe_refresh,
> it provides an arbitrary MMIO write primitive. Similarly, world-readable
> sysfs files can be used to leak other devices' register contents.

If someone can compromise firmware to provide different addresses, then
what stops this person to change DTB with completely different MMIO
ranges for this device?

Isn't better just to drop root-writeable sysfs interfaces, since they
are the insecure parts?

> 
> Fix this by recording the resource_size() of the dmem and regs ioremaps
> at probe time, and rejecting any offset that, along with the largest
> field accessed (DRAM_VENDOR_ERROR + sizeof(u32)), exceeds the recorded
> mapping size.
> 
> Fixes: fee5f1ef6cf7 ("memory: brcmstb: dpfe: support new way of passing data from the DCPU")
> Cc: stable@vger.kernel.org
> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
> Assisted-by: Gemini:gemini-3.1-pro-preview cursor
> Signed-off-by: Danesh Petigara <danesh.petigara@broadcom.com>
> ---
>  drivers/memory/brcmstb_dpfe.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/brcmstb_dpfe.c b/drivers/memory/brcmstb_dpfe.c
> index 08d9e05b1b33..66343205f585 100644
> --- a/drivers/memory/brcmstb_dpfe.c
> +++ b/drivers/memory/brcmstb_dpfe.c
> @@ -182,6 +182,8 @@ struct brcmstb_dpfe_priv {
>  	void __iomem *regs;
>  	void __iomem *dmem;
>  	void __iomem *imem;
> +	resource_size_t regs_size;
> +	resource_size_t dmem_size;
>  	struct device *dev;
>  	const struct dpfe_api *dpfe_api;
>  	struct mutex lock;
> @@ -401,9 +403,14 @@ static void __iomem *get_msg_ptr(struct brcmstb_dpfe_priv *priv, u32 response,
>  	 */
>  	switch (msg_type) {
>  	case 1:
> +		if (DCPU_MSG_RAM_START + offset + DRAM_VENDOR_ERROR +
> +		    sizeof(u32) > priv->regs_size)
> +			goto bad_offset;
>  		ptr = priv->regs + DCPU_MSG_RAM_START + offset;
>  		break;
>  	case 0:
> +		if (offset + DRAM_VENDOR_ERROR + sizeof(u32) > priv->dmem_size)
> +			goto bad_offset;
>  		ptr = priv->dmem + offset;
>  		break;
>  	default:
> @@ -415,6 +422,12 @@ static void __iomem *get_msg_ptr(struct brcmstb_dpfe_priv *priv, u32 response,
>  	}
>  
>  	return ptr;
> +
> +bad_offset:
> +	dev_err(priv->dev, "DCPU returned out-of-range offset %#x\n", offset);
> +	if (buf && size)
> +		*size = sprintf(buf, "ERROR: DCPU offset out of range\n");
> +	return NULL;
>  }
>  
>  static void __finalize_command(struct brcmstb_dpfe_priv *priv)
> @@ -858,6 +871,7 @@ static int brcmstb_dpfe_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct brcmstb_dpfe_priv *priv;
> +	struct resource *res;
>  	int ret;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -869,17 +883,21 @@ static int brcmstb_dpfe_probe(struct platform_device *pdev)
>  	mutex_init(&priv->lock);
>  	platform_set_drvdata(pdev, priv);
>  
> -	priv->regs = devm_platform_ioremap_resource_byname(pdev, "dpfe-cpu");
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dpfe-cpu");

You cannot use devm_platform_get_and_ioremap_resource()? Are the 'reg'
entries flexible/random?


Best regards,
Krzysztof