[PATCH 5.15.y] wifi: brcmfmac: fix use-after-free when rescheduling brcmf_btcoex_info work

[PATCH 5.15.y] wifi: brcmfmac: fix use-after-free when rescheduling brcmf_btcoex_info work

[Robert Garcia]

From: Duoming Zhou <duoming@zju.edu.cn>

[ Upstream commit 9cb83d4be0b9b697eae93d321e0da999f9cdfcfc ]

The brcmf_btcoex_detach() only shuts down the btcoex timer, if the
flag timer_on is false. However, the brcmf_btcoex_timerfunc(), which
runs as timer handler, sets timer_on to false. This creates critical
race conditions:

1.If brcmf_btcoex_detach() is called while brcmf_btcoex_timerfunc()
is executing, it may observe timer_on as false and skip the call to
timer_shutdown_sync().

2.The brcmf_btcoex_timerfunc() may then reschedule the brcmf_btcoex_info
worker after the cancel_work_sync() has been executed, resulting in
use-after-free bugs.

The use-after-free bugs occur in two distinct scenarios, depending on
the timing of when the brcmf_btcoex_info struct is freed relative to
the execution of its worker thread.

Scenario 1: Freed before the worker is scheduled

The brcmf_btcoex_info is deallocated before the worker is scheduled.
A race condition can occur when schedule_work(&bt_local->work) is
called after the target memory has been freed. The sequence of events
is detailed below:

CPU0                           | CPU1
brcmf_btcoex_detach            | brcmf_btcoex_timerfunc
                               |   bt_local->timer_on = false;
  if (cfg->btcoex->timer_on)   |
    ...                        |
  cancel_work_sync();          |
  ...                          |
  kfree(cfg->btcoex); // FREE  |
                               |   schedule_work(&bt_local->work); // USE

Scenario 2: Freed after the worker is scheduled

The brcmf_btcoex_info is freed after the worker has been scheduled
but before or during its execution. In this case, statements within
the brcmf_btcoex_handler() — such as the container_of macro and
subsequent dereferences of the brcmf_btcoex_info object will cause
a use-after-free access. The following timeline illustrates this
scenario:

CPU0                            | CPU1
brcmf_btcoex_detach             | brcmf_btcoex_timerfunc
                                |   bt_local->timer_on = false;
  if (cfg->btcoex->timer_on)    |
    ...                         |
  cancel_work_sync();           |
  ...                           |   schedule_work(); // Reschedule
                                |
  kfree(cfg->btcoex); // FREE   |   brcmf_btcoex_handler() // Worker
  /*                            |     btci = container_of(....); // USE
   The kfree() above could      |     ...
   also occur at any point      |     btci-> // USE
   during the worker's execution|
   */                           |

To resolve the race conditions, drop the conditional check and call
timer_shutdown_sync() directly. It can deactivate the timer reliably,
regardless of its current state. Once stopped, the timer_on state is
then set to false.

Fixes: 61730d4dfffc ("brcmfmac: support critical protocol API for DHCP")
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
Link: https://patch.msgid.link/20250822050839.4413-1-duoming@zju.edu.cn
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Robert Garcia <rob_garcia@163.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c
index f9f18ff451ea..f46e40900217 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c
@@ -392,10 +392,8 @@ void brcmf_btcoex_detach(struct brcmf_cfg80211_info *cfg)
 	if (!cfg->btcoex)
 		return;
 
-	if (cfg->btcoex->timer_on) {
-		cfg->btcoex->timer_on = false;
-		del_timer_sync(&cfg->btcoex->timer);
-	}
+	del_timer_sync(&cfg->btcoex->timer);
+	cfg->btcoex->timer_on = false;
 
 	cancel_work_sync(&cfg->btcoex->work);
 
-- 
2.34.1

Re: [PATCH 5.15.y] wifi: brcmfmac: fix use-after-free when rescheduling brcmf_btcoex_info work

[Sasha Levin]

Queued for 5.15.y, thanks.

-- 
Thanks,
Sasha

Re: [PATCH 5.15.y] wifi: brcmfmac: fix use-after-free when rescheduling brcmf_btcoex_info work

[Arend van Spriel]

On Thu, 28 May 2026 13:54:31 +0800, Robert Garcia wrote:
> From: Duoming Zhou <duoming@zju.edu.cn>
>
> [ Upstream commit 9cb83d4be0b9b697eae93d321e0da999f9cdfcfc ]

[...]

> +	del_timer_sync(&cfg->btcoex->timer);
> +	cfg->btcoex->timer_on = false;
>
>  	cancel_work_sync(&cfg->btcoex->work);

The upstream fix uses timer_shutdown_sync() which prevents the timer from
being re-armed after it returns. del_timer_sync() does not have this
guarantee.

brcmf_btcoex_handler() has three mod_timer() calls - two in
BRCMF_BT_DHCP_START and one in BRCMF_BT_DHCP_OPPR_WIN - none of which are
guarded by timer_on. A work item running between del_timer_sync() and
cancel_work_sync() can re-arm the timer, leaving it pending when kfree() is
called.

If the mod_timer() calls were made conditional on timer_on, setting
timer_on = false before del_timer_sync() would be sufficient. That would be
my preferred fix. Alternatively a second del_timer_sync() after
cancel_work_sync() would also work.

Also the commit message still mentions timer_shutdown_sync() rather than
del_timer_sync().

Regards,
Arend

Re: [PATCH 5.15.y] wifi: brcmfmac: fix use-after-free when rescheduling brcmf_btcoex_info work

[Sasha Levin]

> The upstream fix uses timer_shutdown_sync() which prevents the timer from
> being re-armed after it returns. del_timer_sync() does not have this
> guarantee.
>
> brcmf_btcoex_handler() has three mod_timer() calls - two in
> BRCMF_BT_DHCP_START and one in BRCMF_BT_DHCP_OPPR_WIN - none of which are
> guarded by timer_on. A work item running between del_timer_sync() and
> cancel_work_sync() can re-arm the timer, leaving it pending when kfree() is
> called.

Agreed on the analysis. One thing to note: this backport already
shipped in v5.15.210, and 6.1.y shipped the same weakened
del_timer_sync() variant in v6.1.167, so this needs follow-up patches
rather than a v2 of the original.

> If the mod_timer() calls were made conditional on timer_on, setting
> timer_on = false before del_timer_sync() would be sufficient. That would be
> my preferred fix. Alternatively a second del_timer_sync() after
> cancel_work_sync() would also work.
>
> Also the commit message still mentions timer_shutdown_sync() rather than
> del_timer_sync().

There's a simpler option: timer_shutdown_sync() is actually available
in both 5.15.y and 6.1.y, so the del_timer_sync() substitution was
never necessary in the first place. A one-line follow-up converting
del_timer_sync() to timer_shutdown_sync() makes brcmf_btcoex_detach()
match the upstream post-image exactly, closes the re-arm window you
describe, and avoids a stable-only divergence guarding the mod_timer()
calls.

Robert, could you send follow-up patches for 5.15.y and 6.1.y doing
that conversion?

-- 
Thanks,
Sasha