| Message ID | 20250401140924.29168-1-bsdhenrymartin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [v1] wifi: brcmfmac: acpi: Add NULL check in brcmf_acpi_probe() | expand |
On April 1, 2025 4:09:36 PM Henry Martin <bsdhenrymartin@gmail.com> wrote: > devm_kasprintf() returns NULL when memory allocation fails. Currently, > brcmf_acpi_probe() does not check for this case, which results in a NULL > pointer dereference. > > Add NULL check after devm_kasprintf() to prevent this issue. > > Fixes: 0f485805d008 ("wifi: brcmfmac: acpi: Add support for fetching Apple > ACPI properties") > Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > index c4a54861bfb4..4885d5d0a0af 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > @@ -25,6 +25,8 @@ void brcmf_acpi_probe(struct device *dev, enum > brcmf_bus_type bus_type, > settings->board_type = devm_kasprintf(dev, GFP_KERNEL, > "apple,%s", > o->string.pointer); > + if (!settings->board_type) > + return -ENOMEM; brcmf_acpi_probe() has void return type so this would give a compiler warning. I expect submitted patches at least do pass compilation. After digging through the driver code I see a null check everywhere it is used. Can you please provide a kernel log with a null deref so I may have a clue where it goes wrong? If any. Regards, Arend >
Hi Arend, Thank you for your thorough review and catching the return type mismatch. Upon further investigation, I’ve confirmed that this issue was flagged by static analysis but appears to be a false positive, as all call sites already handle NULL checks appropriately. I appreciate your time and insight—please let me know if you’d like me to drop this patch or revise it differently. Best regards, Henry Martin
On 4/2/2025 4:22 AM, Henry Martin wrote: > Hi Arend, > > Thank you for your thorough review and catching the return type mismatch. Upon > further investigation, I’ve confirmed that this issue was flagged by static > analysis but appears to be a false positive, as all call sites already handle > NULL checks appropriately. > > I appreciate your time and insight—please let me know if you’d like me to drop > this patch or revise it differently. If I look at the code I think the driver probe will eventually fail when the board_type is not available although USB devices seem to be the exception here. For PCIe and SDIO the board_type seems required so we could bail out in brcmf_get_module_param() when there is no board_type found, ie. returning NULL iso settings. I think I found another issue for SDIO. Upon failure it may end up with sdiodev->settings being ERR_PTR() so not NULL. This is not properly handled in the remove path. So drop the patch and I will see if I can incorporate the musings above in some driver patches. Regards, Arend
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c index c4a54861bfb4..4885d5d0a0af 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c @@ -25,6 +25,8 @@ void brcmf_acpi_probe(struct device *dev, enum brcmf_bus_type bus_type, settings->board_type = devm_kasprintf(dev, GFP_KERNEL, "apple,%s", o->string.pointer); + if (!settings->board_type) + return -ENOMEM; } else { brcmf_dbg(INFO, "No ACPI module-instance\n"); return;
devm_kasprintf() returns NULL when memory allocation fails. Currently, brcmf_acpi_probe() does not check for this case, which results in a NULL pointer dereference. Add NULL check after devm_kasprintf() to prevent this issue. Fixes: 0f485805d008 ("wifi: brcmfmac: acpi: Add support for fetching Apple ACPI properties") Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c | 2 ++ 1 file changed, 2 insertions(+)