* [PATCH wireless] wifi: mt76: connac3: check headroom before pushing EHT radiotap TLVs
@ 2026-10-01 18:45 Chris Kelly
2026-10-01 18:53 ` Ben Greear
0 siblings, 1 reply; 4+ messages in thread
From: Chris Kelly @ 2026-10-01 18:45 UTC (permalink / raw)
To: linux-wireless
Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, deren.wu,
mingyen.hsieh, lucid_duck, Chris Kelly, stable
mt76_connac3_mac_decode_eht_radiotap() pushes the EHT and U-SIG radiotap
TLVs, 64 bytes, in front of the frame without checking that the skb has
room for them. It may not: the rx skb is built over the buffer with
nothing reserved -- mt76u_build_rx_skb() with MT_DRV_RX_DMA_HDR on USB,
mt76_dma_rx_process() with a zero buf_offset on PCIe -- so the only
headroom is the RX descriptor that mt7925_mac_fill_rx() pulls. A
descriptor carrying the P-RXV (group 3) but no C-RXV (group 5) leaves 48
bytes: the EHT TLV takes all of them and the U-SIG push runs past
skb->head.
Seen once on an MT7925 USB adapter in monitor mode, hopping channels:
skbuff: skb_under_panic: text:ffffffffc16f4a3c len:164 put:16 head:ffff8d4132ca3000 data:ffff8d4132ca2ff0 tail:0x94 end:0xec0 dev:<NULL>
kernel BUG at net/core/skbuff.c:214!
RIP: 0010:skb_panic+0x59/0x5b
Call Trace:
skb_push.cold+0x14/0x14
mt76_connac3_mac_radiotap_push_tlv+0x1c/0x80 [mt76_connac_lib]
mt76_connac3_mac_decode_eht_radiotap+0x7a/0x1d0 [mt76_connac_lib]
mt7925_queue_rx_skb+0x686/0xde0 [mt7925_common]
mt76u_process_rx_entry+0x2eb/0x330 [mt76_usb]
mt76u_rx_worker+0xeb/0x2b0 [mt76_usb]
Leave the EHT fields out of the radiotap header when there is no room
for them; the frame is still delivered with the rest.
Fixes: 97d7ab9f51ec ("wifi: mt76: mt7925: add EHT radiotap support in monitor mode")
Cc: stable@vger.kernel.org
Tested-by: Devin Wittmayer <lucid_duck@justthetip.ca>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Chris Kelly <hoopyfrood42@gmail.com>
---
drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c
index 651fcd4..0ed8ee7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c
@@ -210,6 +210,17 @@ void mt76_connac3_mac_decode_eht_radiotap(struct sk_buff *skb, __le32 *rxv,
"Should push tlv at the top of mac hdr"))
return;
+ /* The TLVs are pushed in front of the frame, into its headroom. The
+ * skb is built over the rx buffer with nothing reserved, on USB and
+ * PCIe alike, so that headroom is only the RX descriptor just pulled:
+ * 48 bytes with the P-RXV and no C-RXV, less than the 64 the EHT and
+ * U-SIG TLVs take, and skb_push() past skb->head is a kernel BUG.
+ * Leave the EHT fields out then.
+ */
+ if (skb_headroom(skb) < 2 * sizeof(struct ieee80211_radiotap_tlv) +
+ sizeof(*eht) + sizeof(u32) + sizeof(*usig))
+ return;
+
eht = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT,
sizeof(*eht) + sizeof(u32));
usig = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT_USIG,
--
2.55.0.windows.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH wireless] wifi: mt76: connac3: check headroom before pushing EHT radiotap TLVs 2026-10-01 18:45 [PATCH wireless] wifi: mt76: connac3: check headroom before pushing EHT radiotap TLVs Chris Kelly @ 2026-10-01 18:53 ` Ben Greear 2026-10-01 19:37 ` Chris 0 siblings, 1 reply; 4+ messages in thread From: Ben Greear @ 2026-10-01 18:53 UTC (permalink / raw) To: Chris Kelly, linux-wireless Cc: nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, deren.wu, mingyen.hsieh, lucid_duck, stable On 10/1/26 11:45, Chris Kelly wrote: > mt76_connac3_mac_decode_eht_radiotap() pushes the EHT and U-SIG radiotap > TLVs, 64 bytes, in front of the frame without checking that the skb has > room for them. It may not: the rx skb is built over the buffer with > nothing reserved -- mt76u_build_rx_skb() with MT_DRV_RX_DMA_HDR on USB, > mt76_dma_rx_process() with a zero buf_offset on PCIe -- so the only > headroom is the RX descriptor that mt7925_mac_fill_rx() pulls. A > descriptor carrying the P-RXV (group 3) but no C-RXV (group 5) leaves 48 > bytes: the EHT TLV takes all of them and the U-SIG push runs past > skb->head. Can you also increase the headroom when monitor mode is active so that it can report the data? Thanks, Ben > > Seen once on an MT7925 USB adapter in monitor mode, hopping channels: > > skbuff: skb_under_panic: text:ffffffffc16f4a3c len:164 put:16 head:ffff8d4132ca3000 data:ffff8d4132ca2ff0 tail:0x94 end:0xec0 dev:<NULL> > kernel BUG at net/core/skbuff.c:214! > RIP: 0010:skb_panic+0x59/0x5b > Call Trace: > skb_push.cold+0x14/0x14 > mt76_connac3_mac_radiotap_push_tlv+0x1c/0x80 [mt76_connac_lib] > mt76_connac3_mac_decode_eht_radiotap+0x7a/0x1d0 [mt76_connac_lib] > mt7925_queue_rx_skb+0x686/0xde0 [mt7925_common] > mt76u_process_rx_entry+0x2eb/0x330 [mt76_usb] > mt76u_rx_worker+0xeb/0x2b0 [mt76_usb] > > Leave the EHT fields out of the radiotap header when there is no room > for them; the frame is still delivered with the rest. > > Fixes: 97d7ab9f51ec ("wifi: mt76: mt7925: add EHT radiotap support in monitor mode") > Cc: stable@vger.kernel.org > Tested-by: Devin Wittmayer <lucid_duck@justthetip.ca> > Assisted-by: Claude:claude-opus-5-5 > Signed-off-by: Chris Kelly <hoopyfrood42@gmail.com> > --- > drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c > index 651fcd4..0ed8ee7 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c > @@ -210,6 +210,17 @@ void mt76_connac3_mac_decode_eht_radiotap(struct sk_buff *skb, __le32 *rxv, > "Should push tlv at the top of mac hdr")) > return; > > + /* The TLVs are pushed in front of the frame, into its headroom. The > + * skb is built over the rx buffer with nothing reserved, on USB and > + * PCIe alike, so that headroom is only the RX descriptor just pulled: > + * 48 bytes with the P-RXV and no C-RXV, less than the 64 the EHT and > + * U-SIG TLVs take, and skb_push() past skb->head is a kernel BUG. > + * Leave the EHT fields out then. > + */ > + if (skb_headroom(skb) < 2 * sizeof(struct ieee80211_radiotap_tlv) + > + sizeof(*eht) + sizeof(u32) + sizeof(*usig)) > + return; > + > eht = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT, > sizeof(*eht) + sizeof(u32)); > usig = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT_USIG, -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH wireless] wifi: mt76: connac3: check headroom before pushing EHT radiotap TLVs 2026-10-01 18:53 ` Ben Greear @ 2026-10-01 19:37 ` Chris 2026-10-01 19:50 ` Ben Greear 0 siblings, 1 reply; 4+ messages in thread From: Chris @ 2026-10-01 19:37 UTC (permalink / raw) To: Ben Greear Cc: linux-wireless, nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, deren.wu, mingyen.hsieh, lucid_duck, stable On Thu, Oct 1, 2026 at 2:53 PM Ben Greear <greearb@candelatech.com> wrote: > > On 10/1/26 11:45, Chris Kelly wrote: > > mt76_connac3_mac_decode_eht_radiotap() pushes the EHT and U-SIG radiotap > > TLVs, 64 bytes, in front of the frame without checking that the skb has > > room for them. It may not: the rx skb is built over the buffer with > > nothing reserved -- mt76u_build_rx_skb() with MT_DRV_RX_DMA_HDR on USB, > > mt76_dma_rx_process() with a zero buf_offset on PCIe -- so the only > > headroom is the RX descriptor that mt7925_mac_fill_rx() pulls. A > > descriptor carrying the P-RXV (group 3) but no C-RXV (group 5) leaves 48 > > bytes: the EHT TLV takes all of them and the U-SIG push runs past > > skb->head. > > Can you also increase the headroom when monitor mode is active so that it > can report the data? > > Thanks, > Ben > > > > > Seen once on an MT7925 USB adapter in monitor mode, hopping channels: > > > > skbuff: skb_under_panic: text:ffffffffc16f4a3c len:164 put:16 head:ffff8d4132ca3000 data:ffff8d4132ca2ff0 tail:0x94 end:0xec0 dev:<NULL> > > kernel BUG at net/core/skbuff.c:214! > > RIP: 0010:skb_panic+0x59/0x5b > > Call Trace: > > skb_push.cold+0x14/0x14 > > mt76_connac3_mac_radiotap_push_tlv+0x1c/0x80 [mt76_connac_lib] > > mt76_connac3_mac_decode_eht_radiotap+0x7a/0x1d0 [mt76_connac_lib] > > mt7925_queue_rx_skb+0x686/0xde0 [mt7925_common] > > mt76u_process_rx_entry+0x2eb/0x330 [mt76_usb] > > mt76u_rx_worker+0xeb/0x2b0 [mt76_usb] > > > > Leave the EHT fields out of the radiotap header when there is no room > > for them; the frame is still delivered with the rest. > > > > Fixes: 97d7ab9f51ec ("wifi: mt76: mt7925: add EHT radiotap support in monitor mode") > > Cc: stable@vger.kernel.org > > Tested-by: Devin Wittmayer <lucid_duck@justthetip.ca> > > Assisted-by: Claude:claude-opus-5-5 > > Signed-off-by: Chris Kelly <hoopyfrood42@gmail.com> > > --- > > drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c > > index 651fcd4..0ed8ee7 100644 > > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c > > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c > > @@ -210,6 +210,17 @@ void mt76_connac3_mac_decode_eht_radiotap(struct sk_buff *skb, __le32 *rxv, > > "Should push tlv at the top of mac hdr")) > > return; > > > > + /* The TLVs are pushed in front of the frame, into its headroom. The > > + * skb is built over the rx buffer with nothing reserved, on USB and > > + * PCIe alike, so that headroom is only the RX descriptor just pulled: > > + * 48 bytes with the P-RXV and no C-RXV, less than the 64 the EHT and > > + * U-SIG TLVs take, and skb_push() past skb->head is a kernel BUG. > > + * Leave the EHT fields out then. > > + */ > > + if (skb_headroom(skb) < 2 * sizeof(struct ieee80211_radiotap_tlv) + > > + sizeof(*eht) + sizeof(u32) + sizeof(*usig)) > > + return; > > + > > eht = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT, > > sizeof(*eht) + sizeof(u32)); > > usig = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT_USIG, > > -- > Ben Greear <greearb@candelatech.com> > Candela Technologies Inc http://www.candelatech.com > > Hi Ben, Thanks for looking at it. When the guard fires, the descriptor is the short one: P-RXV but no C-RXV. Most of what this function reports comes from the C-RXV -- LTF size and LDPC extra symbol (rxv[4]), PE disambiguity and UL/DL (rxv[5]), STA-ID (rxv[8]), BSS color and TXOP (rxv[9]), spatial reuse (rxv[13]) -- so with more headroom those fields would be filled from whatever follows the P-RXV, which is the frame itself. Devin confirmed that reading. What the short descriptor does carry is MCS, NSS, GI, bandwidth, coding and beamforming. So I'd keep this patch as the minimal crash fix for stable, and follow up for wireless-next with a change that, when the C-RXV is absent, reports just the P-RXV fields, making room with skb_cow_head() after copying the rxv words (expanding the head frees the descriptor rxv points into). The caller knows whether group 5 was present, so mt76_connac3_mac_decode_eht_radiotap() would need to be told, which touches mt7996 as well. Would that cover what you're after, or would you rather see headroom reserved for all rx buffers while a monitor interface is up? Devin saw 2.6 million EHT frames with the long descriptor (at least 176 bytes of headroom) and none with the short one; we've hit it once in about 2.5 days of scanning. Thanks, Chris ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH wireless] wifi: mt76: connac3: check headroom before pushing EHT radiotap TLVs 2026-10-01 19:37 ` Chris @ 2026-10-01 19:50 ` Ben Greear 0 siblings, 0 replies; 4+ messages in thread From: Ben Greear @ 2026-10-01 19:50 UTC (permalink / raw) To: Chris Cc: linux-wireless, nbd, lorenzo, ryder.lee, shayne.chen, sean.wang, deren.wu, mingyen.hsieh, lucid_duck, stable On 10/1/26 12:37, Chris wrote: > On Thu, Oct 1, 2026 at 2:53 PM Ben Greear <greearb@candelatech.com> wrote: >> >> On 10/1/26 11:45, Chris Kelly wrote: >>> mt76_connac3_mac_decode_eht_radiotap() pushes the EHT and U-SIG radiotap >>> TLVs, 64 bytes, in front of the frame without checking that the skb has >>> room for them. It may not: the rx skb is built over the buffer with >>> nothing reserved -- mt76u_build_rx_skb() with MT_DRV_RX_DMA_HDR on USB, >>> mt76_dma_rx_process() with a zero buf_offset on PCIe -- so the only >>> headroom is the RX descriptor that mt7925_mac_fill_rx() pulls. A >>> descriptor carrying the P-RXV (group 3) but no C-RXV (group 5) leaves 48 >>> bytes: the EHT TLV takes all of them and the U-SIG push runs past >>> skb->head. >> >> Can you also increase the headroom when monitor mode is active so that it >> can report the data? >> >> Thanks, >> Ben >> >>> >>> Seen once on an MT7925 USB adapter in monitor mode, hopping channels: >>> >>> skbuff: skb_under_panic: text:ffffffffc16f4a3c len:164 put:16 head:ffff8d4132ca3000 data:ffff8d4132ca2ff0 tail:0x94 end:0xec0 dev:<NULL> >>> kernel BUG at net/core/skbuff.c:214! >>> RIP: 0010:skb_panic+0x59/0x5b >>> Call Trace: >>> skb_push.cold+0x14/0x14 >>> mt76_connac3_mac_radiotap_push_tlv+0x1c/0x80 [mt76_connac_lib] >>> mt76_connac3_mac_decode_eht_radiotap+0x7a/0x1d0 [mt76_connac_lib] >>> mt7925_queue_rx_skb+0x686/0xde0 [mt7925_common] >>> mt76u_process_rx_entry+0x2eb/0x330 [mt76_usb] >>> mt76u_rx_worker+0xeb/0x2b0 [mt76_usb] >>> >>> Leave the EHT fields out of the radiotap header when there is no room >>> for them; the frame is still delivered with the rest. >>> >>> Fixes: 97d7ab9f51ec ("wifi: mt76: mt7925: add EHT radiotap support in monitor mode") >>> Cc: stable@vger.kernel.org >>> Tested-by: Devin Wittmayer <lucid_duck@justthetip.ca> >>> Assisted-by: Claude:claude-opus-5-5 >>> Signed-off-by: Chris Kelly <hoopyfrood42@gmail.com> >>> --- >>> drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c >>> index 651fcd4..0ed8ee7 100644 >>> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c >>> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac3_mac.c >>> @@ -210,6 +210,17 @@ void mt76_connac3_mac_decode_eht_radiotap(struct sk_buff *skb, __le32 *rxv, >>> "Should push tlv at the top of mac hdr")) >>> return; >>> >>> + /* The TLVs are pushed in front of the frame, into its headroom. The >>> + * skb is built over the rx buffer with nothing reserved, on USB and >>> + * PCIe alike, so that headroom is only the RX descriptor just pulled: >>> + * 48 bytes with the P-RXV and no C-RXV, less than the 64 the EHT and >>> + * U-SIG TLVs take, and skb_push() past skb->head is a kernel BUG. >>> + * Leave the EHT fields out then. >>> + */ >>> + if (skb_headroom(skb) < 2 * sizeof(struct ieee80211_radiotap_tlv) + >>> + sizeof(*eht) + sizeof(u32) + sizeof(*usig)) >>> + return; >>> + >>> eht = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT, >>> sizeof(*eht) + sizeof(u32)); >>> usig = mt76_connac3_mac_radiotap_push_tlv(skb, IEEE80211_RADIOTAP_EHT_USIG, >> >> -- >> Ben Greear <greearb@candelatech.com> >> Candela Technologies Inc http://www.candelatech.com >> >> > > > Hi Ben, > > Thanks for looking at it. > > When the guard fires, the descriptor is the short one: P-RXV but no > C-RXV. Most of what this function reports comes from the C-RXV -- LTF > size and LDPC extra symbol (rxv[4]), PE disambiguity and UL/DL (rxv[5]), > STA-ID (rxv[8]), BSS color and TXOP (rxv[9]), spatial reuse (rxv[13]) -- > so with more headroom those fields would be filled from whatever follows > the P-RXV, which is the frame itself. Devin confirmed that reading. What > the short descriptor does carry is MCS, NSS, GI, bandwidth, coding and > beamforming. > > So I'd keep this patch as the minimal crash fix for stable, and follow > up for wireless-next with a change that, when the C-RXV is absent, > reports just the P-RXV fields, making room with skb_cow_head() after > copying the rxv words (expanding the head frees the descriptor rxv > points into). The caller knows whether group 5 was present, so > mt76_connac3_mac_decode_eht_radiotap() would need to be told, which > touches mt7996 as well. > > Would that cover what you're after, or would you rather see headroom > reserved for all rx buffers while a monitor interface is up? Devin saw > 2.6 million EHT frames with the long descriptor (at least 176 bytes of > headroom) and none with the short one; we've hit it once in about 2.5 > days of scanning. If it mostly works and only rarely do you fail the head room check, then your current patch seems sufficient. I didn't actually look at the code in question, so if you think current approach is fine, then OK with me. We mostly test with mtk7996, and it seems to be fine in monitor mode. Thanks, Ben > > Thanks, > Chris > -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-01 19:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-01 18:45 [PATCH wireless] wifi: mt76: connac3: check headroom before pushing EHT radiotap TLVs Chris Kelly 2026-10-01 18:53 ` Ben Greear 2026-10-01 19:37 ` Chris 2026-10-01 19:50 ` Ben Greear
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox