[PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes

Pengpeng Hou posted 1 patch 1 month, 2 weeks ago
drivers/memory/brcmstb_dpfe.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
[PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes
Posted by Pengpeng Hou 1 month, 2 weeks ago
The firmware header is read before the image has been shown to contain a
complete header. In addition, the final size check adds two
firmware-provided u32 section lengths before comparing the result with
fw->size, so the addition can wrap.

Reject images shorter than the fixed header and checksum before reading
the header. Then derive the available payload length with subtraction and
require the two declared sections to fill it exactly.

Fixes: 2f330caff577 ("memory: brcmstb: Add driver for DPFE")

Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1: https://lore.kernel.org/all/20260706092223.78541-1-pengpeng@iscas.ac.cn/
- explain why the early fixed-header check is not redundant
- replace the overflowing addition with an exact payload check
- disclose the use of Codex

 drivers/memory/brcmstb_dpfe.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/memory/brcmstb_dpfe.c b/drivers/memory/brcmstb_dpfe.c
index 08d9e05b1b33..121d1a0580dd 100644
--- a/drivers/memory/brcmstb_dpfe.c
+++ b/drivers/memory/brcmstb_dpfe.c
@@ -518,9 +518,15 @@ static int __verify_firmware(struct init_data *init,
 			     const struct firmware *fw)
 {
 	const struct dpfe_firmware_header *header = (void *)fw->data;
-	unsigned int dmem_size, imem_size, total_size;
+	unsigned int dmem_size, imem_size;
 	bool is_big_endian = false;
 	const u32 *chksum_ptr;
+	size_t payload_size;
+
+	if (fw->size < sizeof(*header) + sizeof(*chksum_ptr))
+		return ERR_INVALID_SIZE;
+
+	payload_size = fw->size - sizeof(*header) - sizeof(*chksum_ptr);
 
 	if (header->magic == DPFE_BE_MAGIC)
 		is_big_endian = true;
@@ -539,13 +545,8 @@ static int __verify_firmware(struct init_data *init,
 	if ((dmem_size % sizeof(u32)) != 0 || (imem_size % sizeof(u32)) != 0)
 		return ERR_INVALID_SIZE;
 
-	/*
-	 * The header + the data section + the instruction section + the
-	 * checksum must be equal to the total firmware size.
-	 */
-	total_size = dmem_size + imem_size + sizeof(*header) +
-		sizeof(*chksum_ptr);
-	if (total_size != fw->size)
+	/* The data and instruction sections must fill the payload exactly. */
+	if (dmem_size > payload_size || imem_size != payload_size - dmem_size)
 		return ERR_INVALID_SIZE;
 
 	/* The checksum comes at the very end. */
-- 
2.50.1 (Apple Git-155)
Re: [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes
Posted by Krzysztof Kozlowski 3 weeks, 3 days ago
On 15/08/2026 15:45, Pengpeng Hou wrote:
> The firmware header is read before the image has been shown to contain a
> complete header. In addition, the final size check adds two
> firmware-provided u32 section lengths before comparing the result with
> fw->size, so the addition can wrap.
> 
> Reject images shorter than the fixed header and checksum before reading
> the header. Then derive the available payload length with subtraction and
> require the two declared sections to fill it exactly.
> 
> Fixes: 2f330caff577 ("memory: brcmstb: Add driver for DPFE")
> 

There is no blank line around tags.

> Assisted-by: Codex:gpt-5

So if you ask some LLM tools, then ask them how to write correct Linux
kernel commit, please.

Applied.

Best regards,
Krzysztof
Re: [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes
Posted by Krzysztof Kozlowski 3 weeks, 3 days ago
On Sat, 15 Aug 2026 21:45:59 +0800, Pengpeng Hou wrote:
> The firmware header is read before the image has been shown to contain a
> complete header. In addition, the final size check adds two
> firmware-provided u32 section lengths before comparing the result with
> fw->size, so the addition can wrap.
> 
> Reject images shorter than the fixed header and checksum before reading
> the header. Then derive the available payload length with subtraction and
> require the two declared sections to fill it exactly.
> 
> [...]

Applied, thanks!

[1/1] memory: brcmstb_dpfe: validate firmware section sizes
      https://git.kernel.org/krzk/linux-mem-ctrl/c/e61c1a1103e2617e8f86288368911a8591e35c1e

Best regards,
-- 
Krzysztof Kozlowski <krzk@kernel.org>