Fix packets processed after vif is stopped

PATCH In review Remi Pommarel 4 participants 13 messages Mar 24, 2025 – Jul 17, 2025

Subsystems (3)

Arm MediaTek mac80211 MediaTek MT76 wireless LAN driver

lore

Share & tools

lore

Remi Pommarel

author

2025-03-24 16:34 UTC #0 lore raw

v2 cover Fix packets processed after vif is stopped

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().

v2:
- Add Fixes tag
- Clear SKB's control block key before in ieee80211_tx_dequeue()

    instead of ieee80211_tx_h_select_key()
  - Add wireless tag and fix type in subject
  - Because subject changed, v1 can be found at
    https://lore.kernel.org/lkml/[email protected]/

Remi Pommarel (2):
wifi: mac80211: Update skb's control block key in

    ieee80211_tx_dequeue()

wifi: mac80211: Purge vif txq in ieee80211_do_stop()

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

Signature · 1 line

2.40.0

Remi Pommarel

author

2025-03-24 16:34 UTC #1 lore raw

PATCH v2 1/2 wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

The ieee80211 skb control block key (set when skb was queued) could have been removed before ieee80211_tx_dequeue() call. ieee80211_tx_dequeue() already called ieee80211_tx_h_select_key() to get the current key, but the latter do not update the key in skb control block in case it is NULL. Because some drivers actually use this key in their TX callbacks (e.g. ath1{1,2}k_mac_op_tx()) this could lead to the use after free below:

  BUG: KASAN: slab-use-after-free in ath11k_mac_op_tx+0x590/0x61c
  Read of size 4 at addr ffffff803083c248 by task kworker/u16:4/1440

  CPU: 3 UID: 0 PID: 1440 Comm: kworker/u16:4 Not tainted 6.13.0-ge128f627f404 #2
  Hardware name: HW (DT)
  Workqueue: bat_events batadv_send_outstanding_bcast_packet
  Call trace:
   show_stack+0x14/0x1c (C)
   dump_stack_lvl+0x58/0x74
   print_report+0x164/0x4c0
   kasan_report+0xac/0xe8
   __asan_report_load4_noabort+0x1c/0x24
   ath11k_mac_op_tx+0x590/0x61c
   ieee80211_handle_wake_tx_queue+0x12c/0x1c8
   ieee80211_queue_skb+0xdcc/0x1b4c
   ieee80211_tx+0x1ec/0x2bc
   ieee80211_xmit+0x224/0x324
   __ieee80211_subif_start_xmit+0x85c/0xcf8
   ieee80211_subif_start_xmit+0xc0/0xec4
   dev_hard_start_xmit+0xf4/0x28c
   __dev_queue_xmit+0x6ac/0x318c
   batadv_send_skb_packet+0x38c/0x4b0
   batadv_send_outstanding_bcast_packet+0x110/0x328
   process_one_work+0x578/0xc10
   worker_thread+0x4bc/0xc7c
   kthread+0x2f8/0x380
   ret_from_fork+0x10/0x20

  Allocated by task 1906:
   kasan_save_stack+0x28/0x4c
   kasan_save_track+0x1c/0x40
   kasan_save_alloc_info+0x3c/0x4c
   __kasan_kmalloc+0xac/0xb0
   __kmalloc_noprof+0x1b4/0x380
   ieee80211_key_alloc+0x3c/0xb64
   ieee80211_add_key+0x1b4/0x71c
   nl80211_new_key+0x2b4/0x5d8
   genl_family_rcv_msg_doit+0x198/0x240
  <...>

  Freed by task 1494:
   kasan_save_stack+0x28/0x4c
   kasan_save_track+0x1c/0x40
   kasan_save_free_info+0x48/0x94
   __kasan_slab_free+0x48/0x60
   kfree+0xc8/0x31c
   kfree_sensitive+0x70/0x80
   ieee80211_key_free_common+0x10c/0x174
   ieee80211_free_keys+0x188/0x46c
   ieee80211_stop_mesh+0x70/0x2cc
   ieee80211_leave_mesh+0x1c/0x60
   cfg80211_leave_mesh+0xe0/0x280
   cfg80211_leave+0x1e0/0x244
  <...>

Reset SKB control block key before calling ieee80211_tx_h_select_key() to avoid that.

Fixes bb42f2d13ffc ("mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue")

Signed-off-by Remi Pommarel <[email protected]>

---
 net/mac80211/tx.c | 1 +
 1 file changed, 1 insertion(+)

net/mac80211/tx.c +1 -0

index a24636bda679..0c6214f12ea3 100644
@@ -3893,6 +3893,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	 * The key can be removed while the packet was queued, so need to call
 	 * this here to get the current key.
 	 */
+	info->control.hw_key = NULL;
 	r = ieee80211_tx_h_select_key(&tx);
 	if (r != TX_CONTINUE) {
 		ieee80211_free_txskb(&local->hw, skb);

Signature · 1 line

2.40.0

Bert Karwatzki

2025-04-10 21:55 UTC #2 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410 my mt7921e Wi-Fi controller is no longer able to connect to a network. I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()").

Hardware:
04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz

This debugging patch reveals that the change causes key to be NULL in mt7921_tx_prepare_skb().

drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +1 -0

index 881812ba03ff..3b8552a1055c 100644
@@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;
        int id, pid;

So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?

net/mac80211/tx.c +17 -5

index 34f229a6eab0..2510e3533d13 100644
@@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		case WLAN_CIPHER_SUITE_WEP40:
 		case WLAN_CIPHER_SUITE_WEP104:
 		case WLAN_CIPHER_SUITE_TKIP:
-			if (!ieee80211_is_data_present(hdr->frame_control))
+			if (!ieee80211_is_data_present(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		case WLAN_CIPHER_SUITE_CCMP:
 		case WLAN_CIPHER_SUITE_CCMP_256:

@@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			if (!ieee80211_is_data_present(hdr->frame_control) &&
 			    !ieee80211_use_mfp(hdr->frame_control, tx->sta,
 					       tx->skb) &&
-			    !ieee80211_is_group_privacy_action(tx->skb))
+			    !ieee80211_is_group_privacy_action(tx->skb)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
-			else
+			} else {
 				skip_hw = (tx->key->conf.flags &
 					   IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
 					ieee80211_is_mgmt(hdr->frame_control);
+			}
 			break;
 		case WLAN_CIPHER_SUITE_AES_CMAC:
 		case WLAN_CIPHER_SUITE_BIP_CMAC_256:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_128:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_256:
-			if (!ieee80211_is_mgmt(hdr->frame_control))
+			if (!ieee80211_is_mgmt(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		}

@@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			     tx->skb->protocol != tx->sdata->control_port_protocol)
 			return TX_DROP;
 
+		printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
+				__func__, skip_hw, tx->key);
 		if (!skip_hw && tx->key &&
-		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
+		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 			info->control.hw_key = &tx->key->conf;
+			printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
+		}
 	} else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
 		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
 		return TX_DROP;

@@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	 * The key can be removed while the packet was queued, so need to call
 	 * this here to get the current key.
 	 */
+	printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
+			__func__, info->control.hw_key);
 	info->control.hw_key = NULL;
 	r = ieee80211_tx_h_select_key(&tx);
 	if (r != TX_CONTINUE) {

This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk) and so the updating of info->contro.hw_key is skipped:

[   17.411298] [   T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL
[   17.411300] [   T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
[   17.411307] [   T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000

If I revert commit a104042e2bf6 while keeping the debug patches it shows that the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e relies on the key your patch is setting to NULL.

Is this a problem with your patch or with the mt7921e driver that just got revealed by your patch?

Bert Karwatzki

Remi Pommarel

author

2025-04-11 10:13 UTC #3 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

Hi Bert,

Quoted 95 lines · Bert Karwatzki

Hardware:
04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz

This debugging patch reveals that the change causes key to be NULL in mt7921_tx_prepare_skb().

drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +1 -0

index 881812ba03ff..3b8552a1055c 100644
@@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;
        int id, pid;

So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?

net/mac80211/tx.c +17 -5

index 34f229a6eab0..2510e3533d13 100644
@@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		case WLAN_CIPHER_SUITE_WEP40:
 		case WLAN_CIPHER_SUITE_WEP104:
 		case WLAN_CIPHER_SUITE_TKIP:
-			if (!ieee80211_is_data_present(hdr->frame_control))
+			if (!ieee80211_is_data_present(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		case WLAN_CIPHER_SUITE_CCMP:
 		case WLAN_CIPHER_SUITE_CCMP_256:

@@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			if (!ieee80211_is_data_present(hdr->frame_control) &&
 			    !ieee80211_use_mfp(hdr->frame_control, tx->sta,
 					       tx->skb) &&
-			    !ieee80211_is_group_privacy_action(tx->skb))
+			    !ieee80211_is_group_privacy_action(tx->skb)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
-			else
+			} else {
 				skip_hw = (tx->key->conf.flags &
 					   IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
 					ieee80211_is_mgmt(hdr->frame_control);
+			}
 			break;
 		case WLAN_CIPHER_SUITE_AES_CMAC:
 		case WLAN_CIPHER_SUITE_BIP_CMAC_256:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_128:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_256:
-			if (!ieee80211_is_mgmt(hdr->frame_control))
+			if (!ieee80211_is_mgmt(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		}

@@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			     tx->skb->protocol != tx->sdata->control_port_protocol)
 			return TX_DROP;
 
+		printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
+				__func__, skip_hw, tx->key);
 		if (!skip_hw && tx->key &&
-		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
+		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 			info->control.hw_key = &tx->key->conf;
+			printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
+		}
 	} else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
 		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
 		return TX_DROP;

@@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	 * The key can be removed while the packet was queued, so need to call
 	 * this here to get the current key.
 	 */
+	printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
+			__func__, info->control.hw_key);
 	info->control.hw_key = NULL;
 	r = ieee80211_tx_h_select_key(&tx);
 	if (r != TX_CONTINUE) {

Quoted 4 lines

This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk) and so the updating of info->contro.hw_key is skipped:
[   17.411298] [   T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL

That means that we are trying to send non management frames using AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group management frames ?

Quoted 9 lines

[   17.411300] [   T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
[   17.411307] [   T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
If I revert commit a104042e2bf6 while keeping the debug patches it shows that the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e relies on the key your patch is setting to NULL.
Is this a problem with your patch or with the mt7921e driver that just got revealed by your patch?

Not sure yet, do you happen to know which kind of frame mt7921e is trying to be sent using this NULL key ?

Thanks,

Signature · 1 line

Remi

Bert Karwatzki

2025-04-11 11:10 UTC #4 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:

Quoted 114 lines

Hi Bert,

Quoted 95 lines · Bert Karwatzki

Hardware:
04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz

This debugging patch reveals that the change causes key to be NULL in mt7921_tx_prepare_skb().

drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +1 -0

index 881812ba03ff..3b8552a1055c 100644
@@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;
        int id, pid;

So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?

net/mac80211/tx.c +17 -5

index 34f229a6eab0..2510e3533d13 100644
@@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		case WLAN_CIPHER_SUITE_WEP40:
 		case WLAN_CIPHER_SUITE_WEP104:
 		case WLAN_CIPHER_SUITE_TKIP:
-			if (!ieee80211_is_data_present(hdr->frame_control))
+			if (!ieee80211_is_data_present(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		case WLAN_CIPHER_SUITE_CCMP:
 		case WLAN_CIPHER_SUITE_CCMP_256:

@@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			if (!ieee80211_is_data_present(hdr->frame_control) &&
 			    !ieee80211_use_mfp(hdr->frame_control, tx->sta,
 					       tx->skb) &&
-			    !ieee80211_is_group_privacy_action(tx->skb))
+			    !ieee80211_is_group_privacy_action(tx->skb)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
-			else
+			} else {
 				skip_hw = (tx->key->conf.flags &
 					   IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
 					ieee80211_is_mgmt(hdr->frame_control);
+			}
 			break;
 		case WLAN_CIPHER_SUITE_AES_CMAC:
 		case WLAN_CIPHER_SUITE_BIP_CMAC_256:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_128:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_256:
-			if (!ieee80211_is_mgmt(hdr->frame_control))
+			if (!ieee80211_is_mgmt(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		}

@@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			     tx->skb->protocol != tx->sdata->control_port_protocol)
 			return TX_DROP;
 
+		printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
+				__func__, skip_hw, tx->key);
 		if (!skip_hw && tx->key &&
-		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
+		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 			info->control.hw_key = &tx->key->conf;
+			printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
+		}
 	} else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
 		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
 		return TX_DROP;

@@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	 * The key can be removed while the packet was queued, so need to call
 	 * this here to get the current key.
 	 */
+	printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
+			__func__, info->control.hw_key);
 	info->control.hw_key = NULL;
 	r = ieee80211_tx_h_select_key(&tx);
 	if (r != TX_CONTINUE) {

Quoted 4 lines

This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk) and so the updating of info->contro.hw_key is skipped:
[   17.411298] [   T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL

That means that we are trying to send non management frames using AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group management frames ?

Quoted 7 lines

[   17.411300] [   T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
[   17.411307] [   T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
If I revert commit a104042e2bf6 while keeping the debug patches it shows that the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e relies on the key your patch is setting to NULL.

Quoted 7 lines

Quoted 2 lines
Is this a problem with your patch or with the mt7921e driver that just got revealed by your patch?
Not sure yet, do you happen to know which kind of frame mt7921e is trying to be sent using this NULL key ?
Thanks,

I modified my debugging patch to print mgmt->frame_control, if needed I could also insert a nore complicated function printing out frame types using the ieee80211_is_*() functions:

(quoted diff) +0 -0

b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c index 881812ba03ff..cfbe7e1e4713 100644

drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +0 -0

@@ -13,6 +13,9 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void

*txwi_ptr,

        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       struct ieee80211_mgmt *mgmt = (void *)tx_info->skb->data;
+       __le16 fc = mgmt->frame_control;
+       dev_info(mdev->dev, "%s: key = %px fc = 0x%hx\n", __func__, key, fc);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;
        int id, pid;

and get this, while unsuccesfully trying to connect (also note that one time getting a key worked):

$ dmesg | grep prepare_skb
[   11.775642] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   11.800047] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   13.365330] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   13.370257] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   16.468481] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   16.472407] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   16.542017] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   16.549581] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   16.597120] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   16.612263] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0

Here we actually go a key:

[   16.614478] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =

ffff89c275297230 fc = 0x4188

[   16.654273] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   16.698286] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   17.735855] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   17.837355] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   17.851029] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   18.613079] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   18.786202] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   21.027478] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   21.150212] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   21.843201] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   25.769981] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   29.776926] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   34.424966] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   44.945880] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   50.670382] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   62.054907] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xc0
[   65.042457] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   65.047387] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   65.391881] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   65.405189] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   65.445775] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   65.449289] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   65.479305] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
[   65.854396] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   65.878897] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   65.954047] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   65.961191] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   65.966296] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   65.977287] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
[   66.117317] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   66.626938] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   66.700300] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   66.818440] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   67.347089] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   67.448947] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   67.649713] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   68.535890] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   69.537434] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   69.697331] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   69.866666] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   70.500294] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   71.537535] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   74.615436] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   75.543129] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   77.741585] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   83.309171] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   83.550827] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   91.650611] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   99.564147] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   99.625088] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[  111.050345] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xc0
[  114.346190] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[  114.370450] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[  114.447350] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[  114.454710] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[  114.494605] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[  114.520269] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
[  114.522520] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
ffff89c2e7227230 fc = 0x4188
[  115.138564] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  115.394552] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  116.435866] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  116.494988] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[  116.612522] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  116.724047] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  117.459651] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  117.600286] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  118.816266] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[  118.961067] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  120.304632] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  121.614000] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  123.281756] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[  127.930929] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[  129.617550] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =

0000000000000000 fc = 0x3333

Bert Karwatzki

Remi Pommarel

author

2025-04-11 12:22 UTC #5 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

Quoted 107 lines · Bert Karwatzki

Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:

Quoted 106 lines

Hi Bert,

Quoted 95 lines · Bert Karwatzki

Hardware:
04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz

This debugging patch reveals that the change causes key to be NULL in mt7921_tx_prepare_skb().

drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +1 -0

index 881812ba03ff..3b8552a1055c 100644
@@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;
        int id, pid;

So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?

net/mac80211/tx.c +17 -5

index 34f229a6eab0..2510e3533d13 100644
@@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		case WLAN_CIPHER_SUITE_WEP40:
 		case WLAN_CIPHER_SUITE_WEP104:
 		case WLAN_CIPHER_SUITE_TKIP:
-			if (!ieee80211_is_data_present(hdr->frame_control))
+			if (!ieee80211_is_data_present(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		case WLAN_CIPHER_SUITE_CCMP:
 		case WLAN_CIPHER_SUITE_CCMP_256:

@@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			if (!ieee80211_is_data_present(hdr->frame_control) &&
 			    !ieee80211_use_mfp(hdr->frame_control, tx->sta,
 					       tx->skb) &&
-			    !ieee80211_is_group_privacy_action(tx->skb))
+			    !ieee80211_is_group_privacy_action(tx->skb)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
-			else
+			} else {
 				skip_hw = (tx->key->conf.flags &
 					   IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
 					ieee80211_is_mgmt(hdr->frame_control);
+			}
 			break;
 		case WLAN_CIPHER_SUITE_AES_CMAC:
 		case WLAN_CIPHER_SUITE_BIP_CMAC_256:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_128:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_256:
-			if (!ieee80211_is_mgmt(hdr->frame_control))
+			if (!ieee80211_is_mgmt(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		}

@@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			     tx->skb->protocol != tx->sdata->control_port_protocol)
 			return TX_DROP;
 
+		printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
+				__func__, skip_hw, tx->key);
 		if (!skip_hw && tx->key &&
-		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
+		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 			info->control.hw_key = &tx->key->conf;
+			printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
+		}
 	} else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
 		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
 		return TX_DROP;

@@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	 * The key can be removed while the packet was queued, so need to call
 	 * this here to get the current key.
 	 */
+	printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
+			__func__, info->control.hw_key);
 	info->control.hw_key = NULL;
 	r = ieee80211_tx_h_select_key(&tx);
 	if (r != TX_CONTINUE) {

Quoted 4 lines

This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk) and so the updating of info->contro.hw_key is skipped:
[   17.411298] [   T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL

That means that we are trying to send non management frames using AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group management frames ?

Thanks for the debug.

I completely miscalculated the line here sorry. In fact it means here that mt7921e needs a key for null data frames or for non protected management frames.

As a lot of your frame control dump does not make sense to me (e.g. 0xffff) maybe you could try to print the fc here at line 647. Just a wild guess maybe mt7921e needs MT_TXD3_PROTECT_FRAME for NULL data frames ?

Quoted 51 lines

Quoted 8 lines
Quoted 8 lines
[   17.411300] [   T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
[   17.411307] [   T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
If I revert commit a104042e2bf6 while keeping the debug patches it shows that the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e relies on the key your patch is setting to NULL.
Quoted 7 lines
Quoted 2 lines
Is this a problem with your patch or with the mt7921e driver that just got revealed by your patch?
Not sure yet, do you happen to know which kind of frame mt7921e is trying to be sent using this NULL key ?
Thanks,
I modified my debugging patch to print mgmt->frame_control, if needed I could also insert a nore complicated function printing out frame types using the ieee80211_is_*() functions:
drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +3 -0
index 881812ba03ff..cfbe7e1e4713 100644
@@ -13,6 +13,9 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void
 *txwi_ptr,
        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       struct ieee80211_mgmt *mgmt = (void *)tx_info->skb->data;
+       __le16 fc = mgmt->frame_control;
+       dev_info(mdev->dev, "%s: key = %px fc = 0x%hx\n", __func__, key, fc);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;
        int id, pid;
and get this, while unsuccesfully trying to connect (also note that one time getting a key worked):
$ dmesg | grep prepare_skb
[   11.775642] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   11.800047] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   13.365330] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   13.370257] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   16.468481] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0

Quoted 8 lines

[   16.472407] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   16.542017] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   16.549581] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   16.597120] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff

Thanks again for taking time to debug that.
As said above some of those fc does not make sense to me as I am not expected first two bits (Protocol version) to be non null.

Quoted 7 lines

[   16.612263] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0
Here we actually go a key:
[   16.614478] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
ffff89c275297230 fc = 0x4188
[...]

Signature · 1 line

Remi

Remi Pommarel

author

2025-07-17 13:46 UTC #6 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

In reply to Bert Karwatzki

Hello Bert, Johannes

Quoted 173 lines · Bert Karwatzki

Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:

Quoted 114 lines

Hi Bert,

Quoted 95 lines · Bert Karwatzki

Hardware:
04:00.0 Network controller: MEDIATEK Corp. MT7921K (RZ608) Wi-Fi 6E 80MHz

This debugging patch reveals that the change causes key to be NULL in mt7921_tx_prepare_skb().

drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +1 -0

index 881812ba03ff..3b8552a1055c 100644
@@ -13,6 +13,7 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       dev_info(mdev->dev, "%s: key = %px\n", __func__, key);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;
        int id, pid;

So why is info->control.hw_key not updated by ieee80211_tx_h_select_key()?

net/mac80211/tx.c +17 -5

index 34f229a6eab0..2510e3533d13 100644
@@ -631,8 +631,10 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		case WLAN_CIPHER_SUITE_WEP40:
 		case WLAN_CIPHER_SUITE_WEP104:
 		case WLAN_CIPHER_SUITE_TKIP:
-			if (!ieee80211_is_data_present(hdr->frame_control))
+			if (!ieee80211_is_data_present(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		case WLAN_CIPHER_SUITE_CCMP:
 		case WLAN_CIPHER_SUITE_CCMP_256:

@@ -641,19 +643,23 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			if (!ieee80211_is_data_present(hdr->frame_control) &&
 			    !ieee80211_use_mfp(hdr->frame_control, tx->sta,
 					       tx->skb) &&
-			    !ieee80211_is_group_privacy_action(tx->skb))
+			    !ieee80211_is_group_privacy_action(tx->skb)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
-			else
+			} else {
 				skip_hw = (tx->key->conf.flags &
 					   IEEE80211_KEY_FLAG_SW_MGMT_TX) &&
 					ieee80211_is_mgmt(hdr->frame_control);
+			}
 			break;
 		case WLAN_CIPHER_SUITE_AES_CMAC:
 		case WLAN_CIPHER_SUITE_BIP_CMAC_256:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_128:
 		case WLAN_CIPHER_SUITE_BIP_GMAC_256:
-			if (!ieee80211_is_mgmt(hdr->frame_control))
+			if (!ieee80211_is_mgmt(hdr->frame_control)) {
+				printk(KERN_INFO "%s %d: setting tx->key = NULL\n", __func__, __LINE__);
 				tx->key = NULL;
+			}
 			break;
 		}

@@ -662,9 +668,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 			     tx->skb->protocol != tx->sdata->control_port_protocol)
 			return TX_DROP;
 
+		printk(KERN_INFO "%s: skip_hw=%d tx->key=%px\n",
+				__func__, skip_hw, tx->key);
 		if (!skip_hw && tx->key &&
-		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
+		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 			info->control.hw_key = &tx->key->conf;
+			printk(KERN_INFO "%s: info->control.hw_key = %px\n", __func__, info->control.hw_key);
+		}
 	} else if (ieee80211_is_data_present(hdr->frame_control) && tx->sta &&
 		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
 		return TX_DROP;

@@ -3894,6 +3904,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	 * The key can be removed while the packet was queued, so need to call
 	 * this here to get the current key.
 	 */
+	printk(KERN_INFO "%s: info->control.hw_key = %px, setting to NULL\n",
+			__func__, info->control.hw_key);
 	info->control.hw_key = NULL;
 	r = ieee80211_tx_h_select_key(&tx);
 	if (r != TX_CONTINUE) {

Quoted 4 lines

This patch reveals that tx->key is set to NULL (in the @@ -641,19 +643,23 @@ chunk) and so the updating of info->contro.hw_key is skipped:
[   17.411298] [   T1232] ieee80211_tx_h_select_key 647: setting tx->key = NULL

That means that we are trying to send non management frames using AES_CMAC, or BIP_* cipher, aren't those ciphers used only for group management frames ?

Quoted 7 lines

[   17.411300] [   T1232] ieee80211_tx_h_select_key: skip_hw=0 tx->key=0000000000000000
[   17.411307] [   T1232] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key = 0000000000000000
If I revert commit a104042e2bf6 while keeping the debug patches it shows that the for mt7921e the key is never updated in ieee80211_tx_h_select_key(), mt7921e relies on the key your patch is setting to NULL.

Quoted 7 lines

Quoted 2 lines
Is this a problem with your patch or with the mt7921e driver that just got revealed by your patch?
Not sure yet, do you happen to know which kind of frame mt7921e is trying to be sent using this NULL key ?
Thanks,

I modified my debugging patch to print mgmt->frame_control, if needed I could also insert a nore complicated function printing out frame types using the ieee80211_is_*() functions:

drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c +3 -0

index 881812ba03ff..cfbe7e1e4713 100644
@@ -13,6 +13,9 @@ int mt7921e_tx_prepare_skb(struct mt76_dev *mdev, void
 *txwi_ptr,
        struct mt792x_dev *dev = container_of(mdev, struct mt792x_dev, mt76);
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
        struct ieee80211_key_conf *key = info->control.hw_key;
+       struct ieee80211_mgmt *mgmt = (void *)tx_info->skb->data;
+       __le16 fc = mgmt->frame_control;
+       dev_info(mdev->dev, "%s: key = %px fc = 0x%hx\n", __func__, key, fc);
        struct mt76_connac_hw_txp *txp;
        struct mt76_txwi_cache *t;

        int id, pid;

and get this, while unsuccesfully trying to connect (also note that one time getting a key worked):

$ dmesg | grep prepare_skb
[   11.775642] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   11.800047] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   13.365330] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   13.370257] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   16.468481] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xb0
[   16.472407] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x0
[   16.542017] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   16.549581] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x188
[   16.597120] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xffff
[   16.612263] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0xd0

Here we actually go a key:

[   16.614478] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =

ffff89c275297230 fc = 0x4188

Quoted 7 lines

[   16.654273] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   16.698286] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   17.735855] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =
0000000000000000 fc = 0x3333
[   17.837355] [   T1227] mt7921e 0000:04:00.0: mt7921e_tx_prepare_skb: key =

[....]

I have ordered a mt7921 card so I could reproduce this and finally took time to debug that. The issue comes to the fact that mt7921 uses 802.11 encapsulation offloading and as such we are calling ieee80211_tx_h_select_key() on a 802.3 frame.

This function casts the skb data as a 802.11 header regardless the skb is 802.11 or not in order to decide if the info->control.hw_key needs to be set. So the hw_key is likely to remain NULL in ieee80211_tx_dequeue() and because mt7921 driver needs this key to be valid data frames are dropped.

Will send a patch so that ieee80211_tx_h_select_key() does not try to get info from a ieee80211_hdr mapped on 802.3 packet data (i.e. when IEEE80211_TX_CTL_HW_80211_ENCAP is set).

Thanks

Signature · 1 line

Remi

Johannes Berg

maintainer

2025-07-17 13:46 UTC #7 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

Hi,

Quoted 2 lines · Remi Pommarel

I have ordered a mt7921 card so I could reproduce this and finally took time to debug that.

Oh wow, talk about dedication. Thank you!

Quoted 3 lines

The issue comes to the fact that mt7921 uses 802.11 encapsulation offloading and as such we are calling ieee80211_tx_h_select_key() on a 802.3 frame.

Oh... Guess we could've thought about that, sorry. Though I would've thought ath12k will also do that. Maybe not in the config you were seeing the issue in?

Quoted 10 lines

This function casts the skb data as a 802.11 header regardless the skb is 802.11 or not in order to decide if the info->control.hw_key needs to be set. So the hw_key is likely to remain NULL in ieee80211_tx_dequeue() and because mt7921 driver needs this key to be valid data frames are dropped.
Will send a patch so that ieee80211_tx_h_select_key() does not try to get info from a ieee80211_hdr mapped on 802.3 packet data (i.e. when IEEE80211_TX_CTL_HW_80211_ENCAP is set).

Sounds good!

johannes

Remi Pommarel

author

2025-07-17 14:33 UTC #8 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

Quoted 8 lines · Johannes Berg

Hi,
Quoted 2 lines · Remi Pommarel
I have ordered a mt7921 card so I could reproduce this and finally took time to debug that.
Oh wow, talk about dedication. Thank you!

Quoted 7 lines

Quoted 3 lines
The issue comes to the fact that mt7921 uses 802.11 encapsulation offloading and as such we are calling ieee80211_tx_h_select_key() on a 802.3 frame.
Oh... Guess we could've thought about that, sorry. Though I would've thought ath12k will also do that. Maybe not in the config you were seeing the issue in?

I've mainly tested on ath*k HW. Either without 802.11 encapsulation HW offloading or with it but only on ath12k and it seems that this driver does not need the key in this mode. After a quick look into ath11k and 10k that also seems to be the case for those chips.

For the record, if I am not mistaken, ath12k driver does not seem to support 802.11 encaps HW offloading mainline yet, I do seem to have some vendor patches to support it though.

Quoted 15 lines

Quoted 11 lines
This function casts the skb data as a 802.11 header regardless the skb is 802.11 or not in order to decide if the info->control.hw_key needs to be set. So the hw_key is likely to remain NULL in ieee80211_tx_dequeue() and because mt7921 driver needs this key to be valid data frames are dropped.
Will send a patch so that ieee80211_tx_h_select_key() does not try to get info from a ieee80211_hdr mapped on 802.3 packet data (i.e. when IEEE80211_TX_CTL_HW_80211_ENCAP is set).
Sounds good!
johannes

Signature · 1 line

Remi

Jeff Johnson

2025-07-17 15:46 UTC #9 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

Quoted 3 lines · Remi Pommarel

For the record, if I am not mistaken, ath12k driver does not seem to support 802.11 encaps HW offloading mainline yet, I do seem to have some vendor patches to support it though.

Upstreaming this functionality is in the internal pipeline but won't make the v6.17 merge window.

Jeff Johnson

2025-07-17 19:42 UTC #10 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

Quoted 7 lines · Jeff Johnson

Quoted 3 lines · Remi Pommarel
For the record, if I am not mistaken, ath12k driver does not seem to support 802.11 encaps HW offloading mainline yet, I do seem to have some vendor patches to support it though.
Upstreaming this functionality is in the internal pipeline but won't make the v6.17 merge window.

I make a comment like that and all of a sudden internal reviews get expedited.

s/but won't/and should/

/jeff

Johannes Berg

maintainer

2025-04-11 14:16 UTC #11 lore raw

Re: [PATCH wireless v2 1/2] wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()

In reply to Bert Karwatzki

Quoted 5 lines · Bert Karwatzki

This commit breaks the mediatek mt7921 wireless driver. In linux-next-20250410 my mt7921e Wi-Fi controller is no longer able to connect to a network. I bisected this to commit a104042e2bf6 ("wifi: mac80211: Update skb's control block key in ieee80211_tx_dequeue()").

Thanks for the report.

Remi, I see you've been debugging it already - thanks!

Nonetheless I need to get a couple of fixes out, so I've reverted it for now. We have a couple of weeks to fix it even for this release, and the bug has been around for years, as far as we know.

johannes

Remi Pommarel

author

2025-03-24 16:34 UTC #12 lore raw

PATCH v2 2/2 wifi: mac80211: Purge vif txq in ieee80211_do_stop()

In reply to Remi Pommarel

After ieee80211_do_stop() SKB from vif's txq could still be processed. Indeed another concurrent vif schedule_and_wake_txq call could cause those packets to be dequeued (see ieee80211_handle_wake_tx_queue()) without checking the sdata current state.

Because vif.drv_priv is now cleared in this function, this could lead to driver crash.

For example in ath12k, ahvif is store in vif.drv_priv. Thus if ath12k_mac_op_tx() is called after ieee80211_do_stop(), ahvif->ah can be NULL, leading the ath12k_warn(ahvif->ah,...) call in this function to trigger the NULL deref below.

  Unable to handle kernel paging request at virtual address dfffffc000000001
  KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
  batman_adv: bat0: Interface deactivated: brbh1337
  Mem abort info:
    ESR = 0x0000000096000004
    EC = 0x25: DABT (current EL), IL = 32 bits
    SET = 0, FnV = 0
    EA = 0, S1PTW = 0
    FSC = 0x04: level 0 translation fault
  Data abort info:
    ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
    CM = 0, WnR = 0, TnD = 0, TagAccess = 0
    GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
  [dfffffc000000001] address between user and kernel address ranges
  Internal error: Oops: 0000000096000004 [#1] SMP
  CPU: 1 UID: 0 PID: 978 Comm: lbd Not tainted 6.13.0-g633f875b8f1e #114
  Hardware name: HW (DT)
  pstate: 10000005 (nzcV daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : ath12k_mac_op_tx+0x6cc/0x29b8 [ath12k]
  lr : ath12k_mac_op_tx+0x174/0x29b8 [ath12k]
  sp : ffffffc086ace450
  x29: ffffffc086ace450 x28: 0000000000000000 x27: 1ffffff810d59ca4
  x26: ffffff801d05f7c0 x25: 0000000000000000 x24: 000000004000001e
  x23: ffffff8009ce4926 x22: ffffff801f9c0800 x21: ffffff801d05f7f0
  x20: ffffff8034a19f40 x19: 0000000000000000 x18: ffffff801f9c0958
  x17: ffffff800bc0a504 x16: dfffffc000000000 x15: ffffffc086ace4f8
  x14: ffffff801d05f83c x13: 0000000000000000 x12: ffffffb003a0bf03
  x11: 0000000000000000 x10: ffffffb003a0bf02 x9 : ffffff8034a19f40
  x8 : ffffff801d05f818 x7 : 1ffffff0069433dc x6 : ffffff8034a19ee0
  x5 : ffffff801d05f7f0 x4 : 0000000000000000 x3 : 0000000000000001
  x2 : 0000000000000000 x1 : dfffffc000000000 x0 : 0000000000000008
  Call trace:
   ath12k_mac_op_tx+0x6cc/0x29b8 [ath12k] (P)
   ieee80211_handle_wake_tx_queue+0x16c/0x260
   ieee80211_queue_skb+0xeec/0x1d20
   ieee80211_tx+0x200/0x2c8
   ieee80211_xmit+0x22c/0x338
   __ieee80211_subif_start_xmit+0x7e8/0xc60
   ieee80211_subif_start_xmit+0xc4/0xee0
   __ieee80211_subif_start_xmit_8023.isra.0+0x854/0x17a0
   ieee80211_subif_start_xmit_8023+0x124/0x488
   dev_hard_start_xmit+0x160/0x5a8
   __dev_queue_xmit+0x6f8/0x3120
   br_dev_queue_push_xmit+0x120/0x4a8
   __br_forward+0xe4/0x2b0
   deliver_clone+0x5c/0xd0
   br_flood+0x398/0x580
   br_dev_xmit+0x454/0x9f8
   dev_hard_start_xmit+0x160/0x5a8
   __dev_queue_xmit+0x6f8/0x3120
   ip6_finish_output2+0xc28/0x1b60
   __ip6_finish_output+0x38c/0x638
   ip6_output+0x1b4/0x338
   ip6_local_out+0x7c/0xa8
   ip6_send_skb+0x7c/0x1b0
   ip6_push_pending_frames+0x94/0xd0
   rawv6_sendmsg+0x1a98/0x2898
   inet_sendmsg+0x94/0xe0
   __sys_sendto+0x1e4/0x308
   __arm64_sys_sendto+0xc4/0x140
   do_el0_svc+0x110/0x280
   el0_svc+0x20/0x60
   el0t_64_sync_handler+0x104/0x138
   el0t_64_sync+0x154/0x158

To avoid that, empty vif's txq at ieee80211_do_stop() so no packet could be dequeued after ieee80211_do_stop() (new packets cannot be queued because SDATA_STATE_RUNNING is cleared at this point).

Fixes ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")

Signed-off-by Remi Pommarel <[email protected]>

---
 net/mac80211/iface.c | 3 +++
 1 file changed, 3 insertions(+)

net/mac80211/iface.c +3 -0

index 738de269e13f..e60c1ffebaea 100644
@@ -660,6 +660,9 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
 	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
 		ieee80211_txq_remove_vlan(local, sdata);
 
+	if (sdata->vif.txq)
+		ieee80211_txq_purge(sdata->local, to_txq_info(sdata->vif.txq));
+
 	sdata->bss = NULL;
 
 	if (local->open_count == 0)

Signature · 1 line

2.40.0