[PATCH 0/2] Harden DPFE driver against buggy/malicious DCPU

[PATCH 0/2] Harden DPFE driver against buggy/malicious DCPU

[Danesh Petigara]

This patch series hardens the Broadcom STB DPFE driver against potential
exploitation through a buggy or compromised DCPU coprocessor.

The DPFE driver exchanges messages with the DCPU over shared memory-mapped
regions and currently trusts DCPU supplied values without bounds checking
them before use.

A compromised or malfunctioning DCPU could:

- On API v1/v2 boards, provide an arbitrary MMIO read/write primitive
  through the offset supplied in get_msg_ptr(), combined with
  root-writable and world-readable sysfs files.

- Make a user who reads world-readable sysfs files trigger an out-of-bounds
  stack read of arbitrary extent via an unvalidated checksum index.

Patch 1 addresses the MMIO read/write issue by rejecting an offset that,
along with the largest field accessed (DRAM_VENDOR_ERROR + sizeof(u32)),
exceeds the recorded mapping size.

Patch 2 addresses the stack over-read via the checksum index by checking
it against MSG_FIELD_MAX after reading the response from the DCPU message
RAM.

Justin Chen (2):
  memory: brcmstb_dpfe: Fix out-of-bounds access due to DCPU offset
  memory: brcmstb_dpfe: Bounds check DCPU-supplied MSG_ARG_COUNT

 drivers/memory/brcmstb_dpfe.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

-- 
2.54.0

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

[Danesh Petigara]

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.

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");
+	priv->regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(priv->regs)) {
 		dev_err(dev, "couldn't map DCPU registers\n");
 		return -ENODEV;
 	}
+	priv->regs_size = resource_size(res);
 
-	priv->dmem = devm_platform_ioremap_resource_byname(pdev, "dpfe-dmem");
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dpfe-dmem");
+	priv->dmem = devm_ioremap_resource(dev, res);
 	if (IS_ERR(priv->dmem)) {
 		dev_err(dev, "Couldn't map DCPU data memory\n");
 		return -ENOENT;
 	}
+	priv->dmem_size = resource_size(res);
 
 	priv->imem = devm_platform_ioremap_resource_byname(pdev, "dpfe-imem");
 	if (IS_ERR(priv->imem)) {
-- 
2.54.0

[PATCH 2/2] memory: brcmstb_dpfe: Bounds check DCPU-supplied MSG_ARG_COUNT

[Danesh Petigara]

From: Justin Chen <justin.chen@broadcom.com>

A compromised or buggy DCPU coprocessor can make any user who reads
world-readable /sys/devices/.../dpfe_info|dpfe_dram trigger a kernel
stack out-of-bounds read of arbitrary extent. This bounds checks
chksum_idx against MSG_FIELD_MAX after reading the response from DCPU
message RAM.

Fixes: 5d06f53d9509 ("memory: brcmstb: dpfe: Compute checksum at __send_command() time")
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 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/memory/brcmstb_dpfe.c b/drivers/memory/brcmstb_dpfe.c
index 66343205f585..7b02c36c7fca 100644
--- a/drivers/memory/brcmstb_dpfe.c
+++ b/drivers/memory/brcmstb_dpfe.c
@@ -503,6 +503,8 @@ static int __send_command(struct brcmstb_dpfe_priv *priv, unsigned int cmd,
 		for (i = 0; i < MSG_FIELD_MAX; i++)
 			result[i] = readl_relaxed(regs + DCPU_MSG_RAM(i));
 		chksum_idx = result[MSG_ARG_COUNT] + MSG_ARG_COUNT + 1;
+		if (chksum_idx >= MSG_FIELD_MAX)
+			ret = -EINVAL;
 	}
 
 	/* Tell DCPU we are done */
-- 
2.54.0

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

[Florian Fainelli]

On 8/26/26 13: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.
> 
> 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>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH 2/2] memory: brcmstb_dpfe: Bounds check DCPU-supplied MSG_ARG_COUNT

[Florian Fainelli]

On 8/26/26 13:14, Danesh Petigara wrote:
> From: Justin Chen <justin.chen@broadcom.com>
> 
> A compromised or buggy DCPU coprocessor can make any user who reads
> world-readable /sys/devices/.../dpfe_info|dpfe_dram trigger a kernel
> stack out-of-bounds read of arbitrary extent. This bounds checks
> chksum_idx against MSG_FIELD_MAX after reading the response from DCPU
> message RAM.
> 
> Fixes: 5d06f53d9509 ("memory: brcmstb: dpfe: Compute checksum at __send_command() time")
> 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>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

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

[Krzysztof Kozlowski]

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

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

[Florian Fainelli]

On 8/26/26 23:48, Krzysztof Kozlowski wrote:
> 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?

On our platforms the DTB is part of the boot loader which is 
signed+encrypted and customers do not build any UART driver or 
user-interface so changing the DTB is somewhat difficult. Same goes with 
the Linux kernel, it's also signed+encrypted. This is defense in depth 
at this point.

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

Yes that would probably be an acceptable move forward. Markus do you 
remember what was the use case for the dpfe_refresh file to be R/W?

> 
>>
>> 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?

IIRC the names and indexes are stable so we could do a positional index 
resource lookup, but maybe what we want is to introduce a 
devm_platform_get_and_ioremap_resource_by_name()?
-- 
Florian