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 <repk@triplefau.lt>
---
net/mac80211/tx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index a24636bda679..0c6214f12ea3 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -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);
--
2.40.0This 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().
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
index 881812ba03ff..3b8552a1055c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
@@ -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()?
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 34f229a6eab0..2510e3533d13 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -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
On Thu, 2025-04-10 at 23:55 +0200, Bert Karwatzki wrote:
> 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
Hi Bert,
On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> 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().
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> index 881812ba03ff..3b8552a1055c 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> @@ -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()?
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 34f229a6eab0..2510e3533d13 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -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
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 ?
> [ 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,
--
Remi
Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:
> Hi Bert,
>
> On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> > 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().
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > index 881812ba03ff..3b8552a1055c 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > @@ -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()?
> >
> > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > index 34f229a6eab0..2510e3533d13 100644
> > --- a/net/mac80211/tx.c
> > +++ b/net/mac80211/tx.c
> > @@ -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
>
> 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 ?
>
> > [ 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,
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:
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
index 881812ba03ff..cfbe7e1e4713 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
@@ -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
Hello Bert, Johannes
On Fri, Apr 11, 2025 at 01:10:02PM +0200, Bert Karwatzki wrote:
> Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:
> > Hi Bert,
> >
> > On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> > > 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().
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > index 881812ba03ff..3b8552a1055c 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > @@ -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()?
> > >
> > > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > > index 34f229a6eab0..2510e3533d13 100644
> > > --- a/net/mac80211/tx.c
> > > +++ b/net/mac80211/tx.c
> > > @@ -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
> >
> > 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 ?
> >
> > > [ 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,
>
> 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:
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> index 881812ba03ff..cfbe7e1e4713 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> @@ -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 =
[....]
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
--
Remi
Hi, On Thu, 2025-07-17 at 15:21 +0200, Remi Pommarel wrote: > 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! > 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? > 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
On Thu, Jul 17, 2025 at 03:46:00PM +0200, Johannes Berg wrote: > Hi, > > On Thu, 2025-07-17 at 15:21 +0200, Remi Pommarel wrote: > > 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! > > > 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. > > > 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
On 7/17/2025 7:22 AM, Remi Pommarel wrote: > 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.
On 7/17/2025 8:46 AM, Jeff Johnson wrote: > On 7/17/2025 7:22 AM, Remi Pommarel wrote: >> 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
On Fri, Apr 11, 2025 at 01:10:02PM +0200, Bert Karwatzki wrote:
> Am Freitag, dem 11.04.2025 um 12:06 +0200 schrieb Remi Pommarel:
> > Hi Bert,
> >
> > On Thu, Apr 10, 2025 at 11:55:26PM +0200, Bert Karwatzki wrote:
> > > 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().
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > index 881812ba03ff..3b8552a1055c 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> > > @@ -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()?
> > >
> > > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > > index 34f229a6eab0..2510e3533d13 100644
> > > --- a/net/mac80211/tx.c
> > > +++ b/net/mac80211/tx.c
> > > @@ -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
> >
> > 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 ?
> >
> > > [ 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,
>
> 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:
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> index 881812ba03ff..cfbe7e1e4713 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci_mac.c
> @@ -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
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.
> [ 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
> [...]
--
Remi
© 2016 - 2025 Red Hat, Inc.