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

[PATCH rtw-next] 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. With no
byte left after pkt_offset, c2h->id is read beyond skb->len and len
becomes 254; with one byte left, c2h->seq is beyond skb->len and len
becomes 255. 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 | 6 ++++++
 1 file changed, 6 insertions(+)

Tested on rtw-next (9b0be45d4d52) with a temporary handler-level KUnit
regression test (UML). Each case allocates a 64-byte skb, writes a C2H
at offset 24 (pkt_offset), and skb_put()s only part of it:

  1. skb->len == pkt_offset (no C2H byte), C2H_SCAN_RESULT, payload 0xab
  2. skb->len == pkt_offset + 1 (id only), same C2H
  3. skb->len == pkt_offset + 3 (valid C2H, one payload byte)
  4. C2H_WLAN_INFO passed directly to rtw_fw_c2h_cmd_handle(), as the
     c2h work does for queued skbs: first with skb->len == pkt_offset + 5
     (len 3), then with skb->len == pkt_offset + 1 (id only)

Cases 1-3 go through rtw_fw_c2h_cmd_rx_irqsafe(). Verbatim KUnit output:

  before this change:
    # rtw_c2h_short_packet_test: skb->len=pkt_offset scan_density=0xab (want 0x00)
    not ok 1 rtw_c2h_short_packet_test
    # rtw_c2h_one_byte_packet_test: skb->len=pkt_offset+1 scan_density=0xab (want 0x00)
    not ok 2 rtw_c2h_one_byte_packet_test
    # rtw_c2h_min_valid_packet_test: skb->len=pkt_offset+3 scan_density=0xab (want 0xab)
    ok 3 rtw_c2h_min_valid_packet_test
    # rtw_c2h_wl_info_len_check_test: skb->len=pkt_offset+5 fw_notify_cnt=0 (handler accepted no)
    # rtw_c2h_wl_info_len_check_test: skb->len=pkt_offset+1 fw_notify_cnt=1 wl_fw_dbg_info[7]=0x17 (handler accepted yes)
  after this change:
    # rtw_c2h_short_packet_test: skb->len=pkt_offset scan_density=0x00 (want 0x00)
    ok 1 rtw_c2h_short_packet_test
    # rtw_c2h_one_byte_packet_test: skb->len=pkt_offset+1 scan_density=0x00 (want 0x00)
    ok 2 rtw_c2h_one_byte_packet_test
    # rtw_c2h_min_valid_packet_test: skb->len=pkt_offset+3 scan_density=0xab (want 0xab)
    ok 3 rtw_c2h_min_valid_packet_test

Cases 1 and 2 show that incomplete C2H packets are processed and update
scan_density using bytes beyond skb->len before this change, and are
dropped after it; case 3 shows a valid C2H is still handled.

Case 4 bypasses the RX entry point to demonstrate the downstream
length-check bypass; it is not a before/after regression test. With
len 3 the "invalid wl info c2h length" WARN rejects the report, while
with the header cut after the id the wrapped len (255) passes the check
and wl_fw_dbg_info[] is filled from bytes beyond skb->len (0x17 is the
byte placed there). After this change such a packet is dropped in
rtw_fw_c2h_cmd_rx_irqsafe() and is not queued for
rtw_fw_c2h_cmd_handle(). The RA report path was not exercised.

Not tested on real hardware, and I have not observed such a short C2H
from a device; this hardens the driver against a malformed RX
descriptor.


base-commit: 9b0be45d4d52fd223994e151f169743a97abd290

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 945fedcd375b..2e37c304cde0 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -353,6 +353,12 @@ 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);
+		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] wifi: rtw88: drop C2H packets shorter than the C2H header

[Luka Gejak]

Hi Huihui,

I have two comments below, nothing major.

October 29, 2026 at 6:00 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 945fedcd375b..2e37c304cde0 100644
> --- a/drivers/net/wireless/realtek/rtw88/fw.c
> +++ b/drivers/net/wireless/realtek/rtw88/fw.c
> @@ -353,6 +353,12 @@ 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);

Small thing, skb->len here still includes the RX descriptor, so the value
printed is not the C2H length. Maybe print skb->len - pkt_offset instead,
so it shows how much of the C2H actually arrived?

> +             dev_kfree_skb_any(skb);
> +             return;
> +     }
> +
>       c2h = (struct rtw_c2h_cmd *)(skb->data + pkt_offset);
>       len = skb->len - pkt_offset - 2;

This takes care of the header, but as far as I can see a C2H with only
the two header bytes still gets through, with len 0. Most handlers read
their payload without looking at len, e.g. right below in this function:

        case C2H_SCAN_RESULT:
                complete(&rtwdev->fw_scan_density);
                rtw_fw_scan_result(rtwdev, c2h->payload, len);

and rtw_fw_scan_result() reads payload[0] unconditionally. So if I read
it right, skb->len == pkt_offset + 2 ends the same way as your test cases
1 and 2: scan_density is taken from the byte right after skb->len.

rtw_coex_info_response(), rtw_fw_c2h_cmd_handle_ext() and
rtw_tx_report_handle() seem to have the same problem. They read fixed
offsets in the payload and don't get len at all.

Would you be willing to add length checks to those handlers too? Maybe
as a separate patch, so this one stays small for stable.

Best regards,
Luka Gejak