[PATCH] wifi: brcmfmac: bound NVRAM comment parsing

Pengpeng Hou posted 1 patch 3 weeks, 6 days ago
There is a newer version of this series
.../net/wireless/broadcom/brcm80211/brcmfmac/firmware.c    | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
[PATCH] wifi: brcmfmac: bound NVRAM comment parsing
Posted by Pengpeng Hou 3 weeks, 6 days ago
The NVRAM parser tracks a length-bounded firmware blob but its comment
handler uses unbounded strchr() calls. A comment without a newline or NUL
inside the remaining blob can make the search read beyond the current
input.

Store the bounded input length and use memchr() for comment terminators.

Fixes: 3e99b08ab53c ("brcmfmac: enhance nvram processing")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/firmware.c    | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 22ff326f1924a..c68dee6d7b344 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -37,6 +37,7 @@ enum nvram_parser_state {
  *
  * @state: current parser state.
  * @data: input buffer being parsed.
+ * @data_len: size of the input buffer.
  * @nvram: output buffer with parse result.
  * @nvram_len: length of parse result.
  * @line: current line.
@@ -51,6 +52,7 @@ enum nvram_parser_state {
 struct nvram_parser {
 	enum nvram_parser_state state;
 	const u8 *data;
+	u32 data_len;
 	u8 *nvram;
 	u32 nvram_len;
 	u32 line;
@@ -171,12 +173,15 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp)
 static enum nvram_parser_state
 brcmf_nvram_handle_comment(struct nvram_parser *nvp)
 {
-	char *eoc, *sol;
+	char *eoc;
+	char *sol;
+	size_t remaining;
 
 	sol = (char *)&nvp->data[nvp->pos];
-	eoc = strchr(sol, '\n');
+	remaining = nvp->data_len - nvp->pos;
+	eoc = memchr(sol, '\n', remaining);
 	if (!eoc) {
-		eoc = strchr(sol, '\0');
+		eoc = memchr(sol, '\0', remaining);
 		if (!eoc)
 			return END;
 	}
@@ -215,6 +220,7 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
 		size = BRCMF_FW_MAX_NVRAM_SIZE;
 	else
 		size = data_len;
+	nvp->data_len = size;
 	/* Add space for properties we may add */
 	size += strlen(BRCMF_FW_DEFAULT_BOARDREV) + 1;
 	size += BRCMF_FW_MACADDR_LEN + 1;
@@ -408,7 +414,7 @@ static void *brcmf_fw_nvram_strip(const u8 *data, size_t data_len,
 	if (eth_platform_get_mac_address(dev, mac) == 0)
 		nvp.strip_mac = true;
 
-	while (nvp.pos < data_len) {
+	while (nvp.pos < nvp.data_len) {
 		nvp.state = nv_parser_states[nvp.state](&nvp);
 		if (nvp.state == END)
 			break;

base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72
-- 
2.50.1
Re: [PATCH] wifi: brcmfmac: bound NVRAM comment parsing
Posted by Arend van Spriel 1 week, 5 days ago
On Sun, 30 Aug 2026 21:53:28 +0800, Pengpeng Hou wrote:
> The NVRAM parser tracks a length-bounded firmware blob but its comment
> handler uses unbounded strchr() calls. A comment without a newline or NUL
> inside the remaining blob can make the search read beyond the current
> input.
> 
> Store the bounded input length and use memchr() for comment terminators.

Please add:
Cc: stable@vger.kernel.org

> Fixes: 3e99b08ab53c ("brcmfmac: enhance nvram processing")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

[...]

> @@ -215,6 +220,7 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
>  		size = BRCMF_FW_MAX_NVRAM_SIZE;
>  	else
>  		size = data_len;
> +	nvp->data_len = size;
>  	/* Add space for properties we may add */
>  	size += strlen(BRCMF_FW_DEFAULT_BOARDREV) + 1;
>  	size += BRCMF_FW_MACADDR_LEN + 1;

size is capped at BRCMF_FW_MAX_NVRAM_SIZE to allocate the output buffer,
as some NVRAM files can be larger than 64KB due to comments. Setting
nvp->data_len to size will prematurely truncate parsing of those files.
This should be:

	nvp->data_len = data_len;

Regards,
Arend