| 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 |
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(+)
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.
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 --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);
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(+)