[PATCH wireless-next 0/2] wifi: wfx: fix two minor issues in error paths

[PATCH wireless-next 0/2] wifi: wfx: fix two minor issues in error paths

[Jérôme Pouiller]

Both patches of this series fix minor issues located in error paths of
the wfx driver. Neither of them is likely to be hit in practice and both
have been there for years (since 2020).

They were found during the review of the series "wifi: wfx: fix possible
device hang during init" [1], which reworks the error path of
wfx_probe(): the first one by the Sashiko review bot, the second one by
Alexander Sverdlin.

The first patch cancels a delayed work that could fire after the device
has been removed and dereference the freed struct wfx_dev. The second one
makes wfx_probe() return a proper errno instead of a leftover positive
value when the firmware enforces the 'secure link' feature.

Note that AI tools have been involved in both patches. See the individual
changelogs for the details.

This series applies on top of [1].

[1]: https://lore.kernel.org/all/20260901083548.113131-1-jerome.pouiller@silabs.com/

Jérôme Pouiller (2):
  wifi: wfx: fix use-after-free of the cooling work on device removal
  wifi: wfx: fix error code on unsupported firmware

 drivers/net/wireless/silabs/wfx/main.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.47.3

[PATCH wireless-next 1/2] wifi: wfx: fix use-after-free of the cooling work on device removal

[Jérôme Pouiller]

When the device reports that it is too hot, wfx_suspend_hot_dev() blocks
the Tx queues and arms cooling_timeout_work with a 10s delay. If the
device recovers a normal temperature before the delay expires, the work
is canceled. Else, the work declares the chip frozen and unblocks the Tx
queues.

However, this work is never canceled when the device is removed.
cooling_timeout_work is queued on the system workqueue, while struct
wfx_dev is released by wfx_free_common() (through ieee80211_free_hw()).
So, if the device is unbound during this 10s window, the work fires
after struct wfx_dev has been freed and dereferences it.

Cancel the work during the teardown. It has to be done after
wfx_bh_unregister(): the "device too hot" indication is processed by the
bh, so canceling the work earlier would allow the bh to rearm it. On the
other hand, the work calls wfx_tx_unlock(), which may in turn call
wfx_bh_request_tx(). So it has to be canceled before bh_wq is destroyed.

Note that the Tx queues are intentionally left blocked: the device is
going away, so there is nothing to unblock.

The issue was reported by the Sashiko review bot and the fix has been
written by Copilot (including the commit log). The use-case is difficult
to reproduce, so this code has not been tested. However, I don't believe
this patch could cause any regression.

Fixes: 1d52d29983e5d ("staging: wfx: add support for 'device too hot' indication")
Assisted-by: Sashiko:gemini-3.1-pro-preview
Assisted-by: Copilot:claude-opus-5
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index 4e99fe7e5bb78..b6fd9d4c1f5fc 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -487,6 +487,7 @@ int wfx_probe(struct wfx_dev *wdev)
 		wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
 bh_unregister:
 	wfx_bh_unregister(wdev);
+	cancel_delayed_work_sync(&wdev->cooling_timeout_work);
 	destroy_workqueue(wdev->bh_wq);
 	return err;
 }
@@ -497,6 +498,7 @@ void wfx_release(struct wfx_dev *wdev)
 	wfx_hif_shutdown(wdev);
 	wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
 	wfx_bh_unregister(wdev);
+	cancel_delayed_work_sync(&wdev->cooling_timeout_work);
 	destroy_workqueue(wdev->bh_wq);
 }
 
-- 
2.47.3

[PATCH wireless-next 2/2] wifi: wfx: fix error code on unsupported firmware

[Jérôme Pouiller]

The 'secure link' feature is not supported by upstream (the reference
code relies on mbedTLS to implement it, which is not an option for the
Linux kernel).

If the firmware enforce the 'secure link' feature, wfx_probe() exits
early. However, err still holds the value returned by the previous call
to wait_for_completion_timeout(), which is the number of jiffies left
before the timeout, hence strictly positive (the zero case is handled
just above).

wfx_probe() therefore returns a positive value. The bus probe functions
forward it as-is and the driver core negates it (see really_probe()).
The device does not get bound, which is the expected outcome, but the
reported error code is a meaningless negative jiffies count instead of
an errno.

Return -EOPNOTSUPP, as already done a few lines above when the firmware
API version is not supported.

Copilot reviewed the code and wrote some parts of the log.

Reported-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Closes: https://lore.kernel.org/all/cc4de4e40e3ca50e7288be2b9067802576567c25.camel@siemens.com/
Assisted-by: Copilot:claude-opus-5
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index b6fd9d4c1f5fc..16d5b51f6f03a 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -411,6 +411,7 @@ int wfx_probe(struct wfx_dev *wdev)
 
 	if (wdev->hw_caps.link_mode == SEC_LINK_ENFORCED) {
 		dev_err(wdev->dev, "chip require secure_link, but can't negotiate it\n");
+		err = -EOPNOTSUPP;
 		goto irq_unsubscribe;
 	}
 
-- 
2.47.3

Re: [PATCH wireless-next 2/2] wifi: wfx: fix error code on unsupported firmware

[Sverdlin, Alexander]

Hi Jérôme,

On Sun, 2026-09-06 at 22:47 +0200, Jérôme Pouiller wrote:
> The 'secure link' feature is not supported by upstream (the reference
> code relies on mbedTLS to implement it, which is not an option for the
> Linux kernel).
> 
> If the firmware enforce the 'secure link' feature, wfx_probe() exits
> early. However, err still holds the value returned by the previous call
> to wait_for_completion_timeout(), which is the number of jiffies left
> before the timeout, hence strictly positive (the zero case is handled
> just above).
> 
> wfx_probe() therefore returns a positive value. The bus probe functions
> forward it as-is and the driver core negates it (see really_probe()).
> The device does not get bound, which is the expected outcome, but the
> reported error code is a meaningless negative jiffies count instead of
                                       ^^^^^^^^
minor: I suppose it should have read "positive"

> an errno.
> 
> Return -EOPNOTSUPP, as already done a few lines above when the firmware
> API version is not supported.
> 
> Copilot reviewed the code and wrote some parts of the log.
> 
> Reported-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> Closes: https://lore.kernel.org/all/cc4de4e40e3ca50e7288be2b9067802576567c25.camel@siemens.com/
> Assisted-by: Copilot:claude-opus-5
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
>  drivers/net/wireless/silabs/wfx/main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> index b6fd9d4c1f5fc..16d5b51f6f03a 100644
> --- a/drivers/net/wireless/silabs/wfx/main.c
> +++ b/drivers/net/wireless/silabs/wfx/main.c
> @@ -411,6 +411,7 @@ int wfx_probe(struct wfx_dev *wdev)
>  
>  	if (wdev->hw_caps.link_mode == SEC_LINK_ENFORCED) {
>  		dev_err(wdev->dev, "chip require secure_link, but can't negotiate it\n");
> +		err = -EOPNOTSUPP;
>  		goto irq_unsubscribe;
>  	}

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH wireless-next 2/2] wifi: wfx: fix error code on unsupported firmware

[Jérôme Pouiller]

On Monday 7 September 2026 08:20:54 Central European Summer Time Sverdlin, Alexander wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> Hi Jérôme,
> 
> On Sun, 2026-09-06 at 22:47 +0200, Jérôme Pouiller wrote:
> > The 'secure link' feature is not supported by upstream (the reference
> > code relies on mbedTLS to implement it, which is not an option for the
> > Linux kernel).
> >
> > If the firmware enforce the 'secure link' feature, wfx_probe() exits
> > early. However, err still holds the value returned by the previous call
> > to wait_for_completion_timeout(), which is the number of jiffies left
> > before the timeout, hence strictly positive (the zero case is handled
> > just above).
> >
> > wfx_probe() therefore returns a positive value. The bus probe functions
> > forward it as-is and the driver core negates it (see really_probe()).
> > The device does not get bound, which is the expected outcome, but the
> > reported error code is a meaningless negative jiffies count instead of
>                                        ^^^^^^^^
> minor: I suppose it should have read "positive"

I mean, really_probe() inverts the value:

    /*
     * Return probe errors as positive values so that the callers
     * can distinguish them from other errors.
     */
    ret = -ret;
    goto probe_failed;

So, the jiffies count become negative

-- 
Jérôme Pouiller

Re: [PATCH wireless-next 2/2] wifi: wfx: fix error code on unsupported firmware

[Sverdlin, Alexander]

Hi Jérôme,

On Mon, 2026-09-07 at 08:59 +0200, Jérôme Pouiller wrote:
> > > The 'secure link' feature is not supported by upstream (the reference
> > > code relies on mbedTLS to implement it, which is not an option for the
> > > Linux kernel).
> > > 
> > > If the firmware enforce the 'secure link' feature, wfx_probe() exits
> > > early. However, err still holds the value returned by the previous call
> > > to wait_for_completion_timeout(), which is the number of jiffies left
> > > before the timeout, hence strictly positive (the zero case is handled
> > > just above).
> > > 
> > > wfx_probe() therefore returns a positive value. The bus probe functions
> > > forward it as-is and the driver core negates it (see really_probe()).
> > > The device does not get bound, which is the expected outcome, but the
> > > reported error code is a meaningless negative jiffies count instead of
> >                                         ^^^^^^^^
> > minor: I suppose it should have read "positive"
> 
> I mean, really_probe() inverts the value:
> 
>     /*
>      * Return probe errors as positive values so that the callers
>      * can distinguish them from other errors.
>      */
>     ret = -ret;
>     goto probe_failed;
> 
> So, the jiffies count become negative

you are right, I didn't realize the above explains the driver core ;-)
Thanks for explanation and sorry for the noise!

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH wireless-next 1/2] wifi: wfx: fix use-after-free of the cooling work on device removal

[Sverdlin, Alexander]

Hi Jérôme,

On Sun, 2026-09-06 at 22:47 +0200, Jérôme Pouiller wrote:
> When the device reports that it is too hot, wfx_suspend_hot_dev() blocks
> the Tx queues and arms cooling_timeout_work with a 10s delay. If the
> device recovers a normal temperature before the delay expires, the work
> is canceled. Else, the work declares the chip frozen and unblocks the Tx
> queues.
> 
> However, this work is never canceled when the device is removed.
> cooling_timeout_work is queued on the system workqueue, while struct
> wfx_dev is released by wfx_free_common() (through ieee80211_free_hw()).
> So, if the device is unbound during this 10s window, the work fires
> after struct wfx_dev has been freed and dereferences it.
> 
> Cancel the work during the teardown. It has to be done after
> wfx_bh_unregister(): the "device too hot" indication is processed by the
> bh, so canceling the work earlier would allow the bh to rearm it. On the
> other hand, the work calls wfx_tx_unlock(), which may in turn call
> wfx_bh_request_tx(). So it has to be canceled before bh_wq is destroyed.
> 
> Note that the Tx queues are intentionally left blocked: the device is
> going away, so there is nothing to unblock.
> 
> The issue was reported by the Sashiko review bot and the fix has been
> written by Copilot (including the commit log). The use-case is difficult
> to reproduce, so this code has not been tested. However, I don't believe
> this patch could cause any regression.
> 
> Fixes: 1d52d29983e5d ("staging: wfx: add support for 'device too hot' indication")
> Assisted-by: Sashiko:gemini-3.1-pro-preview
> Assisted-by: Copilot:claude-opus-5
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>

thanks for the patch! Tested with rmmod, no regression visible,

Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
>  drivers/net/wireless/silabs/wfx/main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> index 4e99fe7e5bb78..b6fd9d4c1f5fc 100644
> --- a/drivers/net/wireless/silabs/wfx/main.c
> +++ b/drivers/net/wireless/silabs/wfx/main.c
> @@ -487,6 +487,7 @@ int wfx_probe(struct wfx_dev *wdev)
>  		wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
>  bh_unregister:
>  	wfx_bh_unregister(wdev);
> +	cancel_delayed_work_sync(&wdev->cooling_timeout_work);
>  	destroy_workqueue(wdev->bh_wq);
>  	return err;
>  }
> @@ -497,6 +498,7 @@ void wfx_release(struct wfx_dev *wdev)
>  	wfx_hif_shutdown(wdev);
>  	wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
>  	wfx_bh_unregister(wdev);
> +	cancel_delayed_work_sync(&wdev->cooling_timeout_work);
>  	destroy_workqueue(wdev->bh_wq);
>  }

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com