Re: [PATCH] wifi: brcmfmac: Fix potential kernel oops when probe fails

Op 15 januari 2026 00:16:31 schreef Marek Szyprowski 
<[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?

Sorry for the late response. Indeed the bus instance is allocated and 
assigned to sdiodev. So that is something the caller needs to repeat. So 
changing the return type of brcmf_sdio_probe() makes sense. The only thing 
that should be taken care of in the failure path of brcmf_sdio_probe() is 
to set sdiodev->bus to NULL after calling brcmf_sdio_remove().

Regards,
Arend