diff mbox series

[v3,2/9] brcmfmac: firmware: Allocate space for default boardrev in nvram

Message ID 20220117142919.207370-3-marcan@marcan.st (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series misc brcmfmac fixes (M1/T2 series spin-off) | expand

Commit Message

Hector Martin Jan. 17, 2022, 2:29 p.m. UTC
If boardrev is missing from the NVRAM we add a default one, but this
might need more space in the output buffer than was allocated. Ensure
we have enough padding for this in the buffer.

Fixes: 46f2b38a91b0 ("brcmfmac: insert default boardrev in nvram data if missing")
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Arend van Spriel Jan. 19, 2022, 12:34 p.m. UTC | #1
On 1/17/2022 3:29 PM, Hector Martin wrote:
> If boardrev is missing from the NVRAM we add a default one, but this
> might need more space in the output buffer than was allocated. Ensure
> we have enough padding for this in the buffer.
> 
> Fixes: 46f2b38a91b0 ("brcmfmac: insert default boardrev in nvram data if missing")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 ++
>   1 file changed, 2 insertions(+)
Andy Shevchenko Jan. 19, 2022, 9:35 p.m. UTC | #2
On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>
> If boardrev is missing from the NVRAM we add a default one, but this
> might need more space in the output buffer than was allocated. Ensure
> we have enough padding for this in the buffer.

Do you know this ahead (before allocation happens)?
If yes, the size change should happen conditionally.

Alternatively, krealloc() may be used.
Hector Martin Jan. 20, 2022, 6:08 a.m. UTC | #3
On 20/01/2022 06.35, Andy Shevchenko wrote:
> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>>
>> If boardrev is missing from the NVRAM we add a default one, but this
>> might need more space in the output buffer than was allocated. Ensure
>> we have enough padding for this in the buffer.
> 
> Do you know this ahead (before allocation happens)?
> If yes, the size change should happen conditionally.
> 
> Alternatively, krealloc() may be used.

We don't know if we will have to add a boardrev until we find it's
missing. The buffer is already oversized anyway, as its size is computed
based on the input NVRAM size, while comments and such will be removed.
This is just increasing the existing worst-case allocation to properly
account for the true worst-case case.

I don't think trying to keep the buffer perfectly sized buys us anything
here, since it will be freed after download anyway. There's no point in
saving a few bytes of memory in the interim.
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 0eb13e5df517..1001c8888bfe 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -207,6 +207,8 @@  static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
 		size = BRCMF_FW_MAX_NVRAM_SIZE;
 	else
 		size = data_len;
+	/* Add space for properties we may add */
+	size += strlen(BRCMF_FW_DEFAULT_BOARDREV) + 1;
 	/* Alloc for extra 0 byte + roundup by 4 + length field */
 	size += 1 + 3 + sizeof(u32);
 	nvp->nvram = kzalloc(size, GFP_KERNEL);