* [RFC PATCH rtw-next 1/5] wifi: rtw88: do not trust a claimed decryption
2026-10-21 19:05 ` Bob Beckett
@ 2026-10-21 19:05 ` Bob Beckett
2026-10-21 19:05 ` [RFC PATCH rtw-next 2/5] wifi: rtw88: report and optionally re-verify suspect hw-decrypted frames Bob Beckett
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Bob Beckett @ 2026-10-21 19:05 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: linux-wireless, Luka Gejak, Bitterblue Smith, Timlee,
matthew.leach, kernel
From: Luka Gejak <luka.gejak@linux.dev>
The RX descriptor says how the CRC and ICV checks on the device's own copy
of the frame went, not that the bytes in our buffer are that frame. For a
frame that is marked decrypted mac80211 reads the PN out of the CCMP header
and stores it as the key's last PN, rejecting only a lower PN, so a single
frame with a garbage CCMP header makes every genuine frame after it be
dropped as a replay.
Only claim decryption for a frame that passed the CRC and ICV checks, and
only for an AES frame whose CCMP or GCMP header is well formed. Such a
frame is left to mac80211, which drops it without moving the replay
counter. Mark a frame that failed the ICV with RX_FLAG_FAILED_FCS_CRC,
the way rtw89 does.
Reported-by: Bob Beckett <bob.beckett@collabora.com>
Closes: https://lore.kernel.org/all/20261016191100.1781056-1-bob.beckett@collabora.com/
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
Link: https://lore.kernel.org/r/20261017083313.56113-1-luka.gejak@linux.dev
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
---
drivers/net/wireless/realtek/rtw88/rx.c | 30 +++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c
index 01fd299abb7f..e292b060ac31 100644
--- a/drivers/net/wireless/realtek/rtw88/rx.c
+++ b/drivers/net/wireless/realtek/rtw88/rx.c
@@ -208,7 +208,7 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev,
if (rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_SCAN_OFFLOAD) &&
test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
rtw_set_rx_freq_by_pktstat(pkt_stat, rx_status);
- if (pkt_stat->crc_err)
+ if (pkt_stat->crc_err || pkt_stat->icv_err)
rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
if (pkt_stat->decrypted)
rx_status->flag |= RX_FLAG_DECRYPTED;
@@ -262,6 +262,24 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev,
}
}
+static bool rtw_rx_ccmp_hdr_valid(struct rtw_rx_pkt_stat *pkt_stat,
+ struct ieee80211_hdr *hdr)
+{
+ u32 hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ u8 *iv;
+
+ if (!ieee80211_has_protected(hdr->frame_control))
+ return true;
+
+ if (pkt_stat->pkt_len < hdr_len + IEEE80211_CCMP_HDR_LEN + FCS_LEN)
+ return false;
+
+ iv = (u8 *)hdr + hdr_len;
+
+ /* reserved octet is zero, key ID octet has ExtIV set */
+ return iv[2] == 0 && (iv[3] & BIT(5));
+}
+
int rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
struct rtw_rx_pkt_stat *pkt_stat,
struct ieee80211_rx_status *rx_status)
@@ -283,7 +301,8 @@ int rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
pkt_stat->shift = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_SHIFT);
pkt_stat->phy_status = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_PHYST);
swdec = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_SWDEC);
- pkt_stat->decrypted = !swdec && enc_type != RX_DESC_ENC_NONE;
+ pkt_stat->decrypted = !swdec && enc_type != RX_DESC_ENC_NONE &&
+ !pkt_stat->crc_err && !pkt_stat->icv_err;
pkt_stat->cam_id = le32_get_bits(rx_desc->w1, RTW_RX_DESC_W1_MACID);
@@ -326,6 +345,13 @@ int rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
if (pkt_stat->phy_status)
rtwdev->chip->ops->query_phy_status(rtwdev, phy_status, pkt_stat);
+ if (pkt_stat->decrypted && enc_type == RX_DESC_ENC_AES &&
+ !rtw_rx_ccmp_hdr_valid(pkt_stat, hdr)) {
+ rtw_dbg(rtwdev, RTW_DBG_RX,
+ "AES frame without a usable CCMP header, drop the decryption flag\n");
+ pkt_stat->decrypted = false;
+ }
+
rtw_rx_fill_rx_status(rtwdev, pkt_stat, hdr, rx_status);
return 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [RFC PATCH rtw-next 2/5] wifi: rtw88: report and optionally re-verify suspect hw-decrypted frames
2026-10-21 19:05 ` Bob Beckett
2026-10-21 19:05 ` [RFC PATCH rtw-next 1/5] wifi: rtw88: do not trust a claimed decryption Bob Beckett
@ 2026-10-21 19:05 ` Bob Beckett
2026-10-21 19:05 ` [RFC PATCH rtw-next 3/5] wifi: rtw88: pci: optional RX buffer poisoning and recheck Bob Beckett
` (3 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Bob Beckett @ 2026-10-21 19:05 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: linux-wireless, Luka Gejak, Bitterblue Smith, Timlee,
matthew.leach, kernel
From: Robert Beckett <bob.beckett@collabora.com>
Debug aid for the 8822CE 5 GHz replay-counter poisoning reported in
https://lore.kernel.org/linux-wireless/20261016191100.1781056-1-bob.beckett@collabora.com/
Some 8822CE devices occasionally deliver a frame whose RX descriptor and
802.11 header are valid but whose remaining bytes, including the
cleartext CCMP header, are not that frame's, while the descriptor still
reports a successful hardware decryption. mac80211 then stores the
garbage PN as the key's last PN and drops every later frame as a replay
until the next rekey. "wifi: rtw88: do not trust a claimed decryption"
catches frames whose CCMP header is malformed, but not those where only
the PN bytes are garbage.
Track the highest PN seen per transmitter and unicast/group key for
hardware-decrypted AES data frames, reset on every key change, and treat
a frame whose PN is more than 2^24 beyond it as suspect. The transmitter
uses one PN counter per key for all TIDs, so a genuine frame on any TID
cannot get that far ahead, however long that TID was idle, while a frame
with garbage after the header carries a random 48-bit PN. A suspect PN
never becomes the reference. Suspect frames, and frames
rejected for a malformed CCMP header, are always reported with a dump of
the RX descriptor and the start of the frame (the first 20 in full, then
rate limited), so field logs show whether and how often this happens.
By default nothing else changes. With rtw88_core.sw_decrypt_suspect=Y
(also writable at runtime in /sys/module/rtw88_core/parameters/) a
suspect frame is handed to mac80211 without RX_FLAG_DECRYPTED, so it is
verified in software and dropped on MIC failure without moving the
replay counter.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
---
drivers/net/wireless/realtek/rtw88/mac80211.c | 3 +
drivers/net/wireless/realtek/rtw88/main.c | 4 +
drivers/net/wireless/realtek/rtw88/main.h | 17 +++
drivers/net/wireless/realtek/rtw88/rx.c | 118 ++++++++++++++++++
4 files changed, 142 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 766f22d31079..d5228e36fca9 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -558,6 +558,9 @@ static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
u8 hw_key_idx;
int ret = 0;
+ /* new keys restart the PN sequences rtw_rx_pn_guard() tracks */
+ atomic_inc(&rtwdev->rx_pn_guard_gen);
+
switch (key->cipher) {
case WLAN_CIPHER_SUITE_WEP40:
hw_key_type = RTW_CAM_WEP40;
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index cd9254370fcc..d667e7452e4f 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -34,14 +34,18 @@ EXPORT_SYMBOL(rtw_debug_mask);
* there is a tough reason to maintain rtw_edcca_enabled by device.
*/
bool rtw_edcca_enabled = true;
+bool rtw_sw_decrypt_suspect;
module_param_named(disable_lps_deep, rtw_disable_lps_deep_mode, bool, 0644);
module_param_named(support_bf, rtw_bf_support, bool, 0644);
module_param_named(debug_mask, rtw_debug_mask, uint, 0644);
+module_param_named(sw_decrypt_suspect, rtw_sw_decrypt_suspect, bool, 0644);
MODULE_PARM_DESC(disable_lps_deep, "Set Y to disable Deep PS");
MODULE_PARM_DESC(support_bf, "Set Y to enable beamformee support");
MODULE_PARM_DESC(debug_mask, "Debugging mask");
+MODULE_PARM_DESC(sw_decrypt_suspect,
+ "Set Y to have mac80211 verify hardware-decrypted frames whose CCMP PN jumps implausibly (default: only report them)");
static struct ieee80211_channel rtw_channeltable_2g[] = {
{.center_freq = 2412, .hw_value = 1,},
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 9c0b746540b0..49f13034db4d 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -43,6 +43,7 @@
extern bool rtw_bf_support;
extern bool rtw_disable_lps_deep_mode;
extern unsigned int rtw_debug_mask;
+extern bool rtw_sw_decrypt_suspect;
extern bool rtw_edcca_enabled;
extern const struct ieee80211_ops rtw_ops;
@@ -637,6 +638,20 @@ struct rtw_tx_pkt_info {
bool bt_null;
};
+/* Suspect frames reported in full; later ones get rate-limited one-liners */
+#define RTW_RX_PN_GUARD_DUMPS 20
+
+/* Highest CCMP PN seen per [group-addressed] key for hardware-decrypted
+ * data frames, see rtw_rx_pn_guard().
+ */
+struct rtw_rx_pn_guard {
+ u64 pn[2];
+ u8 ta[2][ETH_ALEN];
+ bool valid[2];
+ u32 gen;
+ u32 events;
+};
+
struct rtw_rx_pkt_stat {
bool phy_status;
bool icv_err;
@@ -2069,6 +2084,8 @@ struct rtw_dev {
struct rtw_fw_state fw;
struct rtw_efuse efuse;
struct rtw_sec_desc sec;
+ struct rtw_rx_pn_guard rx_pn_guard;
+ atomic_t rx_pn_guard_gen;
struct rtw_traffic_stats stats;
struct rtw_regd regd;
struct rtw_bf_info bf_info;
diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c
index e292b060ac31..7e6b0022ad50 100644
--- a/drivers/net/wireless/realtek/rtw88/rx.c
+++ b/drivers/net/wireless/realtek/rtw88/rx.c
@@ -280,6 +280,119 @@ static bool rtw_rx_ccmp_hdr_valid(struct rtw_rx_pkt_stat *pkt_stat,
return iv[2] == 0 && (iv[3] & BIT(5));
}
+/* A forward PN jump this large between two frames of one key/TID is not
+ * plausible; frames with garbage after the 802.11 header produce random
+ * 48-bit PNs.
+ */
+#define RTW_RX_PN_GUARD_JUMP BIT_ULL(24)
+
+static u64 rtw_rx_ccmp_pn(const u8 *iv)
+{
+ return (u64)iv[0] | (u64)iv[1] << 8 | (u64)iv[4] << 16 |
+ (u64)iv[5] << 24 | (u64)iv[6] << 32 | (u64)iv[7] << 40;
+}
+
+static void rtw_rx_report_suspect(struct rtw_dev *rtwdev, const char *why,
+ struct rtw_rx_pkt_stat *pkt_stat,
+ void *rx_desc8, struct ieee80211_hdr *hdr,
+ u64 max_pn)
+{
+ u32 hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ u32 n = ++rtwdev->rx_pn_guard.events;
+ u8 tid = ieee80211_is_data_qos(hdr->frame_control) ?
+ ieee80211_get_tid(hdr) : 0;
+ u64 pn = 0;
+
+ if (pkt_stat->pkt_len >= hdr_len + IEEE80211_CCMP_HDR_LEN)
+ pn = rtw_rx_ccmp_pn((u8 *)hdr + hdr_len);
+
+ if (n > RTW_RX_PN_GUARD_DUMPS) {
+ dev_warn_ratelimited(rtwdev->dev,
+ "suspect hw-decrypted frame #%u (%s): tid %u max pn 0x%012llx pn 0x%012llx%s\n",
+ n, why, tid, max_pn, pn,
+ pkt_stat->decrypted ? "" : ", left to mac80211");
+ return;
+ }
+
+ rtw_warn(rtwdev,
+ "suspect hw-decrypted frame #%u (%s): seq %u tid %u max pn 0x%012llx pn 0x%012llx len %u rate 0x%x%s\n",
+ n, why, ieee80211_get_sn(hdr), tid, max_pn, pn,
+ pkt_stat->pkt_len, pkt_stat->rate,
+ pkt_stat->decrypted ? "" : ", left to mac80211");
+ print_hex_dump(KERN_WARNING, "rtw88 rx desc: ", DUMP_PREFIX_OFFSET,
+ 16, 1, rx_desc8, (u8 *)hdr - (u8 *)rx_desc8, false);
+ print_hex_dump(KERN_WARNING, "rtw88 rx frame: ", DUMP_PREFIX_OFFSET,
+ 16, 1, hdr, min_t(u32, pkt_stat->pkt_len, hdr_len + 128),
+ false);
+}
+
+/* Some 8822CE devices occasionally deliver a frame whose descriptor and
+ * 802.11 header are valid but whose remaining bytes, including the
+ * cleartext CCMP header, are not that frame's, while the descriptor still
+ * claims a successful hardware decryption. mac80211 then stores the garbage
+ * PN as the key's last PN and drops every later frame as a replay until
+ * the next rekey.
+ *
+ * The transmitter numbers all frames of a key from one PN counter, shared
+ * by every TID (the receiver keeps per-TID replay counters only because of
+ * QoS reordering). So track the highest PN seen per transmitter and
+ * unicast/group key for hardware-decrypted data frames, and report frames
+ * whose PN is implausibly far beyond it. A genuine frame on any TID cannot
+ * be, however long that TID was idle. A suspect PN never becomes the
+ * reference, so later genuine frames are not flagged and later bad frames
+ * are still caught. With sw_decrypt_suspect set, a suspect frame is passed
+ * to mac80211 as not decrypted, so it is verified in software and dropped
+ * on MIC failure without moving the replay counter.
+ */
+static void rtw_rx_pn_guard(struct rtw_dev *rtwdev,
+ struct rtw_rx_pkt_stat *pkt_stat,
+ void *rx_desc8, struct ieee80211_hdr *hdr)
+{
+ struct rtw_rx_pn_guard *guard = &rtwdev->rx_pn_guard;
+ u32 gen = atomic_read(&rtwdev->rx_pn_guard_gen);
+ u32 hdr_len;
+ u64 pn, max;
+ u8 grp;
+
+ if (guard->gen != gen) {
+ memset(guard->valid, 0, sizeof(guard->valid));
+ guard->gen = gen;
+ }
+
+ if (!ieee80211_is_data(hdr->frame_control) ||
+ !ieee80211_has_protected(hdr->frame_control))
+ return;
+
+ hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ if (pkt_stat->pkt_len < hdr_len + IEEE80211_CCMP_HDR_LEN + FCS_LEN)
+ return;
+
+ grp = is_multicast_ether_addr(hdr->addr1);
+ pn = rtw_rx_ccmp_pn((u8 *)hdr + hdr_len);
+
+ if (!guard->valid[grp] ||
+ !ether_addr_equal(guard->ta[grp], hdr->addr2)) {
+ guard->pn[grp] = pn;
+ ether_addr_copy(guard->ta[grp], hdr->addr2);
+ guard->valid[grp] = true;
+ return;
+ }
+
+ max = guard->pn[grp];
+ if (pn <= max)
+ return;
+
+ if (pn - max > RTW_RX_PN_GUARD_JUMP) {
+ if (READ_ONCE(rtw_sw_decrypt_suspect))
+ pkt_stat->decrypted = false;
+ rtw_rx_report_suspect(rtwdev, "PN jump", pkt_stat, rx_desc8,
+ hdr, max);
+ return;
+ }
+
+ guard->pn[grp] = pn;
+}
+
int rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
struct rtw_rx_pkt_stat *pkt_stat,
struct ieee80211_rx_status *rx_status)
@@ -350,8 +463,13 @@ int rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
rtw_dbg(rtwdev, RTW_DBG_RX,
"AES frame without a usable CCMP header, drop the decryption flag\n");
pkt_stat->decrypted = false;
+ rtw_rx_report_suspect(rtwdev, "bad CCMP header", pkt_stat,
+ rx_desc8, hdr, 0);
}
+ if (pkt_stat->decrypted && enc_type == RX_DESC_ENC_AES)
+ rtw_rx_pn_guard(rtwdev, pkt_stat, rx_desc8, hdr);
+
rtw_rx_fill_rx_status(rtwdev, pkt_stat, hdr, rx_status);
return 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [RFC PATCH rtw-next 3/5] wifi: rtw88: pci: optional RX buffer poisoning and recheck
2026-10-21 19:05 ` Bob Beckett
2026-10-21 19:05 ` [RFC PATCH rtw-next 1/5] wifi: rtw88: do not trust a claimed decryption Bob Beckett
2026-10-21 19:05 ` [RFC PATCH rtw-next 2/5] wifi: rtw88: report and optionally re-verify suspect hw-decrypted frames Bob Beckett
@ 2026-10-21 19:05 ` Bob Beckett
2026-10-21 19:05 ` [RFC PATCH rtw-next 4/5] wifi: rtw88: make the PN guard threshold a test parameter Bob Beckett
` (2 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Bob Beckett @ 2026-10-21 19:05 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: linux-wireless, Luka Gejak, Bitterblue Smith, Timlee,
matthew.leach, kernel
From: Robert Beckett <bob.beckett@collabora.com>
Debug aid for the 8822CE 5 GHz replay-counter poisoning reported in
https://lore.kernel.org/linux-wireless/20261016191100.1781056-1-bob.beckett@collabora.com/
The bad frames have a valid RX descriptor and 802.11 header but foreign
bytes from the CCMP header on. On the list three explanations are open:
the device stopped writing after the header, the driver consumed the
buffer before the remaining writes were visible, or the device wrote the
wrong data (e.g. from the wrong place in its RX FIFO).
With rtw88_pci.rx_debug=Y (read when the rings are set up, so set it at
module load):
- RX buffers are mapped DMA_BIDIRECTIONAL and bytes [24, 256) of every
buffer are filled with a position-dependent pattern before the buffer
is handed to the device, also reaching a swiotlb bounce buffer;
- for each frame the core reports as suspect, the poison left in the
drv_info area and in the frame's own bytes after the 802.11 header is
counted, the head of the buffer is synced for the CPU again after
100us, and any change is reported and dumped. Reports follow the
suspect-frame budget (full for the first ones, then rate limited),
except that a reread that changed is always reported in full.
Poison after the header means the device never wrote those bytes;
foreign non-poison bytes mean it wrote the wrong data; bytes that change
on the reread mean the buffer was consumed before the device finished.
Without the parameter nothing changes.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
---
drivers/net/wireless/realtek/rtw88/main.h | 1 +
drivers/net/wireless/realtek/rtw88/pci.c | 131 +++++++++++++++++++++-
drivers/net/wireless/realtek/rtw88/pci.h | 2 +
drivers/net/wireless/realtek/rtw88/rx.c | 2 +
4 files changed, 131 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 49f13034db4d..295af5529fcc 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -659,6 +659,7 @@ struct rtw_rx_pkt_stat {
bool decrypted;
bool is_c2h;
bool channel_invalid;
+ bool suspect;
s32 signal_power;
u16 pkt_len;
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 69f2840fed09..d65408084f72 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -22,6 +22,36 @@ module_param_named(disable_aspm, rtw_pci_disable_aspm, bool, 0644);
MODULE_PARM_DESC(disable_msi, "Set Y to disable MSI interrupt support");
MODULE_PARM_DESC(disable_aspm, "Set Y to disable PCI ASPM support");
+static bool rtw_pci_rx_debug;
+module_param_named(rx_debug, rtw_pci_rx_debug, bool, 0644);
+MODULE_PARM_DESC(rx_debug,
+ "Set Y to poison RX buffers and recheck suspect frames (debug, takes effect at probe)");
+
+/* With rx_debug, bytes [RTW_PCI_RX_POISON_START, RTW_PCI_RX_POISON_END) of
+ * every RX buffer are filled with a known pattern before the buffer is
+ * handed to the device, and frames the core reports as suspect are read
+ * again after a short delay. That shows whether the device wrote the bytes
+ * after the 802.11 header at all, and whether they were still changing when
+ * the driver consumed the buffer. The buffers are mapped bidirectionally so
+ * that the pattern also reaches a swiotlb bounce buffer.
+ */
+#define RTW_PCI_RX_POISON_START 24
+#define RTW_PCI_RX_POISON_END 256
+#define RTW_PCI_RX_RECHECK_US 100
+
+static u8 rtw_pci_rx_poison_byte(u32 off)
+{
+ return 0xa5 ^ (u8)off;
+}
+
+static void rtw_pci_rx_poison(struct sk_buff *skb)
+{
+ u32 i;
+
+ for (i = RTW_PCI_RX_POISON_START; i < RTW_PCI_RX_POISON_END; i++)
+ skb->data[i] = rtw_pci_rx_poison_byte(i);
+}
+
static const u32 rtw_pci_tx_queue_idx_addr[] = {
[RTW_TX_QUEUE_BK] = RTK_PCI_TXBD_IDX_BKQ,
[RTW_TX_QUEUE_BE] = RTK_PCI_TXBD_IDX_BEQ,
@@ -128,6 +158,7 @@ static void rtw_pci_free_tx_ring(struct rtw_dev *rtwdev,
static void rtw_pci_free_rx_ring_skbs(struct rtw_dev *rtwdev,
struct rtw_pci_rx_ring *rx_ring)
{
+ struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct pci_dev *pdev = to_pci_dev(rtwdev->dev);
struct sk_buff *skb;
int buf_sz = RTK_PCI_RX_BUF_SIZE;
@@ -140,7 +171,7 @@ static void rtw_pci_free_rx_ring_skbs(struct rtw_dev *rtwdev,
continue;
dma = *((dma_addr_t *)skb->cb);
- dma_unmap_single(&pdev->dev, dma, buf_sz, DMA_FROM_DEVICE);
+ dma_unmap_single(&pdev->dev, dma, buf_sz, rtwpci->rx_dma_dir);
dev_kfree_skb(skb);
rx_ring->buf[i] = NULL;
}
@@ -211,6 +242,7 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
struct rtw_pci_rx_ring *rx_ring,
u32 idx, u32 desc_sz)
{
+ struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct pci_dev *pdev = to_pci_dev(rtwdev->dev);
struct rtw_pci_rx_buffer_desc *buf_desc;
int buf_sz = RTK_PCI_RX_BUF_SIZE;
@@ -219,7 +251,10 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
if (!skb)
return -EINVAL;
- dma = dma_map_single(&pdev->dev, skb->data, buf_sz, DMA_FROM_DEVICE);
+ if (rtwpci->rx_debug)
+ rtw_pci_rx_poison(skb);
+
+ dma = dma_map_single(&pdev->dev, skb->data, buf_sz, rtwpci->rx_dma_dir);
if (dma_mapping_error(&pdev->dev, dma))
return -EBUSY;
@@ -237,11 +272,22 @@ static void rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
struct rtw_pci_rx_ring *rx_ring,
u32 idx, u32 desc_sz)
{
+ struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct device *dev = rtwdev->dev;
struct rtw_pci_rx_buffer_desc *buf_desc;
int buf_sz = RTK_PCI_RX_BUF_SIZE;
- dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
+ if (rtwpci->rx_debug) {
+ /* Only the poisoned head needs to reach the device copy;
+ * the CPU did not write the rest of the buffer.
+ */
+ rtw_pci_rx_poison(rx_ring->buf[idx]);
+ dma_sync_single_range_for_device(dev, dma, 0,
+ RTW_PCI_RX_POISON_END,
+ DMA_BIDIRECTIONAL);
+ } else {
+ dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
+ }
buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
idx * desc_sz);
@@ -254,6 +300,7 @@ static int rtw_pci_init_rx_ring(struct rtw_dev *rtwdev,
struct rtw_pci_rx_ring *rx_ring,
u8 desc_size, u32 len)
{
+ struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct pci_dev *pdev = to_pci_dev(rtwdev->dev);
struct sk_buff *skb = NULL;
dma_addr_t dma;
@@ -302,7 +349,7 @@ static int rtw_pci_init_rx_ring(struct rtw_dev *rtwdev,
if (!skb)
continue;
dma = *((dma_addr_t *)skb->cb);
- dma_unmap_single(&pdev->dev, dma, buf_sz, DMA_FROM_DEVICE);
+ dma_unmap_single(&pdev->dev, dma, buf_sz, rtwpci->rx_dma_dir);
dev_kfree_skb_any(skb);
rx_ring->buf[i] = NULL;
}
@@ -324,6 +371,13 @@ static int rtw_pci_init_trx_ring(struct rtw_dev *rtwdev)
u32 len;
int ret;
+ rtwpci->rx_debug = READ_ONCE(rtw_pci_rx_debug);
+ rtwpci->rx_dma_dir = rtwpci->rx_debug ? DMA_BIDIRECTIONAL :
+ DMA_FROM_DEVICE;
+ if (rtwpci->rx_debug)
+ rtw_info(rtwdev, "rx_debug: poisoning RX buffers [%u, %u)\n",
+ RTW_PCI_RX_POISON_START, RTW_PCI_RX_POISON_END);
+
tx_desc_size = chip->tx_buf_desc_sz;
for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++) {
@@ -1039,6 +1093,69 @@ static int rtw_pci_get_hw_rx_ring_nr(struct rtw_dev *rtwdev,
return count;
}
+/* Called for a frame the core reported as suspect, with the buffer synced
+ * for the CPU. Count how much of the poison survived in the drv_info area
+ * and after the 802.11 header, then read the head of the buffer again a
+ * little later to see whether the device was still writing to it.
+ */
+static void rtw_pci_rx_debug_recheck(struct rtw_dev *rtwdev,
+ struct rtw_pci *rtwpci,
+ struct sk_buff *skb, dma_addr_t dma,
+ u32 idx, struct rtw_rx_pkt_stat *pkt_stat)
+{
+ u32 drv_start = rtwdev->chip->rx_pkt_desc_sz + pkt_stat->shift;
+ u32 drv_end = drv_start + pkt_stat->drv_info_sz;
+ u32 hdr_off = (u8 *)pkt_stat->hdr - skb->data;
+ u32 tail_start = hdr_off + ieee80211_hdrlen(pkt_stat->hdr->frame_control);
+ /* only bytes that belong to the frame; shorter frames leave poison */
+ u32 tail_end = min_t(u32, hdr_off + pkt_stat->pkt_len,
+ RTW_PCI_RX_POISON_END);
+ u32 tail_len = tail_end > tail_start ? tail_end - tail_start : 0;
+ u32 drv_len = drv_end > drv_start ? drv_end - drv_start : 0;
+ u32 drv_poison = 0, tail_poison = 0, changed = 0, first_changed = 0;
+ u8 before[RTW_PCI_RX_POISON_END];
+ u32 i;
+
+ memcpy(before, skb->data, sizeof(before));
+
+ for (i = max_t(u32, drv_start, RTW_PCI_RX_POISON_START);
+ i < min_t(u32, drv_end, RTW_PCI_RX_POISON_END); i++)
+ drv_poison += before[i] == rtw_pci_rx_poison_byte(i);
+ for (i = tail_start; i < tail_end; i++)
+ tail_poison += before[i] == rtw_pci_rx_poison_byte(i);
+
+ udelay(RTW_PCI_RX_RECHECK_US);
+ dma_sync_single_range_for_cpu(rtwdev->dev, dma, 0,
+ RTW_PCI_RX_POISON_END, rtwpci->rx_dma_dir);
+
+ for (i = 0; i < RTW_PCI_RX_POISON_END; i++) {
+ if (skb->data[i] == before[i])
+ continue;
+ if (!changed)
+ first_changed = i;
+ changed++;
+ }
+
+ /* a changed reread is the interesting case, always report it in full */
+ if (!changed && rtwdev->rx_pn_guard.events > RTW_RX_PN_GUARD_DUMPS) {
+ dev_warn_ratelimited(rtwdev->dev,
+ "rx_debug: ring %u: drv_info %u/%u poison, after header %u/%u poison, %u bytes changed\n",
+ idx, drv_poison, drv_len, tail_poison, tail_len,
+ changed);
+ return;
+ }
+
+ rtw_warn(rtwdev,
+ "rx_debug: ring %u: drv_info %u/%u poison, after header [%u,%u) %u/%u poison, %u bytes changed after %uus (first at %u)\n",
+ idx, drv_poison, drv_len, tail_start, tail_end,
+ tail_poison, tail_len, changed, RTW_PCI_RX_RECHECK_US,
+ first_changed);
+ if (changed)
+ print_hex_dump(KERN_WARNING, "rtw88 rx reread: ",
+ DUMP_PREFIX_OFFSET, 16, 1, skb->data,
+ RTW_PCI_RX_POISON_END, false);
+}
+
static u32 rtw_pci_rx_napi(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
u8 hw_queue, u32 limit)
{
@@ -1066,13 +1183,17 @@ static u32 rtw_pci_rx_napi(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
skb = ring->buf[cur_rp];
dma = *((dma_addr_t *)skb->cb);
dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
- DMA_FROM_DEVICE);
+ rtwpci->rx_dma_dir);
rx_desc = skb->data;
ret = rtw_rx_query_rx_desc(rtwdev, rx_desc,
&pkt_stat, &rx_status);
if (ret)
goto next_rp;
+ if (unlikely(rtwpci->rx_debug && pkt_stat.suspect))
+ rtw_pci_rx_debug_recheck(rtwdev, rtwpci, skb, dma,
+ cur_rp, &pkt_stat);
+
/* offset from rx_desc to payload */
pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
pkt_stat.shift;
diff --git a/drivers/net/wireless/realtek/rtw88/pci.h b/drivers/net/wireless/realtek/rtw88/pci.h
index 8ffdea11378f..4a08554512f7 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.h
+++ b/drivers/net/wireless/realtek/rtw88/pci.h
@@ -225,6 +225,8 @@ struct rtw_pci {
u16 link_ctrl;
atomic_t link_usage;
bool rx_no_aspm;
+ bool rx_debug;
+ enum dma_data_direction rx_dma_dir;
DECLARE_BITMAP(flags, NUM_OF_RTW_PCI_FLAGS);
void __iomem *mmap;
diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c
index 7e6b0022ad50..d29b4bed2ad3 100644
--- a/drivers/net/wireless/realtek/rtw88/rx.c
+++ b/drivers/net/wireless/realtek/rtw88/rx.c
@@ -306,6 +306,8 @@ static void rtw_rx_report_suspect(struct rtw_dev *rtwdev, const char *why,
if (pkt_stat->pkt_len >= hdr_len + IEEE80211_CCMP_HDR_LEN)
pn = rtw_rx_ccmp_pn((u8 *)hdr + hdr_len);
+ pkt_stat->suspect = true;
+
if (n > RTW_RX_PN_GUARD_DUMPS) {
dev_warn_ratelimited(rtwdev->dev,
"suspect hw-decrypted frame #%u (%s): tid %u max pn 0x%012llx pn 0x%012llx%s\n",
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [RFC PATCH rtw-next 4/5] wifi: rtw88: make the PN guard threshold a test parameter
2026-10-21 19:05 ` Bob Beckett
` (2 preceding siblings ...)
2026-10-21 19:05 ` [RFC PATCH rtw-next 3/5] wifi: rtw88: pci: optional RX buffer poisoning and recheck Bob Beckett
@ 2026-10-21 19:05 ` Bob Beckett
2026-10-21 19:05 ` [RFC PATCH rtw-next 5/5] wifi: rtw88: pci: clear CLKREQ/ASPM enables when disable_aspm is set Bob Beckett
2026-10-22 5:39 ` rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter Ping-Ke Shih
5 siblings, 0 replies; 20+ messages in thread
From: Bob Beckett @ 2026-10-21 19:05 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: linux-wireless, Luka Gejak, Bitterblue Smith, Timlee,
matthew.leach, kernel
From: Robert Beckett <bob.beckett@collabora.com>
The suspect-frame report, the rx_debug recheck and the
sw_decrypt_suspect drop path only run when a hardware-decrypted frame's
CCMP PN jumps forward by more than 2^24, which never happens on a
healthy link. Make the threshold rtw88_core.pn_guard_jump (default
2^24, unchanged behaviour) so these paths can be exercised in testing,
e.g. with pn_guard_jump=0 every frame whose PN advances is treated as
suspect. That also gives a baseline of the rx_debug poison/reread
results for good frames.
Test only; leave it at the default in the field.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
---
drivers/net/wireless/realtek/rtw88/main.c | 5 +++++
drivers/net/wireless/realtek/rtw88/main.h | 1 +
drivers/net/wireless/realtek/rtw88/rx.c | 9 ++-------
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index d667e7452e4f..0cbf65c7f2c5 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -35,17 +35,22 @@ EXPORT_SYMBOL(rtw_debug_mask);
*/
bool rtw_edcca_enabled = true;
bool rtw_sw_decrypt_suspect;
+/* Test only: lower it to make rtw_rx_pn_guard() treat ordinary frames as suspect. */
+unsigned long long rtw_pn_guard_jump = BIT_ULL(24);
module_param_named(disable_lps_deep, rtw_disable_lps_deep_mode, bool, 0644);
module_param_named(support_bf, rtw_bf_support, bool, 0644);
module_param_named(debug_mask, rtw_debug_mask, uint, 0644);
module_param_named(sw_decrypt_suspect, rtw_sw_decrypt_suspect, bool, 0644);
+module_param_named(pn_guard_jump, rtw_pn_guard_jump, ullong, 0644);
MODULE_PARM_DESC(disable_lps_deep, "Set Y to disable Deep PS");
MODULE_PARM_DESC(support_bf, "Set Y to enable beamformee support");
MODULE_PARM_DESC(debug_mask, "Debugging mask");
MODULE_PARM_DESC(sw_decrypt_suspect,
"Set Y to have mac80211 verify hardware-decrypted frames whose CCMP PN jumps implausibly (default: only report them)");
+MODULE_PARM_DESC(pn_guard_jump,
+ "CCMP PN forward jump treated as suspect (test only, default 2^24)");
static struct ieee80211_channel rtw_channeltable_2g[] = {
{.center_freq = 2412, .hw_value = 1,},
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 295af5529fcc..43c4602eb400 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -44,6 +44,7 @@ extern bool rtw_bf_support;
extern bool rtw_disable_lps_deep_mode;
extern unsigned int rtw_debug_mask;
extern bool rtw_sw_decrypt_suspect;
+extern unsigned long long rtw_pn_guard_jump;
extern bool rtw_edcca_enabled;
extern const struct ieee80211_ops rtw_ops;
diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c
index d29b4bed2ad3..8f00153568a1 100644
--- a/drivers/net/wireless/realtek/rtw88/rx.c
+++ b/drivers/net/wireless/realtek/rtw88/rx.c
@@ -280,12 +280,6 @@ static bool rtw_rx_ccmp_hdr_valid(struct rtw_rx_pkt_stat *pkt_stat,
return iv[2] == 0 && (iv[3] & BIT(5));
}
-/* A forward PN jump this large between two frames of one key/TID is not
- * plausible; frames with garbage after the 802.11 header produce random
- * 48-bit PNs.
- */
-#define RTW_RX_PN_GUARD_JUMP BIT_ULL(24)
-
static u64 rtw_rx_ccmp_pn(const u8 *iv)
{
return (u64)iv[0] | (u64)iv[1] << 8 | (u64)iv[4] << 16 |
@@ -384,7 +378,8 @@ static void rtw_rx_pn_guard(struct rtw_dev *rtwdev,
if (pn <= max)
return;
- if (pn - max > RTW_RX_PN_GUARD_JUMP) {
+ /* rtw_pn_guard_jump is 2^24 by default; see the comment above */
+ if (pn - max > READ_ONCE(rtw_pn_guard_jump)) {
if (READ_ONCE(rtw_sw_decrypt_suspect))
pkt_stat->decrypted = false;
rtw_rx_report_suspect(rtwdev, "PN jump", pkt_stat, rx_desc8,
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [RFC PATCH rtw-next 5/5] wifi: rtw88: pci: clear CLKREQ/ASPM enables when disable_aspm is set
2026-10-21 19:05 ` Bob Beckett
` (3 preceding siblings ...)
2026-10-21 19:05 ` [RFC PATCH rtw-next 4/5] wifi: rtw88: make the PN guard threshold a test parameter Bob Beckett
@ 2026-10-21 19:05 ` Bob Beckett
2026-10-21 20:50 ` Luka Gejak
2026-10-22 5:39 ` rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter Ping-Ke Shih
5 siblings, 1 reply; 20+ messages in thread
From: Bob Beckett @ 2026-10-21 19:05 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: linux-wireless, Luka Gejak, Bitterblue Smith, Timlee,
matthew.leach, kernel
From: Robert Beckett <bob.beckett@collabora.com>
disable_aspm only makes rtw_pci_clkreq_set() and rtw_pci_aspm_set()
return early, so the driver stops setting BIT_CLKREQ_SW_EN and
BIT_L1_SW_EN in RTK_PCIE_LINK_CFG but never clears them. These bits are
device state and survive rmmod/modprobe, as nothing resets the function
in between. Reloading rtw88_pci with disable_aspm=1 after a load without
it therefore leaves CLKREQ, and possibly L1, enabled in the device, and
the parameter only takes effect after a platform power cycle.
Clear both bits in rtw_pci_link_cfg() when disable_aspm is set, so the
parameter does what it says from the moment the driver probes.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
---
drivers/net/wireless/realtek/rtw88/pci.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index d65408084f72..a047409bc0b0 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -1501,6 +1501,21 @@ static void rtw_pci_aspm_set(struct rtw_dev *rtwdev, bool enable)
rtw_dbi_write8(rtwdev, RTK_PCIE_LINK_CFG, value);
}
+static void rtw_pci_link_hw_disable(struct rtw_dev *rtwdev)
+{
+ u8 value;
+ int ret;
+
+ ret = rtw_dbi_read8(rtwdev, RTK_PCIE_LINK_CFG, &value);
+ if (ret) {
+ rtw_err(rtwdev, "failed to read link config, ret=%d", ret);
+ return;
+ }
+
+ value &= ~(BIT_CLKREQ_SW_EN | BIT_L1_SW_EN);
+ rtw_dbi_write8(rtwdev, RTK_PCIE_LINK_CFG, value);
+}
+
static void rtw_pci_link_ps(struct rtw_dev *rtwdev, bool enter)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
@@ -1537,6 +1552,13 @@ static void rtw_pci_link_cfg(struct rtw_dev *rtwdev)
if (chip->id == RTW_CHIP_TYPE_8822C)
rtw_dbi_write8(rtwdev, RTK_PCIE_CLKDLY_CTRL, 0);
+ /* The CLKREQ/ASPM enable bits live in the device and are not reset
+ * by a driver reload, so a previous load may have left them set.
+ * Clear them explicitly when ASPM is disabled.
+ */
+ if (rtw_pci_disable_aspm)
+ rtw_pci_link_hw_disable(rtwdev);
+
/* Though there is standard PCIE configuration space to set the
* link control register, but by Realtek's design, driver should
* check if host supports CLKREQ/ASPM to enable the HW module.
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [RFC PATCH rtw-next 5/5] wifi: rtw88: pci: clear CLKREQ/ASPM enables when disable_aspm is set
2026-10-21 19:05 ` [RFC PATCH rtw-next 5/5] wifi: rtw88: pci: clear CLKREQ/ASPM enables when disable_aspm is set Bob Beckett
@ 2026-10-21 20:50 ` Luka Gejak
0 siblings, 0 replies; 20+ messages in thread
From: Luka Gejak @ 2026-10-21 20:50 UTC (permalink / raw)
To: Bob Beckett, Ping-Ke Shih
Cc: linux-wireless, Bitterblue Smith, Timlee, matthew.leach, kernel
Hi Bob,
For the SteamOS test build this should do the job. Few notes for upstraming
though:
On Wed, 21 Oct 2026, Bob Beckett wrote:
> Clear both bits in rtw_pci_link_cfg() when disable_aspm is set, so the
> parameter does what it says from the moment the driver probes.
rtw_pci_link_cfg() runs near the end of probe. By then
rtw_chip_info_setup() has already powered the MAC on, downloaded the
firmware and powered it off again with the old bits still set:
rtw_pci_probe()
rtw_chip_info_setup()
rtw_chip_efuse_info_setup()
rtw_chip_efuse_enable()
rtw_mac_power_on()
rtw_download_firmware()
rtw_chip_efuse_disable()
rtw_pci_phy_cfg()
rtw_pci_link_cfg()
If that power on fails, probe bails out before the bits are cleared.
Should the clear go before rtw_chip_info_setup(), or should the message
say it takes effect once probe reaches rtw_pci_phy_cfg()?
> Assisted-by: Claude:claude-opus-5-5
816d9992d9ed simplified this tag, it now just says LLM instead of the
agent and model. Or you could just drop the tag in whole as that tag
requirement was dropped(will be dropped) in linux 7.4.
[...]
> +static void rtw_pci_link_hw_disable(struct rtw_dev *rtwdev)
> +{
> + u8 value;
> + int ret;
> +
> + ret = rtw_dbi_read8(rtwdev, RTK_PCIE_LINK_CFG, &value);
disable_aspm was added in 68aa716b7dd3 for platforms where this very read
fails and the device then drops off the bus, and the FA506II quirk
(b0034a2e4156) reports the same thing. With this patch every disable_aspm
user, and the five rtw_pci_quirks[] machines on every boot, now do the
read at probe.
The reports I found hit it when leaving power save or around suspend, for
example https://bugs.launchpad.net/bugs/1872984 where it fails in
rtw_leave_ips() with the MAC off and BIT_L1_SW_EN set. After a reload
your read runs in that same state. Is it known to be safe there?
If not, clearing the bits in rtw_pci_remove() on loads that set them
would keep disable_aspm users off this register. It would also cover a
reload without disable_aspm, which today starts with L1 still on from
the last rtw_enter_ips().
> + if (ret) {
> + rtw_err(rtwdev, "failed to read link config, ret=%d", ret);
Missing \n, the helpers above have the same problem.
> + return;
> + }
> +
> + value &= ~(BIT_CLKREQ_SW_EN | BIT_L1_SW_EN);
> + rtw_dbi_write8(rtwdev, RTK_PCIE_LINK_CFG, value);
> +}
This is the fourth copy of the same read, mask and write of
RTK_PCIE_LINK_CFG in pci.c, a small helper taking the bits to clear and
set could serve all of them. rtw_pci_link_hw_disable() also reads as if
it turned the link off, a name closer to rtw_pci_clkreq_set() and
rtw_pci_aspm_set() would fit better.
[...]
> + /* The CLKREQ/ASPM enable bits live in the device and are not reset
> + * by a driver reload, so a previous load may have left them set.
> + * Clear them explicitly when ASPM is disabled.
> + */
> + if (rtw_pci_disable_aspm)
> + rtw_pci_link_hw_disable(rtwdev);
Block comments should start with /* on its own line, as netdev dropped
that comment style. The last sentence only repeats the if below and can
go, and naming BIT_CLKREQ_SW_EN and BIT_L1_SW_EN says more than "the
CLKREQ/ASPM enable bits".
Best regards,
Luka Gejak
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
2026-10-21 19:05 ` Bob Beckett
` (4 preceding siblings ...)
2026-10-21 19:05 ` [RFC PATCH rtw-next 5/5] wifi: rtw88: pci: clear CLKREQ/ASPM enables when disable_aspm is set Bob Beckett
@ 2026-10-22 5:39 ` Ping-Ke Shih
2026-10-22 12:31 ` Robert Beckett
5 siblings, 1 reply; 20+ messages in thread
From: Ping-Ke Shih @ 2026-10-22 5:39 UTC (permalink / raw)
To: Bob Beckett
Cc: linux-wireless@vger.kernel.org, Luka Gejak, Bitterblue Smith,
Timlee, matthew.leach@collabora.com, kernel@collabora.com
Bob Beckett <bob.beckett@collabora.com> wrote:
> On Wed, 21 Oct 2026 07:30:58 +0000, Ping-Ke Shih <pkshih@realtek.com> wrote:
> > Bob Beckett <bob.beckett@collabora.com> wrote:
> > >
> > > The AP's own PN sequence is consistent before and after the bad frame,
> > > so the corruption happens on the receive side.
> >
> > Maybe this is a hint to make a patch to defer (drop) reporting weird packet,
> > as Luka has fixed 3 of 4 problems.
I quickly checked the debug patches, which seem not detect if PN sequence is
correct after the bad frame, for example
seq PN
1 101
2 99999 // bad frame. should be 102.
3 103 // not detect this
Or I missed something?
> > > 4. For completeness: on these chips can RXBD_IDX or the
> > > buffer-descriptor RX tag ever be updated before the packet data DMA
> > > has completed, as described for WiFi 7 in rtw89 0bc7d1d4e63c?
> >
> > The RX tag in rtw88 located on memory allocated by dma_alloc_coherent(), and
> > the RX tag must be set after packet data MAC completed.
(fix typo: ... data DMA completed.)
>
> Understood that this is the design intent. My question was whether the host can
> ever observe the tag before the data. dma_alloc_coherent() only makes the tag
> visible to the CPU without cache maintenance; the payload is in a separate
> streaming mapping (a SWIOTLB bounce buffer on this platform), so the order in
> which the host sees them depends on the order of the device's PCIe writes.
> Relaxed Ordering is enabled on the 8822CE here (DevCtl RlxdOrd+).
> Does the 8822CE set the RO attribute on RX DMA writes, in particular the
> buffer-descriptor/tag update? If it does, the tag write may pass the payload
> writes.
>
> We are hoping to test this directly with the next round of tests. the rx_debug
> option re-syncs a suspect buffer 100 us later, so late payload writes might
> show up as changed bytes, and we can repeat the test with Relaxed Ordering
> disabled.
I'll check the behavior internally and get back to you.
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
2026-10-22 5:39 ` rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter Ping-Ke Shih
@ 2026-10-22 12:31 ` Robert Beckett
2026-11-02 8:15 ` Bernie Huang
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Robert Beckett @ 2026-10-22 12:31 UTC (permalink / raw)
To: Ping-Ke Shih
Cc: linux-wireless@vger.kernel.org, Luka Gejak, Bitterblue Smith,
Timlee, matthew.leach@collabora.com, kernel@collabora.com
From: Ping-Ke Shih <pkshih@realtek.com>
To: "Bob Beckett"<bob.beckett@collabora.com>
Cc: "linux-wireless@vger.kernel.org"<linux-wireless@vger.kernel.org>, "Luka Gejak"<luka.gejak@linux.dev>, "Bitterblue Smith"<rtl8821cerfe2@gmail.com>, "Timlee"<timlee@realtek.com>, "matthew.leach@collabora.com"<matthew.leach@collabora.com>, "kernel@collabora.com"<kernel@collabora.com>
Date: Thu, 22 Oct 2026 06:39:55 +0100
Subject: RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
> Bob Beckett <bob.beckett@collabora.com> wrote:
> > On Wed, 21 Oct 2026 07:30:58 +0000, Ping-Ke Shih <pkshih@realtek.com> wrote:
> > > Bob Beckett <bob.beckett@collabora.com> wrote:
> > > >
> > > > The AP's own PN sequence is consistent before and after the bad frame,
> > > > so the corruption happens on the receive side.
> > >
> > > Maybe this is a hint to make a patch to defer (drop) reporting weird packet,
> > > as Luka has fixed 3 of 4 problems.
>
> I quickly checked the debug patches, which seem not detect if PN sequence is
> correct after the bad frame, for example
> seq PN
> 1 101
> 2 99999 // bad frame. should be 102.
> 3 103 // not detect this
>
> Or I missed something?
if pn jumps, rtw_rx_pn_guard returns early before latching pn, so the next corrected
pn would just be a regular increase and not get dropped.
>
> > > > 4. For completeness: on these chips can RXBD_IDX or the
> > > > buffer-descriptor RX tag ever be updated before the packet data DMA
> > > > has completed, as described for WiFi 7 in rtw89 0bc7d1d4e63c?
> > >
> > > The RX tag in rtw88 located on memory allocated by dma_alloc_coherent(), and
> > > the RX tag must be set after packet data MAC completed.
>
> (fix typo: ... data DMA completed.)
>
> >
> > Understood that this is the design intent. My question was whether the host can
> > ever observe the tag before the data. dma_alloc_coherent() only makes the tag
> > visible to the CPU without cache maintenance; the payload is in a separate
> > streaming mapping (a SWIOTLB bounce buffer on this platform), so the order in
> > which the host sees them depends on the order of the device's PCIe writes.
> > Relaxed Ordering is enabled on the 8822CE here (DevCtl RlxdOrd+).
> > Does the 8822CE set the RO attribute on RX DMA writes, in particular the
> > buffer-descriptor/tag update? If it does, the tag write may pass the payload
> > writes.
> >
> > We are hoping to test this directly with the next round of tests. the rx_debug
> > option re-syncs a suspect buffer 100 us later, so late payload writes might
> > show up as changed bytes, and we can repeat the test with Relaxed Ordering
> > disabled.
>
> I'll check the behavior internally and get back to you.
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
2026-10-22 12:31 ` Robert Beckett
@ 2026-11-02 8:15 ` Bernie Huang
2026-11-02 8:33 ` Bernie Huang
2026-11-02 9:27 ` Bernie Huang
2 siblings, 0 replies; 20+ messages in thread
From: Bernie Huang @ 2026-11-02 8:15 UTC (permalink / raw)
To: Robert Beckett, Ping-Ke Shih
Cc: linux-wireless@vger.kernel.org, Luka Gejak, Bitterblue Smith,
Timlee, matthew.leach@collabora.com, kernel@collabora.com
[-- Attachment #1: Type: text/plain, Size: 4234 bytes --]
Hi Bob,
Do you mind testing this patch?
Reproduce again with the same ftrace and sniffer logs
By the result we can differentiate which is the case:
(1) AP sends incorrect CCMP header (less likely)
(2) DMA incomplete at some moment, but later the correct data can be retrieved
(3) DMA incomplete, data never recovers
IIUC, our hardware writes payload first then the tag.
The RO attribute is never set in our TLP, so despite the configuration space, PCIE RC shouldn't do reordering.
I am waiting for protocol analyzer to verify this, and will update when I have the result.
If the buffer poisoning somehow changes the timing, and fail becomes not reproducible, please let us know, thanks!
-----Original Message-----
From: Robert Beckett <bob.beckett@collabora.com>
Sent: Thursday, October 22, 2026 8:31 PM
To: Ping-Ke Shih <pkshih@realtek.com>
Cc: linux-wireless@vger.kernel.org; Luka Gejak <luka.gejak@linux.dev>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Timlee <timlee@realtek.com>; matthew.leach@collabora.com; kernel@collabora.com
Subject: RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
External mail : This email originated from outside the organization. Do not reply, click links, or open attachments unless you recognize the sender and know the content is safe.
From: Ping-Ke Shih <pkshih@realtek.com>
To: "Bob Beckett"<bob.beckett@collabora.com>
Cc: "linux-wireless@vger.kernel.org"<linux-wireless@vger.kernel.org>, "Luka Gejak"<luka.gejak@linux.dev>, "Bitterblue Smith"<rtl8821cerfe2@gmail.com>, "Timlee"<timlee@realtek.com>, "matthew.leach@collabora.com"<matthew.leach@collabora.com>, "kernel@collabora.com"<kernel@collabora.com>
Date: Thu, 22 Oct 2026 06:39:55 +0100
Subject: RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
> Bob Beckett <bob.beckett@collabora.com> wrote:
> > On Wed, 21 Oct 2026 07:30:58 +0000, Ping-Ke Shih <pkshih@realtek.com> wrote:
> > > Bob Beckett <bob.beckett@collabora.com> wrote:
> > > >
> > > > The AP's own PN sequence is consistent before and after the bad frame, > > > > so the corruption happens on the receive side.
> > >
> > > Maybe this is a hint to make a patch to defer (drop) reporting weird packet, > > > as Luka has fixed 3 of 4 problems.
>
> I quickly checked the debug patches, which seem not detect if PN sequence is > correct after the bad frame, for example
> seq PN
> 1 101
> 2 99999 // bad frame. should be 102.
> 3 103 // not detect this
>
> Or I missed something?
if pn jumps, rtw_rx_pn_guard returns early before latching pn, so the next corrected pn would just be a regular increase and not get dropped.
>
> > > > 4. For completeness: on these chips can RXBD_IDX or the
> > > > buffer-descriptor RX tag ever be updated before the packet data DMA
> > > > has completed, as described for WiFi 7 in rtw89 0bc7d1d4e63c?
> > >
> > > The RX tag in rtw88 located on memory allocated by dma_alloc_coherent(), and > > > the RX tag must be set after packet data MAC completed.
>
> (fix typo: ... data DMA completed.)
>
> >
> > Understood that this is the design intent. My question was whether the host can > > ever observe the tag before the data. dma_alloc_coherent() only makes the tag > > visible to the CPU without cache maintenance; the payload is in a separate > > streaming mapping (a SWIOTLB bounce buffer on this platform), so the order in > > which the host sees them depends on the order of the device's PCIe writes.
> > Relaxed Ordering is enabled on the 8822CE here (DevCtl RlxdOrd+).
> > Does the 8822CE set the RO attribute on RX DMA writes, in particular the > > buffer-descriptor/tag update? If it does, the tag write may pass the payload > > writes.
> >
> > We are hoping to test this directly with the next round of tests. the rx_debug > > option re-syncs a suspect buffer 100 us later, so late payload writes might > > show up as changed bytes, and we can repeat the test with Relaxed Ordering > > disabled.
>
> I'll check the behavior internally and get back to you.
>
>
>
[-- Attachment #2: 0001-rtw88-test-for-stale-packet.patch --]
[-- Type: application/octet-stream, Size: 5189 bytes --]
From cb62a316e27e952c9cf1c273952c98e7e0b0104e Mon Sep 17 00:00:00 2001
From: Po-Hao Huang <phhuang@realtek.com>
Date: Wed, 28 Oct 2026 16:28:36 +0800
Subject: [PATCH] rtw88: test for stale packet
Fellow developers found evidence that DMA operations might
be incomplete during debugging. e.g. Rx tag is new but the
remaining Rx buffer is old data.
Since protocol anaylzer is not available at the moment,
write the Rx buffer to a magic number after each use, and check
for more convincing evidence for further troubleshooting.
Change-Id: Icbe818746353b79478f00f2ab7f635997e3bf2f9
Signed-off-by: Po-Hao Huang <phhuang@realtek.com>
---
pci.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 97 insertions(+), 4 deletions(-)
diff --git a/pci.c b/pci.c
index 452df655..049bd88c 100644
--- a/pci.c
+++ b/pci.c
@@ -15,6 +15,10 @@
#include "debug.h"
#include "mac.h"
+#define RTW_PCI_RX_POISON 0xDEADBEEF
+#define RTW_PCI_RX_POISON_LEN 256
+#define RTW_PCI_RX_POISON_TIMEOUT 100
+
static bool rtw_disable_msi;
static bool rtw_pci_disable_aspm;
module_param_named(disable_msi, rtw_disable_msi, bool, 0644);
@@ -207,6 +211,94 @@ static int rtw_pci_init_tx_ring(struct rtw_dev *rtwdev,
return 0;
}
+static void rtw_pci_rx_poison(struct sk_buff *skb)
+{
+ memset32((u32 *)skb->data, RTW_PCI_RX_POISON,
+ RTW_PCI_RX_POISON_LEN / 4);
+}
+
+static bool rtw_pci_rx_range_poisoned(struct rtw_dev *rtwdev,
+ struct sk_buff *skb, u32 start, u32 len)
+{
+ u32 *word = (u32 *)skb->data;
+ u32 i, first = start / 4, last = DIV_ROUND_UP(start + len, 4);
+
+ if (!len)
+ return false;
+
+ for (i = first; i < last; i++)
+ if (READ_ONCE(word[i]) == RTW_PCI_RX_POISON) {
+ rtw_warn(rtwdev, "rx buffer still 0xdeadbeef\n");
+ return true;
+ }
+
+ return false;
+}
+
+static bool rtw_pci_rx_stale(struct rtw_dev *rtwdev, struct sk_buff *skb)
+{
+ struct rtw_rx_desc *rx_desc = (void *)skb->data;
+ u32 desc_sz = rtwdev->chip->rx_pkt_desc_sz;
+ u8 drv_info_sz, shift, enc_type;
+ struct ieee80211_hdr *hdr;
+ u32 pkt_len, offset, len;
+ bool is_c2h;
+ __le16 fc;
+
+ if (rtw_pci_rx_range_poisoned(rtwdev, skb, 0, desc_sz))
+ return true;
+
+ pkt_len = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_PKT_LEN);
+ drv_info_sz = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_DRV_INFO_SIZE);
+ shift = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_SHIFT);
+ offset = desc_sz + drv_info_sz * 8 + shift;
+ if (offset >= RTW_PCI_RX_POISON_LEN)
+ return false;
+
+ is_c2h = le32_get_bits(rx_desc->w2, RTW_RX_DESC_W2_C2H);
+ if (is_c2h) {
+ len = min_t(u32, pkt_len, 4);
+ } else if (pkt_len < sizeof(fc)) {
+ len = pkt_len;
+ } else {
+ if (rtw_pci_rx_range_poisoned(rtwdev, skb, offset, sizeof(fc)))
+ return true;
+
+ hdr = (struct ieee80211_hdr *)(skb->data + offset);
+ fc = hdr->frame_control;
+ len = ieee80211_hdrlen(fc);
+
+ enc_type = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_ENC_TYPE);
+ if (ieee80211_has_protected(fc) && enc_type == RX_DESC_ENC_AES)
+ len += IEEE80211_CCMP_HDR_LEN;
+ len = min(len, pkt_len);
+ }
+
+ len = min(len, RTW_PCI_RX_POISON_LEN - offset);
+
+ return rtw_pci_rx_range_poisoned(rtwdev, skb, offset, len);
+}
+
+static void rtw_pci_rx_sync_and_wait_fresh(struct rtw_dev *rtwdev,
+ struct sk_buff *skb, dma_addr_t dma)
+{
+ ktime_t deadline = ktime_add_ms(ktime_get(), RTW_PCI_RX_POISON_TIMEOUT);
+
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
+
+ while (rtw_pci_rx_stale(rtwdev, skb)) {
+ if (!ktime_before(ktime_get(), deadline)) {
+ rtw_warn(rtwdev, "rx buffer still poisoned after %d ms\n",
+ RTW_PCI_RX_POISON_TIMEOUT);
+ return;
+ }
+ udelay(1);
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
+ }
+}
+
static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
struct rtw_pci_rx_ring *rx_ring,
u32 idx, u32 desc_sz)
@@ -219,7 +311,8 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
if (!skb)
return -EINVAL;
- dma = dma_map_single(&pdev->dev, skb->data, buf_sz, DMA_FROM_DEVICE);
+ rtw_pci_rx_poison(skb);
+ dma = dma_map_single(&pdev->dev, skb->data, buf_sz, DMA_BIDIRECTIONAL);
if (dma_mapping_error(&pdev->dev, dma))
return -EBUSY;
@@ -241,7 +334,8 @@ static void rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
struct rtw_pci_rx_buffer_desc *buf_desc;
int buf_sz = RTK_PCI_RX_BUF_SIZE;
- dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
+ rtw_pci_rx_poison(rx_ring->buf[idx]);
+ dma_sync_single_for_device(dev, dma, buf_sz, DMA_BIDIRECTIONAL);
buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
idx * desc_sz);
@@ -1064,8 +1158,7 @@ static u32 rtw_pci_rx_napi(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
rtw_pci_dma_check(rtwdev, ring, cur_rp);
skb = ring->buf[cur_rp];
dma = *((dma_addr_t *)skb->cb);
- dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
- DMA_FROM_DEVICE);
+ rtw_pci_rx_sync_and_wait_fresh(rtwdev, skb, dma);
rx_desc = skb->data;
rtw_rx_query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
2026-10-22 12:31 ` Robert Beckett
2026-11-02 8:15 ` Bernie Huang
@ 2026-11-02 8:33 ` Bernie Huang
2026-11-02 9:27 ` Bernie Huang
2 siblings, 0 replies; 20+ messages in thread
From: Bernie Huang @ 2026-11-02 8:33 UTC (permalink / raw)
To: Robert Beckett, Ping-Ke Shih
Cc: linux-wireless@vger.kernel.org, Luka Gejak, Bitterblue Smith,
Timlee, matthew.leach@collabora.com, kernel@collabora.com
> > Bob Beckett <bob.beckett@collabora.com> wrote:
> > > On Wed, 21 Oct 2026 07:30:58 +0000, Ping-Ke Shih
> <pkshih@realtek.com> wrote:
> > > > Bob Beckett <bob.beckett@collabora.com> wrote:
> > > > >
> > > > > The AP's own PN sequence is consistent before and after the bad
> frame, > > > > so the corruption happens on the receive side.
> > > >
> > > > Maybe this is a hint to make a patch to defer (drop) reporting weird
> packet, > > > as Luka has fixed 3 of 4 problems.
> >
> > I quickly checked the debug patches, which seem not detect if PN sequence
> is > correct after the bad frame, for example
> > seq PN
> > 1 101
> > 2 99999 // bad frame. should be 102.
> > 3 103 // not detect this
> >
> > Or I missed something?
>
> if pn jumps, rtw_rx_pn_guard returns early before latching pn, so the next
> corrected pn would just be a regular increase and not get dropped.
>
> >
> > > > > 4. For completeness: on these chips can RXBD_IDX or the
> > > > > buffer-descriptor RX tag ever be updated before the packet data
> DMA
> > > > > has completed, as described for WiFi 7 in rtw89 0bc7d1d4e63c?
> > > >
> > > > The RX tag in rtw88 located on memory allocated by
> dma_alloc_coherent(), and > > > the RX tag must be set after packet data
> MAC completed.
> >
> > (fix typo: ... data DMA completed.)
> >
> > >
> > > Understood that this is the design intent. My question was whether the
> host can > > ever observe the tag before the data. dma_alloc_coherent()
> only makes the tag > > visible to the CPU without cache maintenance; the
> payload is in a separate > > streaming mapping (a SWIOTLB bounce buffer on
> this platform), so the order in > > which the host sees them depends on the
> order of the device's PCIe writes.
> > > Relaxed Ordering is enabled on the 8822CE here (DevCtl RlxdOrd+).
> > > Does the 8822CE set the RO attribute on RX DMA writes, in particular the
> > > buffer-descriptor/tag update? If it does, the tag write may pass the payload
> > > writes.
> > >
> > > We are hoping to test this directly with the next round of tests. the
> rx_debug > > option re-syncs a suspect buffer 100 us later, so late payload
> writes might > > show up as changed bytes, and we can repeat the test with
> Relaxed Ordering > > disabled.
> >
> > I'll check the behavior internally and get back to you.
> >
> >
> >
>
Hi Bob,
Do you mind testing this patch?
Reproduce again with the same ftrace and sniffer logs
By the result we can differentiate which is the case:
(1) AP sends incorrect CCMP header (less likely)
(2) DMA incomplete at some moment, but later the correct data can be retrieved
(3) DMA incomplete, data never recovers
IIUC, our hardware writes payload first then the tag.
The RO attribute is never set in our TLP, so despite the configuration space, PCIE RC shouldn't do reordering.
I am waiting for protocol analyzer to verify this, and will update when I have the result.
If the buffer poisoning somehow changes the timing, and fail becomes not reproducible, please let us know, thanks!
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
2026-10-22 12:31 ` Robert Beckett
2026-11-02 8:15 ` Bernie Huang
2026-11-02 8:33 ` Bernie Huang
@ 2026-11-02 9:27 ` Bernie Huang
2026-11-03 16:25 ` Robert Beckett
2 siblings, 1 reply; 20+ messages in thread
From: Bernie Huang @ 2026-11-02 9:27 UTC (permalink / raw)
To: Robert Beckett, Ping-Ke Shih
Cc: linux-wireless@vger.kernel.org, Luka Gejak, Bitterblue Smith,
Timlee, matthew.leach@collabora.com, kernel@collabora.com
[-- Attachment #1: Type: text/plain, Size: 3246 bytes --]
Bob Beckett <bob.beckett@collabora.com> wrote:
> > > On Wed, 21 Oct 2026 07:30:58 +0000, Ping-Ke Shih
> <pkshih@realtek.com> wrote:
> > > > Bob Beckett <bob.beckett@collabora.com> wrote:
> > > > >
> > > > > The AP's own PN sequence is consistent before and after the bad
> frame, > > > > so the corruption happens on the receive side.
> > > >
> > > > Maybe this is a hint to make a patch to defer (drop) reporting weird
> packet, > > > as Luka has fixed 3 of 4 problems.
> >
> > I quickly checked the debug patches, which seem not detect if PN sequence
> is > correct after the bad frame, for example
> > seq PN
> > 1 101
> > 2 99999 // bad frame. should be 102.
> > 3 103 // not detect this
> >
> > Or I missed something?
>
> if pn jumps, rtw_rx_pn_guard returns early before latching pn, so the next
> corrected pn would just be a regular increase and not get dropped.
>
> >
> > > > > 4. For completeness: on these chips can RXBD_IDX or the
> > > > > buffer-descriptor RX tag ever be updated before the packet data
> DMA
> > > > > has completed, as described for WiFi 7 in rtw89 0bc7d1d4e63c?
> > > >
> > > > The RX tag in rtw88 located on memory allocated by
> dma_alloc_coherent(), and > > > the RX tag must be set after packet data
> MAC completed.
> >
> > (fix typo: ... data DMA completed.)
> >
> > >
> > > Understood that this is the design intent. My question was whether the
> host can > > ever observe the tag before the data. dma_alloc_coherent()
> only makes the tag > > visible to the CPU without cache maintenance; the
> payload is in a separate > > streaming mapping (a SWIOTLB bounce buffer on
> this platform), so the order in > > which the host sees them depends on the
> order of the device's PCIe writes.
> > > Relaxed Ordering is enabled on the 8822CE here (DevCtl RlxdOrd+).
> > > Does the 8822CE set the RO attribute on RX DMA writes, in particular the
> > > buffer-descriptor/tag update? If it does, the tag write may pass the payload
> > > writes.
> > >
> > > We are hoping to test this directly with the next round of tests. the
> rx_debug > > option re-syncs a suspect buffer 100 us later, so late payload
> writes might > > show up as changed bytes, and we can repeat the test with
> Relaxed Ordering > > disabled.
> >
> > I'll check the behavior internally and get back to you.
> >
> >
> >
>
// Please ignore the previous two emails
Hi Bob,
Do you mind testing this patch?
Reproduce again with the same ftrace and sniffer logs
By the result we can differentiate which is the case:
(1) AP sends incorrect CCMP header (less likely)
(2) DMA incomplete, but later the correct data can be retrieved
(3) DMA incomplete, data never recovers
IIUC, our hardware writes payload first then the tag.
The RO attribute is never set in our TLP, so despite the configuration
space settings, PCIE RC shouldn't do reordering.
I am waiting for protocol analyzer to verify this, and will update
later when I have the result.
If the buffer poisoning somehow changes the timing, and fail becomes
not reproducible, please let us know, thanks!
[-- Attachment #2: 0001-wifi-rtw88-test-for-stale-packet.patch --]
[-- Type: application/octet-stream, Size: 5302 bytes --]
From 41175f8e54a044a5a0e273e3ac9b91e8866fcdf0 Mon Sep 17 00:00:00 2001
From: Po-Hao Huang <phhuang@realtek.com>
Date: Mon, 2 Nov 2026 17:26:31 +0800
Subject: [PATCH] wifi: rtw88: test for stale packet
Fellow developers found evidence that DMA operations might
be incomplete during debugging. e.g. Rx tag is new but the
remaining Rx buffer is old data.
Since protocol anaylzer is not available at the moment,
write the Rx buffer to a magic number after each use, and check
for more convincing evidence for further troubleshooting.
Signed-off-by: Po-Hao Huang <phhuang@realtek.com>
---
drivers/net/wireless/realtek/rtw88/pci.c | 101 ++++++++++++++++++++++-
1 file changed, 97 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 66d2e5f51444..815087789b93 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -15,6 +15,10 @@
#include "debug.h"
#include "mac.h"
+#define RTW_PCI_RX_POISON 0xDEADBEEF
+#define RTW_PCI_RX_POISON_LEN 256
+#define RTW_PCI_RX_POISON_TIMEOUT 100
+
static bool rtw_disable_msi;
static bool rtw_pci_disable_aspm;
module_param_named(disable_msi, rtw_disable_msi, bool, 0644);
@@ -207,6 +211,94 @@ static int rtw_pci_init_tx_ring(struct rtw_dev *rtwdev,
return 0;
}
+static void rtw_pci_rx_poison(struct sk_buff *skb)
+{
+ memset32((u32 *)skb->data, RTW_PCI_RX_POISON,
+ RTW_PCI_RX_POISON_LEN / 4);
+}
+
+static bool rtw_pci_rx_range_poisoned(struct rtw_dev *rtwdev,
+ struct sk_buff *skb, u32 start, u32 len)
+{
+ u32 *word = (u32 *)skb->data;
+ u32 i, first = start / 4, last = DIV_ROUND_UP(start + len, 4);
+
+ if (!len)
+ return false;
+
+ for (i = first; i < last; i++)
+ if (READ_ONCE(word[i]) == RTW_PCI_RX_POISON) {
+ rtw_warn(rtwdev, "rx buffer still 0xdeadbeef\n");
+ return true;
+ }
+
+ return false;
+}
+
+static bool rtw_pci_rx_stale(struct rtw_dev *rtwdev, struct sk_buff *skb)
+{
+ struct rtw_rx_desc *rx_desc = (void *)skb->data;
+ u32 desc_sz = rtwdev->chip->rx_pkt_desc_sz;
+ u8 drv_info_sz, shift, enc_type;
+ struct ieee80211_hdr *hdr;
+ u32 pkt_len, offset, len;
+ bool is_c2h;
+ __le16 fc;
+
+ if (rtw_pci_rx_range_poisoned(rtwdev, skb, 0, desc_sz))
+ return true;
+
+ pkt_len = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_PKT_LEN);
+ drv_info_sz = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_DRV_INFO_SIZE);
+ shift = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_SHIFT);
+ offset = desc_sz + drv_info_sz * 8 + shift;
+ if (offset >= RTW_PCI_RX_POISON_LEN)
+ return false;
+
+ is_c2h = le32_get_bits(rx_desc->w2, RTW_RX_DESC_W2_C2H);
+ if (is_c2h) {
+ len = min_t(u32, pkt_len, 4);
+ } else if (pkt_len < sizeof(fc)) {
+ len = pkt_len;
+ } else {
+ if (rtw_pci_rx_range_poisoned(rtwdev, skb, offset, sizeof(fc)))
+ return true;
+
+ hdr = (struct ieee80211_hdr *)(skb->data + offset);
+ fc = hdr->frame_control;
+ len = ieee80211_hdrlen(fc);
+
+ enc_type = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_ENC_TYPE);
+ if (ieee80211_has_protected(fc) && enc_type == RX_DESC_ENC_AES)
+ len += IEEE80211_CCMP_HDR_LEN;
+ len = min(len, pkt_len);
+ }
+
+ len = min(len, RTW_PCI_RX_POISON_LEN - offset);
+
+ return rtw_pci_rx_range_poisoned(rtwdev, skb, offset, len);
+}
+
+static void rtw_pci_rx_sync_and_wait_fresh(struct rtw_dev *rtwdev,
+ struct sk_buff *skb, dma_addr_t dma)
+{
+ ktime_t deadline = ktime_add_ms(ktime_get(), RTW_PCI_RX_POISON_TIMEOUT);
+
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
+
+ while (rtw_pci_rx_stale(rtwdev, skb)) {
+ if (!ktime_before(ktime_get(), deadline)) {
+ rtw_warn(rtwdev, "rx buffer still poisoned after %d ms\n",
+ RTW_PCI_RX_POISON_TIMEOUT);
+ return;
+ }
+ udelay(1);
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
+ }
+}
+
static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
struct rtw_pci_rx_ring *rx_ring,
u32 idx, u32 desc_sz)
@@ -219,7 +311,8 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
if (!skb)
return -EINVAL;
- dma = dma_map_single(&pdev->dev, skb->data, buf_sz, DMA_FROM_DEVICE);
+ rtw_pci_rx_poison(skb);
+ dma = dma_map_single(&pdev->dev, skb->data, buf_sz, DMA_BIDIRECTIONAL);
if (dma_mapping_error(&pdev->dev, dma))
return -EBUSY;
@@ -241,7 +334,8 @@ static void rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
struct rtw_pci_rx_buffer_desc *buf_desc;
int buf_sz = RTK_PCI_RX_BUF_SIZE;
- dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
+ rtw_pci_rx_poison(rx_ring->buf[idx]);
+ dma_sync_single_for_device(dev, dma, buf_sz, DMA_BIDIRECTIONAL);
buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
idx * desc_sz);
@@ -1065,8 +1159,7 @@ static u32 rtw_pci_rx_napi(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
rtw_pci_dma_check(rtwdev, ring, cur_rp);
skb = ring->buf[cur_rp];
dma = *((dma_addr_t *)skb->cb);
- dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
- DMA_FROM_DEVICE);
+ rtw_pci_rx_sync_and_wait_fresh(rtwdev, skb, dma);
rx_desc = skb->data;
ret = rtw_rx_query_rx_desc(rtwdev, rx_desc,
&pkt_stat, &rx_status);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
2026-11-02 9:27 ` Bernie Huang
@ 2026-11-03 16:25 ` Robert Beckett
0 siblings, 0 replies; 20+ messages in thread
From: Robert Beckett @ 2026-11-03 16:25 UTC (permalink / raw)
To: Bernie Huang
Cc: Ping-Ke Shih, linux-wireless@vger.kernel.org, Luka Gejak,
Bitterblue Smith, Timlee, matthew.leach@collabora.com,
kernel@collabora.com
[-- Attachment #1: Type: text/plain, Size: 4453 bytes --]
From: Bernie Huang <phhuang@realtek.com>
To: "Robert Beckett"<bob.beckett@collabora.com>, "Ping-Ke Shih"<pkshih@realtek.com>
Cc: "linux-wireless@vger.kernel.org"<linux-wireless@vger.kernel.org>, "Luka Gejak"<luka.gejak@linux.dev>, "Bitterblue Smith"<rtl8821cerfe2@gmail.com>, "Timlee"<timlee@realtek.com>, "matthew.leach@collabora.com"<matthew.leach@collabora.com>, "kernel@collabora.com"<kernel@collabora.com>
Date: Mon, 02 Nov 2026 09:27:43 +0000
Subject: RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter
> Bob Beckett <bob.beckett@collabora.com> wrote:
> > > > On Wed, 21 Oct 2026 07:30:58 +0000, Ping-Ke Shih
> > <pkshih@realtek.com> wrote:
> > > > > Bob Beckett <bob.beckett@collabora.com> wrote:
> > > > > >
> > > > > > The AP's own PN sequence is consistent before and after the bad
> > frame, > > > > so the corruption happens on the receive side.
> > > > >
> > > > > Maybe this is a hint to make a patch to defer (drop) reporting weird
> > packet, > > > as Luka has fixed 3 of 4 problems.
> > >
> > > I quickly checked the debug patches, which seem not detect if PN sequence
> > is > correct after the bad frame, for example
> > > seq PN
> > > 1 101
> > > 2 99999 // bad frame. should be 102.
> > > 3 103 // not detect this
> > >
> > > Or I missed something?
> >
> > if pn jumps, rtw_rx_pn_guard returns early before latching pn, so the next
> > corrected pn would just be a regular increase and not get dropped.
> >
> > >
> > > > > > 4. For completeness: on these chips can RXBD_IDX or the
> > > > > > buffer-descriptor RX tag ever be updated before the packet data
> > DMA
> > > > > > has completed, as described for WiFi 7 in rtw89 0bc7d1d4e63c?
> > > > >
> > > > > The RX tag in rtw88 located on memory allocated by
> > dma_alloc_coherent(), and > > > the RX tag must be set after packet data
> > MAC completed.
> > >
> > > (fix typo: ... data DMA completed.)
> > >
> > > >
> > > > Understood that this is the design intent. My question was whether the
> > host can > > ever observe the tag before the data. dma_alloc_coherent()
> > only makes the tag > > visible to the CPU without cache maintenance; the
> > payload is in a separate > > streaming mapping (a SWIOTLB bounce buffer on
> > this platform), so the order in > > which the host sees them depends on the
> > order of the device's PCIe writes.
> > > > Relaxed Ordering is enabled on the 8822CE here (DevCtl RlxdOrd+).
> > > > Does the 8822CE set the RO attribute on RX DMA writes, in particular the
> > > > buffer-descriptor/tag update? If it does, the tag write may pass the payload
> > > > writes.
> > > >
> > > > We are hoping to test this directly with the next round of tests. the
> > rx_debug > > option re-syncs a suspect buffer 100 us later, so late payload
> > writes might > > show up as changed bytes, and we can repeat the test with
> > Relaxed Ordering > > disabled.
> > >
> > > I'll check the behavior internally and get back to you.
> > >
> > >
> > >
> >
>
> // Please ignore the previous two emails
>
> Hi Bob,
>
> Do you mind testing this patch?
> Reproduce again with the same ftrace and sniffer logs
> By the result we can differentiate which is the case:
> (1) AP sends incorrect CCMP header (less likely)
> (2) DMA incomplete, but later the correct data can be retrieved
> (3) DMA incomplete, data never recovers
>
> IIUC, our hardware writes payload first then the tag.
> The RO attribute is never set in our TLP, so despite the configuration
> space settings, PCIE RC shouldn't do reordering.
> I am waiting for protocol analyzer to verify this, and will update
> later when I have the result.
> If the buffer poisoning somehow changes the timing, and fail becomes
> not reproducible, please let us know, thanks!
>
I can't test this yet, only an end cusomer is able to repro on their router so far.
I have the previous patches included in the next version going out to customers soon, but it takes time currently.
The good news is that I managed to find one of the affected routers on ebay France. It should be with me my end of the week and I'll be able to try reproducing the issue myself and debug much more quickly from then.
Once I have the device, I'll test with your patch and report back in due course.
[-- Attachment #2: 0001-wifi-rtw88-test-for-stale-packet.patch --]
[-- Type: application/octet-stream, Size: 5302 bytes --]
From 41175f8e54a044a5a0e273e3ac9b91e8866fcdf0 Mon Sep 17 00:00:00 2001
From: Po-Hao Huang <phhuang@realtek.com>
Date: Mon, 2 Nov 2026 17:26:31 +0800
Subject: [PATCH] wifi: rtw88: test for stale packet
Fellow developers found evidence that DMA operations might
be incomplete during debugging. e.g. Rx tag is new but the
remaining Rx buffer is old data.
Since protocol anaylzer is not available at the moment,
write the Rx buffer to a magic number after each use, and check
for more convincing evidence for further troubleshooting.
Signed-off-by: Po-Hao Huang <phhuang@realtek.com>
---
drivers/net/wireless/realtek/rtw88/pci.c | 101 ++++++++++++++++++++++-
1 file changed, 97 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 66d2e5f51444..815087789b93 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -15,6 +15,10 @@
#include "debug.h"
#include "mac.h"
+#define RTW_PCI_RX_POISON 0xDEADBEEF
+#define RTW_PCI_RX_POISON_LEN 256
+#define RTW_PCI_RX_POISON_TIMEOUT 100
+
static bool rtw_disable_msi;
static bool rtw_pci_disable_aspm;
module_param_named(disable_msi, rtw_disable_msi, bool, 0644);
@@ -207,6 +211,94 @@ static int rtw_pci_init_tx_ring(struct rtw_dev *rtwdev,
return 0;
}
+static void rtw_pci_rx_poison(struct sk_buff *skb)
+{
+ memset32((u32 *)skb->data, RTW_PCI_RX_POISON,
+ RTW_PCI_RX_POISON_LEN / 4);
+}
+
+static bool rtw_pci_rx_range_poisoned(struct rtw_dev *rtwdev,
+ struct sk_buff *skb, u32 start, u32 len)
+{
+ u32 *word = (u32 *)skb->data;
+ u32 i, first = start / 4, last = DIV_ROUND_UP(start + len, 4);
+
+ if (!len)
+ return false;
+
+ for (i = first; i < last; i++)
+ if (READ_ONCE(word[i]) == RTW_PCI_RX_POISON) {
+ rtw_warn(rtwdev, "rx buffer still 0xdeadbeef\n");
+ return true;
+ }
+
+ return false;
+}
+
+static bool rtw_pci_rx_stale(struct rtw_dev *rtwdev, struct sk_buff *skb)
+{
+ struct rtw_rx_desc *rx_desc = (void *)skb->data;
+ u32 desc_sz = rtwdev->chip->rx_pkt_desc_sz;
+ u8 drv_info_sz, shift, enc_type;
+ struct ieee80211_hdr *hdr;
+ u32 pkt_len, offset, len;
+ bool is_c2h;
+ __le16 fc;
+
+ if (rtw_pci_rx_range_poisoned(rtwdev, skb, 0, desc_sz))
+ return true;
+
+ pkt_len = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_PKT_LEN);
+ drv_info_sz = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_DRV_INFO_SIZE);
+ shift = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_SHIFT);
+ offset = desc_sz + drv_info_sz * 8 + shift;
+ if (offset >= RTW_PCI_RX_POISON_LEN)
+ return false;
+
+ is_c2h = le32_get_bits(rx_desc->w2, RTW_RX_DESC_W2_C2H);
+ if (is_c2h) {
+ len = min_t(u32, pkt_len, 4);
+ } else if (pkt_len < sizeof(fc)) {
+ len = pkt_len;
+ } else {
+ if (rtw_pci_rx_range_poisoned(rtwdev, skb, offset, sizeof(fc)))
+ return true;
+
+ hdr = (struct ieee80211_hdr *)(skb->data + offset);
+ fc = hdr->frame_control;
+ len = ieee80211_hdrlen(fc);
+
+ enc_type = le32_get_bits(rx_desc->w0, RTW_RX_DESC_W0_ENC_TYPE);
+ if (ieee80211_has_protected(fc) && enc_type == RX_DESC_ENC_AES)
+ len += IEEE80211_CCMP_HDR_LEN;
+ len = min(len, pkt_len);
+ }
+
+ len = min(len, RTW_PCI_RX_POISON_LEN - offset);
+
+ return rtw_pci_rx_range_poisoned(rtwdev, skb, offset, len);
+}
+
+static void rtw_pci_rx_sync_and_wait_fresh(struct rtw_dev *rtwdev,
+ struct sk_buff *skb, dma_addr_t dma)
+{
+ ktime_t deadline = ktime_add_ms(ktime_get(), RTW_PCI_RX_POISON_TIMEOUT);
+
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
+
+ while (rtw_pci_rx_stale(rtwdev, skb)) {
+ if (!ktime_before(ktime_get(), deadline)) {
+ rtw_warn(rtwdev, "rx buffer still poisoned after %d ms\n",
+ RTW_PCI_RX_POISON_TIMEOUT);
+ return;
+ }
+ udelay(1);
+ dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
+ }
+}
+
static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
struct rtw_pci_rx_ring *rx_ring,
u32 idx, u32 desc_sz)
@@ -219,7 +311,8 @@ static int rtw_pci_reset_rx_desc(struct rtw_dev *rtwdev, struct sk_buff *skb,
if (!skb)
return -EINVAL;
- dma = dma_map_single(&pdev->dev, skb->data, buf_sz, DMA_FROM_DEVICE);
+ rtw_pci_rx_poison(skb);
+ dma = dma_map_single(&pdev->dev, skb->data, buf_sz, DMA_BIDIRECTIONAL);
if (dma_mapping_error(&pdev->dev, dma))
return -EBUSY;
@@ -241,7 +334,8 @@ static void rtw_pci_sync_rx_desc_device(struct rtw_dev *rtwdev, dma_addr_t dma,
struct rtw_pci_rx_buffer_desc *buf_desc;
int buf_sz = RTK_PCI_RX_BUF_SIZE;
- dma_sync_single_for_device(dev, dma, buf_sz, DMA_FROM_DEVICE);
+ rtw_pci_rx_poison(rx_ring->buf[idx]);
+ dma_sync_single_for_device(dev, dma, buf_sz, DMA_BIDIRECTIONAL);
buf_desc = (struct rtw_pci_rx_buffer_desc *)(rx_ring->r.head +
idx * desc_sz);
@@ -1065,8 +1159,7 @@ static u32 rtw_pci_rx_napi(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
rtw_pci_dma_check(rtwdev, ring, cur_rp);
skb = ring->buf[cur_rp];
dma = *((dma_addr_t *)skb->cb);
- dma_sync_single_for_cpu(rtwdev->dev, dma, RTK_PCI_RX_BUF_SIZE,
- DMA_FROM_DEVICE);
+ rtw_pci_rx_sync_and_wait_fresh(rtwdev, skb, dma);
rx_desc = skb->data;
ret = rtw_rx_query_rx_desc(rtwdev, rx_desc,
&pkt_stat, &rx_status);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread