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

[PATCH v2] 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. This happens because sdiodev->bus is set 2 times:
first in brcmf_sdio_probe() and second time in brcmf_sdiod_probe(). Fix
this by chaning the brcmf_sdio_probe() function to return the error code
and set sdio->bus only there.

Fixes: 0ff0843310b7 ("wifi: brcmfmac: Add optional lpo clock enable support")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Changelog:
v2:
- changed return type of brcmf_sdio_probe() function

v1: https://lore.kernel.org/all/20251231143544.4158840-1-m.szyprowski@samsung.com
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 7 +++----
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c   | 7 ++++---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h   | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 6a3f187320fc..13952dfeb3e3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -951,11 +951,10 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 		goto out;
 
 	/* try to attach to the target device */
-	sdiodev->bus = brcmf_sdio_probe(sdiodev);
-	if (IS_ERR(sdiodev->bus)) {
-		ret = PTR_ERR(sdiodev->bus);
+	ret = brcmf_sdio_probe(sdiodev);
+	if (ret)
 		goto out;
-	}
+
 	brcmf_sdiod_host_fixup(sdiodev->func2->card->host);
 out:
 	if (ret)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 8cf9d7e7c3f7..4e6ed02c1591 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4445,7 +4445,7 @@ brcmf_sdio_prepare_fw_request(struct brcmf_sdio *bus)
 	return fwreq;
 }
 
-struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
+int brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 {
 	int ret;
 	struct brcmf_sdio *bus;
@@ -4551,11 +4551,12 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 		goto fail;
 	}
 
-	return bus;
+	return 0;
 
 fail:
 	brcmf_sdio_remove(bus);
-	return ERR_PTR(ret);
+	sdiodev->bus = NULL;
+	return ret;
 }
 
 /* Detach and free everything */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index 0d18ed15b403..80180d5c6c87 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -358,7 +358,7 @@ void brcmf_sdiod_freezer_uncount(struct brcmf_sdio_dev *sdiodev);
 int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev);
 int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev);
 
-struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev);
+int brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev);
 void brcmf_sdio_remove(struct brcmf_sdio *bus);
 void brcmf_sdio_isr(struct brcmf_sdio *bus, bool in_isr);
 
-- 
2.34.1

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

[Arend van Spriel]

On 03/02/2026 11:21, 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. This happens because sdiodev->bus is set 2 times:
> first in brcmf_sdio_probe() and second time in brcmf_sdiod_probe(). Fix
> this by chaning the brcmf_sdio_probe() function to return the error code
> and set sdio->bus only there.

Looks much better, but wanted to see what was done in the commit 
referenced in the Fixes: tag.

> Fixes: 0ff0843310b7 ("wifi: brcmfmac: Add optional lpo clock enable support")

So that patch wanted to propagate the result of 
devm_clk_get_optional_enabled() call in brcmf_of_probe():of.c to retry 
later. It ends up in brcmf_sdio_probe_attach() which is called in 
brcmf_sdio_probe(). Anyway, it seems okay to me. Will give it a spin in 
my setup just to be sure.

Thanks,
Arend

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

[Marek Szyprowski]

On 03.02.2026 12:45, Arend van Spriel wrote:
> On 03/02/2026 11:21, 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. This happens because sdiodev->bus is set 2 times:
>> first in brcmf_sdio_probe() and second time in brcmf_sdiod_probe(). Fix
>> this by chaning the brcmf_sdio_probe() function to return the error code
>> and set sdio->bus only there.
>
> Looks much better, but wanted to see what was done in the commit 
> referenced in the Fixes: tag.
>
>> Fixes: 0ff0843310b7 ("wifi: brcmfmac: Add optional lpo clock enable 
>> support")
>
That was the commit which introduced this regression, 
earlierbrcmf_sdio_probe() returned NULL in case of error and the check 
for sdio->bus was fine.

> So that patch wanted to propagate the result of 
> devm_clk_get_optional_enabled() call in brcmf_of_probe():of.c to retry 
> later. It ends up in brcmf_sdio_probe_attach() which is called in 
> brcmf_sdio_probe(). Anyway, it seems okay to me. Will give it a spin 
> in my setup just to be sure.
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Arend van Spriel]

Op 3 februari 2026 11:21:51 schreef Marek Szyprowski 
<m.szyprowski@samsung.com>:

> 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. This happens because sdiodev->bus is set 2 times:
> first in brcmf_sdio_probe() and second time in brcmf_sdiod_probe(). Fix
> this by chaning the brcmf_sdio_probe() function to return the error code
> and set sdio->bus only there.
>
> Fixes: 0ff0843310b7 ("wifi: brcmfmac: Add optional lpo clock enable support")

Acked-by: Arend van Spriel<arend.vanspriel@broadcom.com>

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> Changelog:
> v2:
> - changed return type of brcmf_sdio_probe() function
>
> v1: 
> https://lore.kernel.org/all/20251231143544.4158840-1-m.szyprowski@samsung.com
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 7 +++----
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c   | 7 ++++---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h   | 2 +-
> 3 files changed, 8 insertions(+), 8 deletions(-)