Re: [PATCH] wifi: brcmfmac: Fix potential kernel oops when probe fails
Marek Szyprowski <[email protected]> Thu, 15 Jan 2026 00:16:28 +0100
| Newsgroups | dev.linux.lists.brcm80211,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <[email protected]> |
On 14.01.2026 14:45, Arend van Spriel wrote:
> On 12/31/2025 3:35 PM, Marek Szyprowski wrote:
>> When probe of the sdio brcmfmac device fails for some reasons (i.e.
>> missing firmware), the sdiodev->bus is set to error instead of NULL,
>> thus
>> the cleanup later in brcmf_sdio_remove() tries to free resources via
>> invalid bus pointer. Fix this.
>
> Hi Marek,
>
> Thanks for the fix. Please consider my suggestion below...
>
>> Fixes: 0ff0843310b7 ("wifi: brcmfmac: Add optional lpo clock enable
>> support")
>> Signed-off-by: Marek Szyprowski <[email protected]>
>> ---
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git
>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> index 6a3f187320fc..6615748fa5bb 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> @@ -954,6 +954,7 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev
>> *sdiodev)
>
> Maybe use a local variable bus and assign it only on success:
>
>> bus = brcmf_sdio_probe(sdiodev);
>> if (IS_ERR(bus)) {
>> ret = PTR_ERR(bus);
>> goto out;
>> }
>> + sdiodev->bus = bus;
>> brcmf_sdiod_host_fixup(sdiodev->func2->card->host);
>
Well, that would look much better, but sdiodev->bus is also assigned
inside brcmf_sdio_probe() and I didn't check if this is required by the
functions called there or not. Maybe brcmf_sdio_probe() should simply
return error code to make things easier to track?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland