[PATCH rtw-next v3 0/2] wifi: rtw88: check C2H header and payload lengths

[PATCH rtw-next v3 0/2] wifi: rtw88: check C2H header and payload lengths

[Huihui Huang]

Patch 1 drops C2H packets that are too short to hold the C2H header.
Patch 2 adds payload length checks to the handlers identified by Luka:
scan results, coexistence responses, HALMAC extended events and TX
reports. The header check remains a separate patch to keep the stable
backport small.

Other handlers, including rtw_coex_bt_info_notify(),
rtw_coex_bt_hid_info_notify(), rtw_fw_bcn_filter_notify(),
rtw_fw_adaptivity_result() and the GET_COEX_RESP_*() users of the
queued coexistence response, require a separate payload-length audit
and are outside the scope of this series.

Changes in v3:
- Patch 1: add Luka's Reviewed-by.
- Patch 2: complete the HW scan as aborted when the scan status report
  is too short, instead of leaving the scan pending (Luka).

Changes in v2:
- Patch 1: print the C2H length instead of the full skb length in the
  debug message, as suggested by Luka.
- Patch 1: clarify the description of accesses beyond skb->len.
- Patch 2: add payload length checks in response to Luka's review.

v2: https://lore.kernel.org/r/20261031175621.2228762-1-hhhuang@smu.edu.sg
v1: https://lore.kernel.org/r/20261029170017.832708-1-hhhuang@smu.edu.sg

Huihui Huang (2):
  wifi: rtw88: drop C2H packets shorter than the C2H header
  wifi: rtw88: check the C2H payload length before reading it

 drivers/net/wireless/realtek/rtw88/coex.c |  3 ++-
 drivers/net/wireless/realtek/rtw88/fw.c   | 24 +++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw88/fw.h   |  8 ++++++++
 drivers/net/wireless/realtek/rtw88/tx.c   |  3 +++
 4 files changed, 35 insertions(+), 3 deletions(-)


base-commit: 9b0be45d4d52fd223994e151f169743a97abd290
-- 
2.43.0

[PATCH rtw-next v3 1/2] wifi: rtw88: drop C2H packets shorter than the C2H header

[Huihui Huang]

rtw_fw_c2h_cmd_rx_irqsafe() and rtw_fw_c2h_cmd_handle() compute the C2H
payload length as

	len = skb->len - pkt_offset - 2;

where len is a u8, without checking that the skb holds the 2-byte C2H
header (id and seq).

If the header is incomplete, the C2H payload length underflows: zero
bytes after pkt_offset produce len = 254, and one byte produces
len = 255. With zero bytes, reading c2h->id also accesses data beyond
skb->len. The wrapped len defeats the minimum-length checks in
rtw_fw_ra_report_handle() and rtw_coex_wl_fwdbginfo_notify(), which then
read their fixed-size reports beyond the valid packet data.

skb->len is pkt_offset plus pkt_len from the RX descriptor, and
rtw_rx_query_rx_desc() returns for C2H packets before its
"pkt_len <= FCS_LEN" check, so a short C2H packet is not rejected there.

Drop C2H packets that are too short to hold the C2H header. Since
rtw_fw_c2h_cmd_handle() only sees skbs queued by
rtw_fw_c2h_cmd_rx_irqsafe(), this covers both computations.

Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
---
 drivers/net/wireless/realtek/rtw88/fw.c | 7 +++++++
 1 file changed, 7 insertions(+)

v3: add Luka's Reviewed-by; no code change.
v2: print skb->len - pkt_offset in the debug message (Luka).

Tested on rtw-next with the temporary KUnit test from v1, with the same
results: before this change, C2H packets cut before or inside the
header update scan_density from bytes beyond skb->len; after it they
are dropped, and a valid C2H is still handled. Not tested on real
hardware.

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 945fedcd375b..0a909bf79cf5 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -353,6 +353,13 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset,
 	struct rtw_c2h_cmd *c2h;
 	u8 len;
 
+	if (unlikely(skb->len < pkt_offset + sizeof(*c2h))) {
+		rtw_dbg(rtwdev, RTW_DBG_FW, "C2H too short: %u\n",
+			skb->len - pkt_offset);
+		dev_kfree_skb_any(skb);
+		return;
+	}
+
 	c2h = (struct rtw_c2h_cmd *)(skb->data + pkt_offset);
 	len = skb->len - pkt_offset - 2;
 	*((u32 *)skb->cb) = pkt_offset;
-- 
2.43.0

[PATCH rtw-next v3 2/2] wifi: rtw88: check the C2H payload length before reading it

[Huihui Huang]

With the previous patch, a C2H packet holds at least the 2-byte C2H
header, but its payload can still be shorter than what the handlers
read. These handlers read fixed payload offsets without checking the
payload length:

 - rtw_fw_scan_result() reads payload[0];
 - rtw_coex_info_response() reads payload[0];
 - rtw_fw_c2h_cmd_handle_ext() reads the sub-command ID in payload[0],
   and rtw_hw_scan_status_report() and rtw_hw_scan_chan_switch() read
   up to payload[2] and payload[4];
 - rtw_tx_report_handle() reads up to payload[6] for C2H_CCX_TX_RPT and
   payload[9] for C2H_CCX_RPT.

With a short payload, these reads are beyond skb->len.

Add rtw_c2h_payload_len() to get the payload length of a C2H skb, and
ignore C2H packets whose payload is too short for the fields the
handler reads. For C2H_SCAN_RESULT, check the length before completing
fw_scan_density, so that a report without payload does not wake the
waiter. A short HW scan status report is not ignored but completes the
scan as aborted; otherwise the scan stays pending until something
aborts it.

For coexistence responses, this only validates the byte inspected by
rtw_coex_info_response(). Length validation for the queued response's
GET_COEX_RESP_*() consumers is left for a separate follow-up.

Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Fixes: 4136214f7c46 ("rtw88: add BT co-existence support")
Fixes: a853d234e179 ("rtw88: notify fw when driver in scan-period to avoid potential problem")
Fixes: 10d162b2ed39 ("rtw88: 8822c: add ieee80211_ops::hw_scan")
Suggested-by: Luka Gejak <luka.gejak@linux.dev>
Assisted-by: LLM
Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
---
 drivers/net/wireless/realtek/rtw88/coex.c |  3 ++-
 drivers/net/wireless/realtek/rtw88/fw.c   | 17 +++++++++++++++--
 drivers/net/wireless/realtek/rtw88/fw.h   |  8 ++++++++
 drivers/net/wireless/realtek/rtw88/tx.c   |  3 +++
 4 files changed, 28 insertions(+), 3 deletions(-)

v3: complete the HW scan as aborted on a short scan status report
    instead of ignoring it (Luka).
v2: new patch (Luka).

Tested on rtw-next on top of patch 1 with a temporary KUnit test (UML).
C2H packets with the 2-byte header and 0 or 1 payload byte are passed
to rtw_fw_c2h_cmd_rx_irqsafe(); the byte after the header is set to a
value the handler would act on. Verbatim KUnit output:

  before this change:
    # rtw_c2h_scan_result_no_payload_test: payload_len=0 scan_density=0xab completed=1 (want 0x00, 0)
    not ok 1 rtw_c2h_scan_result_no_payload_test
    # rtw_c2h_scan_result_one_byte_test: payload_len=1 scan_density=0xab completed=1 (want 0xab, 1)
    ok 2 rtw_c2h_scan_result_one_byte_test
    # rtw_c2h_coex_resp_no_payload_test: payload_len=0 coex queue len=1 (want 0)
    not ok 3 rtw_c2h_coex_resp_no_payload_test
    # rtw_c2h_coex_resp_one_byte_test: payload_len=1 coex queue len=1 (want 1)
    ok 4 rtw_c2h_coex_resp_one_byte_test
  after this change:
    # rtw_c2h_scan_result_no_payload_test: payload_len=0 scan_density=0x00 completed=0 (want 0x00, 0)
    ok 1 rtw_c2h_scan_result_no_payload_test
    # rtw_c2h_scan_result_one_byte_test: payload_len=1 scan_density=0xab completed=1 (want 0xab, 1)
    ok 2 rtw_c2h_scan_result_one_byte_test
    # rtw_c2h_coex_resp_no_payload_test: payload_len=0 coex queue len=0 (want 0)
    ok 3 rtw_c2h_coex_resp_no_payload_test
    # rtw_c2h_coex_resp_one_byte_test: payload_len=1 coex queue len=1 (want 1)
    ok 4 rtw_c2h_coex_resp_one_byte_test

The HALMAC paths, including the HW scan status report change in v3,
and the TX report path were only compile-tested. Not tested on real
hardware.

diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c
index 37c336def419..e2c91efc529d 100644
--- a/drivers/net/wireless/realtek/rtw88/coex.c
+++ b/drivers/net/wireless/realtek/rtw88/coex.c
@@ -643,7 +643,8 @@ void rtw_coex_info_response(struct rtw_dev *rtwdev, struct sk_buff *skb)
 	struct rtw_coex *coex = &rtwdev->coex;
 	u8 *payload = get_payload_from_coex_resp(skb);
 
-	if (payload[0] != COEX_RESP_ACK_BY_WL_FW) {
+	if (rtw_c2h_payload_len(skb) < 1 ||
+	    payload[0] != COEX_RESP_ACK_BY_WL_FW) {
 		dev_kfree_skb_any(skb);
 		return;
 	}
diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 0a909bf79cf5..18d3689054e6 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -96,6 +96,9 @@ static void rtw_fw_c2h_cmd_handle_ext(struct rtw_dev *rtwdev,
 	struct rtw_c2h_cmd *c2h;
 	u8 sub_cmd_id;
 
+	if (rtw_c2h_payload_len(skb) < 1)
+		return;
+
 	c2h = get_c2h_from_skb(skb);
 	sub_cmd_id = c2h->payload[0];
 
@@ -376,8 +379,10 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset,
 		dev_kfree_skb_any(skb);
 		break;
 	case C2H_SCAN_RESULT:
-		complete(&rtwdev->fw_scan_density);
-		rtw_fw_scan_result(rtwdev, c2h->payload, len);
+		if (len) {
+			complete(&rtwdev->fw_scan_density);
+			rtw_fw_scan_result(rtwdev, c2h->payload, len);
+		}
 		dev_kfree_skb_any(skb);
 		break;
 	default:
@@ -2381,6 +2386,11 @@ void rtw_hw_scan_status_report(struct rtw_dev *rtwdev, struct sk_buff *skb)
 	if (!test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
 		return;
 
+	if (rtw_c2h_payload_len(skb) < 3) {
+		rtw_hw_scan_complete(rtwdev, vif, true);
+		return;
+	}
+
 	c2h = get_c2h_from_skb(skb);
 	rc = GET_SCAN_REPORT_RETURN_CODE(c2h->payload);
 	aborted = rc != RTW_SCAN_REPORT_SUCCESS;
@@ -2436,6 +2446,9 @@ void rtw_hw_scan_chan_switch(struct rtw_dev *rtwdev, struct sk_buff *skb)
 	if (!test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
 		return;
 
+	if (rtw_c2h_payload_len(skb) < 5)
+		return;
+
 	c2h = get_c2h_from_skb(skb);
 	chan = GET_CHAN_SWITCH_CENTRAL_CH(c2h->payload);
 	id = GET_CHAN_SWITCH_ID(c2h->payload);
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index 48ad9ceab6ea..6713dfac8c94 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -810,6 +810,14 @@ static inline struct rtw_c2h_cmd *get_c2h_from_skb(struct sk_buff *skb)
 	return (struct rtw_c2h_cmd *)(skb->data + pkt_offset);
 }
 
+/* The C2H header has been validated by rtw_fw_c2h_cmd_rx_irqsafe(). */
+static inline u32 rtw_c2h_payload_len(struct sk_buff *skb)
+{
+	u32 pkt_offset = *((u32 *)skb->cb);
+
+	return skb->len - pkt_offset - sizeof(struct rtw_c2h_cmd);
+}
+
 static inline bool rtw_fw_feature_check(struct rtw_fw_state *fw,
 					enum rtw_fw_feature feature)
 {
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index 701ed7c4f3f7..ddfddd7cb4db 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -241,6 +241,9 @@ void rtw_tx_report_handle(struct rtw_dev *rtwdev, struct sk_buff *skb, int src)
 	u8 sn, st;
 	u8 *n;
 
+	if (rtw_c2h_payload_len(skb) < (src == C2H_CCX_TX_RPT ? 7 : 10))
+		return;
+
 	c2h = get_c2h_from_skb(skb);
 
 	if (src == C2H_CCX_TX_RPT) {
-- 
2.43.0

Re: [PATCH rtw-next v3 2/2] wifi: rtw88: check the C2H payload length before reading it

[Luka Gejak]

November 5, 2026 at 1:59 PM, "Huihui Huang" <hhhuang@smu.edu.sg> wrote:

> 
> With the previous patch, a C2H packet holds at least the 2-byte C2H
> header, but its payload can still be shorter than what the handlers
> read. These handlers read fixed payload offsets without checking the
> payload length:
> 
>  - rtw_fw_scan_result() reads payload[0];
>  - rtw_coex_info_response() reads payload[0];
>  - rtw_fw_c2h_cmd_handle_ext() reads the sub-command ID in payload[0],
>  and rtw_hw_scan_status_report() and rtw_hw_scan_chan_switch() read
>  up to payload[2] and payload[4];
>  - rtw_tx_report_handle() reads up to payload[6] for C2H_CCX_TX_RPT and
>  payload[9] for C2H_CCX_RPT.
> 
> With a short payload, these reads are beyond skb->len.
> 
> Add rtw_c2h_payload_len() to get the payload length of a C2H skb, and
> ignore C2H packets whose payload is too short for the fields the
> handler reads. For C2H_SCAN_RESULT, check the length before completing
> fw_scan_density, so that a report without payload does not wake the
> waiter. A short HW scan status report is not ignored but completes the
> scan as aborted; otherwise the scan stays pending until something
> aborts it.
> 
> For coexistence responses, this only validates the byte inspected by
> rtw_coex_info_response(). Length validation for the queued response's
> GET_COEX_RESP_*() consumers is left for a separate follow-up.
> 
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Fixes: 4136214f7c46 ("rtw88: add BT co-existence support")
> Fixes: a853d234e179 ("rtw88: notify fw when driver in scan-period to avoid potential problem")
> Fixes: 10d162b2ed39 ("rtw88: 8822c: add ieee80211_ops::hw_scan")
> Suggested-by: Luka Gejak <luka.gejak@linux.dev>
> Assisted-by: LLM
> Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
> ---

Looking good to me,

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak