public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 13/16] carl9170: rx: gate data frame delivery on STARTED state
@ 2026-03-17 11:06 Masi Osmani
  2026-03-17 11:12 ` Johannes Berg
  2026-03-21 22:34 ` Christian Lamparter
  0 siblings, 2 replies; 4+ messages in thread
From: Masi Osmani @ 2026-03-17 11:06 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless, Masi Osmani

Do not deliver data frames to mac80211 unless the device is fully
started.  After carl9170_op_stop() the driver state drops to IDLE,
but the USB RX path can still receive frames from the hardware.
Without this gate, ieee80211_rx() may reference station data that
sta_info_destroy_part2() is concurrently freeing during interface
teardown, causing a use-after-free kernel panic.

The race occurs when ieee80211_handle_reconfig_failure() clears
IN_DRIVER flags without stopping the hardware: cfg80211 then tears
down interfaces via ieee80211_do_stop() which calls sta_info_flush()
while the driver's RX path still delivers frames.  This was observed
when carl9170 firmware deadlocks during a restart attempt and
ieee80211_reconfig() fails at drv_add_interface().

The gate is placed in carl9170_rx_untie_data() just before the
ieee80211_rx() call, not in the USB layer, because firmware command
responses (including CARL9170_RSP_BOOT needed for firmware upload)
must still flow through the shared RX path via
carl9170_handle_command_response() which returns before reaching
this point.

Signed-off-by: Masi Osmani <mas-i@hotmail.de>
---
 drivers/net/wireless/ath/carl9170/rx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
index 683343013..19c6bd418 100644
--- a/drivers/net/wireless/ath/carl9170/rx.c
+++ b/drivers/net/wireless/ath/carl9170/rx.c
@@ -676,6 +676,14 @@ static int carl9170_handle_mpdu(struct ar9170 *ar, u8 *buf, int len,
 
 	carl9170_ba_check(ar, buf, len);
 
+	/*
+	 * Do not deliver data frames to mac80211 unless the device is
+	 * fully started.  After carl9170_op_stop() the state drops to
+	 * IDLE, preventing a use-after-free when sta_info_destroy_part2()
+	 * races with ieee80211_rx() during interface teardown.
+	 */
+	if (!IS_STARTED(ar))
+		return 0;
 	skb = carl9170_rx_copy_data(buf, len);
 	if (!skb)
 		return -ENOMEM;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 13/16] carl9170: rx: gate data frame delivery on STARTED state
  2026-03-17 11:06 [PATCH 13/16] carl9170: rx: gate data frame delivery on STARTED state Masi Osmani
@ 2026-03-17 11:12 ` Johannes Berg
  2026-03-21 22:34 ` Christian Lamparter
  1 sibling, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2026-03-17 11:12 UTC (permalink / raw)
  To: Masi Osmani, Christian Lamparter; +Cc: linux-wireless

On Tue, 2026-03-17 at 12:06 +0100, Masi Osmani wrote:
> Do not deliver data frames to mac80211 unless the device is fully
> started.  After carl9170_op_stop() the driver state drops to IDLE,
> but the USB RX path can still receive frames from the hardware.
> Without this gate, ieee80211_rx() may reference station data that
> sta_info_destroy_part2() is concurrently freeing during interface
> teardown, causing a use-after-free kernel panic.

You keep claiming this without ever showing how it happened, that's not
so useful ... I really don't think that should happen given there's RCU
protection involved everywhere. Please show what happens so we can fix
_that_ issue instead of papering over it.

johannes

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 13/16] carl9170: rx: gate data frame delivery on STARTED state
  2026-03-17 11:06 [PATCH 13/16] carl9170: rx: gate data frame delivery on STARTED state Masi Osmani
  2026-03-17 11:12 ` Johannes Berg
@ 2026-03-21 22:34 ` Christian Lamparter
  2026-03-21 22:37   ` Johannes Berg
  1 sibling, 1 reply; 4+ messages in thread
From: Christian Lamparter @ 2026-03-21 22:34 UTC (permalink / raw)
  To: Masi Osmani; +Cc: linux-wireless

On 3/17/26 12:06 PM, Masi Osmani wrote:
> Do not deliver data frames to mac80211 unless the device is fully
> started.  After carl9170_op_stop() the driver state drops to IDLE,
> but the USB RX path can still receive frames from the hardware.
> Without this gate, ieee80211_rx() may reference station data that
> sta_info_destroy_part2() is concurrently freeing during interface
> teardown, causing a use-after-free kernel panic.
> 
> The race occurs when ieee80211_handle_reconfig_failure() clears
> IN_DRIVER flags without stopping the hardware: cfg80211 then tears
> down interfaces via ieee80211_do_stop() which calls sta_info_flush()
> while the driver's RX path still delivers frames.  This was observed
> when carl9170 firmware deadlocks during a restart attempt and
> ieee80211_reconfig() fails at drv_add_interface().
> 
> The gate is placed in carl9170_rx_untie_data() just before the
> ieee80211_rx() call, not in the USB layer, because firmware command
> responses (including CARL9170_RSP_BOOT needed for firmware upload)
> must still flow through the shared RX path via
> carl9170_handle_command_response() which returns before reaching
> this point.
> 
> Signed-off-by: Masi Osmani <mas-i@hotmail.de>
> ---
>   drivers/net/wireless/ath/carl9170/rx.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
> index 683343013..19c6bd418 100644
> --- a/drivers/net/wireless/ath/carl9170/rx.c
> +++ b/drivers/net/wireless/ath/carl9170/rx.c
> @@ -676,6 +676,14 @@ static int carl9170_handle_mpdu(struct ar9170 *ar, u8 *buf, int len,
>   
>   	carl9170_ba_check(ar, buf, len);
>   
> +	/*
> +	 * Do not deliver data frames to mac80211 unless the device is
> +	 * fully started.  After carl9170_op_stop() the state drops to
> +	 * IDLE, preventing a use-after-free when sta_info_destroy_part2()
> +	 * races with ieee80211_rx() during interface teardown.
> +	 */

If what you write is true for an up-to-date kernel, this needs to be addressed in mac80211.
Under no circumstance should mac80211 behave that way... for any driver, in any case.

Can you please post the panics/errors/warnings?

> +	if (!IS_STARTED(ar))
> +		return 0;
>   	skb = carl9170_rx_copy_data(buf, len);
>   	if (!skb)
>   		return -ENOMEM;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 13/16] carl9170: rx: gate data frame delivery on STARTED state
  2026-03-21 22:34 ` Christian Lamparter
@ 2026-03-21 22:37   ` Johannes Berg
  0 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2026-03-21 22:37 UTC (permalink / raw)
  To: Christian Lamparter, Masi Osmani; +Cc: linux-wireless

On Sat, 2026-03-21 at 23:34 +0100, Christian Lamparter wrote:
> On 3/17/26 12:06 PM, Masi Osmani wrote:
> > Do not deliver data frames to mac80211 unless the device is fully
> > started.  After carl9170_op_stop() the driver state drops to IDLE,
> > but the USB RX path can still receive frames from the hardware.
> > Without this gate, ieee80211_rx() may reference station data that
> > sta_info_destroy_part2() is concurrently freeing during interface
> > teardown, causing a use-after-free kernel panic.
> > 
> > The race occurs when ieee80211_handle_reconfig_failure() clears
> > IN_DRIVER flags without stopping the hardware: cfg80211 then tears
> > down interfaces via ieee80211_do_stop() which calls sta_info_flush()
> > while the driver's RX path still delivers frames.  This was observed
> > when carl9170 firmware deadlocks during a restart attempt and
> > ieee80211_reconfig() fails at drv_add_interface().
> > 
> > The gate is placed in carl9170_rx_untie_data() just before the
> > ieee80211_rx() call, not in the USB layer, because firmware command
> > responses (including CARL9170_RSP_BOOT needed for firmware upload)
> > must still flow through the shared RX path via
> > carl9170_handle_command_response() which returns before reaching
> > this point.
> > 
> > Signed-off-by: Masi Osmani <mas-i@hotmail.de>
> > ---
> >   drivers/net/wireless/ath/carl9170/rx.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
> > index 683343013..19c6bd418 100644
> > --- a/drivers/net/wireless/ath/carl9170/rx.c
> > +++ b/drivers/net/wireless/ath/carl9170/rx.c
> > @@ -676,6 +676,14 @@ static int carl9170_handle_mpdu(struct ar9170 *ar, u8 *buf, int len,
> >   
> >   	carl9170_ba_check(ar, buf, len);
> >   
> > +	/*
> > +	 * Do not deliver data frames to mac80211 unless the device is
> > +	 * fully started.  After carl9170_op_stop() the state drops to
> > +	 * IDLE, preventing a use-after-free when sta_info_destroy_part2()
> > +	 * races with ieee80211_rx() during interface teardown.
> > +	 */
> 
> If what you write is true for an up-to-date kernel, this needs to be addressed in mac80211.
> Under no circumstance should mac80211 behave that way... for any driver, in any case.
> 
> Can you please post the panics/errors/warnings?

Yeah, this whole use-after-free is super fishy since we have RCU for it
all. Masi proposed a patch to mac80211 with similar strange reasoning
which I rejected, and never provided the errors that were happening ...

johannes

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-21 22:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17 11:06 [PATCH 13/16] carl9170: rx: gate data frame delivery on STARTED state Masi Osmani
2026-03-17 11:12 ` Johannes Berg
2026-03-21 22:34 ` Christian Lamparter
2026-03-21 22:37   ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox