mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes
@ 2026-08-15 13:45 Pengpeng Hou
  2026-09-04 13:53 ` Krzysztof Kozlowski
  2026-09-04 13:54 ` Krzysztof Kozlowski
  0 siblings, 2 replies; 3+ messages in thread
From: Pengpeng Hou @ 2026-08-15 13:45 UTC (permalink / raw)
  To: mmayer, Krzysztof Kozlowski
  Cc: Pengpeng Hou, Broadcom internal kernel review list,
	Florian Fainelli, linux-arm-kernel, linux-kernel

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)


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes
  2026-08-15 13:45 [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes Pengpeng Hou
@ 2026-09-04 13:53 ` Krzysztof Kozlowski
  2026-09-04 13:54 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-04 13:53 UTC (permalink / raw)
  To: Pengpeng Hou, mmayer
  Cc: Broadcom internal kernel review list, Florian Fainelli,
	linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes
  2026-08-15 13:45 [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes Pengpeng Hou
  2026-09-04 13:53 ` Krzysztof Kozlowski
@ 2026-09-04 13:54 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-04 13:54 UTC (permalink / raw)
  To: mmayer, Pengpeng Hou
  Cc: Broadcom internal kernel review list, Florian Fainelli,
	linux-arm-kernel, linux-kernel


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>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-04 13:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-15 13:45 [PATCH v2] memory: brcmstb_dpfe: validate firmware section sizes Pengpeng Hou
2026-09-04 13:53 ` Krzysztof Kozlowski
2026-09-04 13:54 ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®