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: 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