[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

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

[Ping-Ke Shih]

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.

No need much commit message. Just say that you are validating C2H packet
length before reading.

> 
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM

This tag is no long necessary. 

> Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

As I requested to reword commit message, you can still take Luka's reviewed-by.

[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

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

[Ping-Ke Shih]

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. 

If you want to describe "previous patch", say it in cover-letter. Just
describe what you do in this patch.

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

You should think how to summarize these in subject... 

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

The detail of what the patch does isn't so useful for reviewers. LLM can
help to generate many stuffs (but not helpful). Please remove unnecessary
part. Reading long long commit message spends a lot of time.

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

Should we need these Fixes? I think the changes of this patch should not
happen. 

> Suggested-by: Luka Gejak <luka.gejak@linux.dev>
> Assisted-by: LLM

No this tag now.

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

If the c2h->id is SCAN already, validate 'len' in rtw_fw_scan_result().

I don't want to add condition which I think no possible happen.

> +                       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) {

The 3 is because of GET_SCAN_REPORT_RETURN_CODE(). How does people understand
what 3 means?

Maybe, we should define specific struct for status_report, like

struct status_report {
	u8 id;
	u8 seq; 
	u8 rsvd[2]
	u8 retrun_code;
};

struct status_report *status_report = get_c2h_from_skb(skb);

if (rtw_c2h_payload_len(skb) < sizeof(*status_report))


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

ditto. This is ever harder to know why here is 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))

Ditto. 7 and 10.... very hard to understand. 

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

Hi Ping-Ke,

Just one comment on the struct idea.

November 8, 2026 at 3:56 AM, "Ping-Ke Shih" <pkshih@realtek.com> wrote:

> 
> Huihui Huang <hhhuang@smu.edu.sg> wrote:
> 
>>
>> +    if (rtw_c2h_payload_len(skb) < 3) {
>
> The 3 is because of GET_SCAN_REPORT_RETURN_CODE(). How does people understand
> what 3 means?
>
> Maybe, we should define specific struct for status_report, like
>
> struct status_report {
>     u8 id;
>     u8 seq;
>     u8 rsvd[2]
>     u8 retrun_code;
> };
>
> struct status_report *status_report = get_c2h_from_skb(skb);
>
> if (rtw_c2h_payload_len(skb) < sizeof(*status_report))

Agreed, a struct is much easier to follow than the bare numbers. One
small thing though. This struct starts at the C2H header, but
rtw_c2h_payload_len() leaves the two header bytes out. So as far as I can
see the check would ask for 5 payload bytes while only 3 are read, and a
valid report with 3 or 4 payload bytes would be treated as too short.

Maybe the struct could cover only the payload, like struct rtw_c2h_ra_rpt
and struct rtw_c2h_adaptivity already do:

        struct rtw_c2h_scan_status_rpt {
                u8 rsvd[2];
                u8 return_code;
        } __packed;

Then it compares with rtw_c2h_payload_len() directly, and the same would
work for the channel switch report and the two TX report formats.

Best regards,
Luka Gejak

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

[Ping-Ke Shih]

Luka Gejak <luka.gejak@linux.dev> wrote:
> Hi Ping-Ke,
> 
> Just one comment on the struct idea.
> 
> November 8, 2026 at 3:56 AM, "Ping-Ke Shih" <pkshih@realtek.com> wrote:
> 
> >
> > Huihui Huang <hhhuang@smu.edu.sg> wrote:
> >
> >>
> >> +    if (rtw_c2h_payload_len(skb) < 3) {
> >
> > The 3 is because of GET_SCAN_REPORT_RETURN_CODE(). How does people understand
> > what 3 means?
> >
> > Maybe, we should define specific struct for status_report, like
> >
> > struct status_report {
> >     u8 id;
> >     u8 seq;
> >     u8 rsvd[2]
> >     u8 retrun_code;
> > };
> >
> > struct status_report *status_report = get_c2h_from_skb(skb);
> >
> > if (rtw_c2h_payload_len(skb) < sizeof(*status_report))
> 
> Agreed, a struct is much easier to follow than the bare numbers. One
> small thing though. This struct starts at the C2H header, but
> rtw_c2h_payload_len() leaves the two header bytes out. So as far as I can
> see the check would ask for 5 payload bytes while only 3 are read, and a
> valid report with 3 or 4 payload bytes would be treated as too short.
> 
> Maybe the struct could cover only the payload, like struct rtw_c2h_ra_rpt
> and struct rtw_c2h_adaptivity already do:
> 
>         struct rtw_c2h_scan_status_rpt {
>                 u8 rsvd[2];
>                 u8 return_code;
>         } __packed;
> 
> Then it compares with rtw_c2h_payload_len() directly, and the same would
> work for the channel switch report and the two TX report formats.

I just don't want magic number 3, 5 and etc. That's very hard to people
to understand. No strict opinion about the struct.