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

[PATCH rtw-next v2 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 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.

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   | 22 ++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw88/fw.h   |  8 ++++++++
 drivers/net/wireless/realtek/rtw88/tx.c   |  3 +++
 4 files changed, 33 insertions(+), 3 deletions(-)


base-commit: 9b0be45d4d52fd223994e151f169743a97abd290
-- 
2.43.0

[PATCH rtw-next v2 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>
---
 drivers/net/wireless/realtek/rtw88/fw.c | 7 +++++++
 1 file changed, 7 insertions(+)

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

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

[Luka Gejak]

October 31, 2026 at 6:56 PM, "Huihui Huang" <hhhuang@smu.edu.sg> wrote:

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

Looking good to me.

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

Best regards,
Luka Gejak

[PATCH rtw-next v2 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.

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   | 15 +++++++++++++--
 drivers/net/wireless/realtek/rtw88/fw.h   |  8 ++++++++
 drivers/net/wireless/realtek/rtw88/tx.c   |  3 +++
 4 files changed, 26 insertions(+), 3 deletions(-)

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 and TX report paths 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..6bc684726669 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,9 @@ 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)
+		return;
+
 	c2h = get_c2h_from_skb(skb);
 	rc = GET_SCAN_REPORT_RETURN_CODE(c2h->payload);
 	aborted = rc != RTW_SCAN_REPORT_SUCCESS;
@@ -2436,6 +2444,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 v2 2/2] wifi: rtw88: check the C2H payload length before reading it

[Luka Gejak]

Hi Huihui,

Thanks for the v2. One comment below.

October 31, 2026 at 6:56 PM, "Huihui Huang" <hhhuang@smu.edu.sg> wrote:

> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
> index 0a909bf79cf5..6bc684726669 100644
> --- a/drivers/net/wireless/realtek/rtw88/fw.c
> +++ b/drivers/net/wireless/realtek/rtw88/fw.c

[ ... ]

> @@ -2381,6 +2386,9 @@ 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)
> +             return;
> +

As far as I can see, apart from the abort paths this report is the only
thing that completes a hw scan. So, a short report now leaves the scan
pending. RTW_FLAG_SCANNING stays set, scanning_vif is never cleared,
and I couldn't find anything in rtw88 that times the scan out.

Until something cancels it, e.g. a disconnect or the interface going
down, every new scan then fails with -EBUSY, since the old request is
still pending.

Before this patch the scan still completed, only with a return code read
past skb->len. Maybe it is better to complete it as aborted here instead
of dropping the report, something like:

        if (rtw_c2h_payload_len(skb) < 3) {
                rtw_hw_scan_complete(rtwdev, vif, true);
                return;
        }

Best regards,
Luka Gejak