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