RE: rtw88: 8822CE passes corrupted A-MSDUs as hw-decrypted, poisoning CCMP replay counter

[Bernie Huang <phhuang@realtek.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