mbox series

[0/2] Fixes packet processes after vif is stopped

Message ID cover.1741950009.git.repk@triplefau.lt
Headers show
Series Fixes packet processes after vif is stopped | expand

Message

Remi Pommarel March 14, 2025, 11:04 a.m. UTC 
Those are a couple of fixes that prevent crashes due to processing
packets (especially multicast ones) for TX after vif is stopped (either
after a mesh interface left the group or interface is put down).

The first one ensure the key info passed to drivers through ieee80211
skb control block is up to date, even after key removal.

The second one ensure no packets get processed after vif driver private
data is cleared in ieee80211_do_stop().

As I tried to explain in second patch footnote, I can still see a
theoretical reason that packets get queued after ieee80211_do_stop()
call. But I was not able to reproduce it, so I may be missing a
something here; making that more as an open question.

Remi Pommarel (2):
  wifi: mac80211: Update skb's NULL key in ieee80211_tx_h_select_key()
  wifi: mac80211: Purge vif txq in ieee80211_do_stop()

 net/mac80211/iface.c | 3 +++
 net/mac80211/tx.c    | 6 ++++++
 2 files changed, 9 insertions(+)

Comments

Remi Pommarel March 16, 2025, 7:47 p.m. UTC | #1 
On Fri, Mar 14, 2025 at 12:04:23PM +0100, Remi Pommarel wrote:
> Those are a couple of fixes that prevent crashes due to processing
> packets (especially multicast ones) for TX after vif is stopped (either
> after a mesh interface left the group or interface is put down).
> 
> The first one ensure the key info passed to drivers through ieee80211
> skb control block is up to date, even after key removal.
> 
> The second one ensure no packets get processed after vif driver private
> data is cleared in ieee80211_do_stop().
> 
> As I tried to explain in second patch footnote, I can still see a
> theoretical reason that packets get queued after ieee80211_do_stop()
> call. But I was not able to reproduce it, so I may be missing a
> something here; making that more as an open question.

And I forgot to include the footnote in Patch 2/2. I was worried that
because the rcu_read_lock() in __ieee80211_subif_start_xmit() is taken
only after the sdata running state it could create a small window during
which a packet could still be enqueued passed the synchronize_rcu() of
ieee80211_do_stop(). But after digging a bit more, it seems that
all __ieee80211_subif_start_xmit() callers (e.g. __dev_queue_xmit())
take the rcu_read_lock() already. So please ignore this last remark.

Regards,