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

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

[Marek Szyprowski]

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.

Fixes: 0ff0843310b7 ("wifi: brcmfmac: Add optional lpo clock enable support")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 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)
 	sdiodev->bus = brcmf_sdio_probe(sdiodev);
 	if (IS_ERR(sdiodev->bus)) {
 		ret = PTR_ERR(sdiodev->bus);
+		sdiodev->bus = NULL;
 		goto out;
 	}
 	brcmf_sdiod_host_fixup(sdiodev->func2->card->host);
-- 
2.34.1

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

[Arend van Spriel]

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 <m.szyprowski@samsung.com>
> ---
>   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);

Regards,
Arend

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

[Marek Szyprowski]

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 <m.szyprowski@samsung.com>
>> ---
>>   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

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

[Arend van Spriel]

Op 15 januari 2026 00:16:31 schreef Marek Szyprowski 
<m.szyprowski@samsung.com>:

> 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 <m.szyprowski@samsung.com>
>>> ---
>>>  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

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

[Arend van Spriel]

Op 2 februari 2026 19:31:17 schreef Arend van Spriel 
<arend.vanspriel@broadcom.com>:

> Op 15 januari 2026 00:16:31 schreef Marek Szyprowski 
> <m.szyprowski@samsung.com>:
>
>> 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 <m.szyprowski@samsung.com>
>>>> ---
>>>> 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?

Correcting some typos here...

>>
> Sorry for the late response. Indeed the bus instance is allocated and 
> assigned to sdiodev

...in brcmf_sdio_probe.

> So that is

*NOT*

> 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