[PATCH v2] wifi: nxpwifi: validate monitor RX extra-header length

[Heyang Tan]

nxpwifi_recv_packet_to_monif() assumes that rxpd_extra_info fits
between the rxpd header and the start of the 802.11 frame.  It ensures
that rx_pkt_offset is at least sizeof(*rxpd), but the following check
only proves that sizeof(struct ieee80211_hdr) bytes starting at
rx_pkt_offset are present.

If a malformed descriptor sets RXPD_FLAG_EXTRA_HEADER with a
rx_pkt_offset below sizeof(*rxpd) + sizeof(ext), the 32-byte copy from
rxpd + 1 can extend past the descriptor-reported end of the frame.  For
example, when rx_pkt_offset is sizeof(*rxpd), skb->len can be only
sizeof(*rxpd) + sizeof(struct ieee80211_hdr), while the extra-header
copy reads through offset sizeof(*rxpd) + sizeof(ext) - 1.

Reject such descriptors before copying rxpd_extra_info.  A single check
next to the existing flag extraction also covers the HE copy site.

The normal skb allocation has enough slack that the over-read stays
within the allocation, so this is a defensive bounds fix.

Signed-off-by: Heyang Tan <thy15333007817@xxxxxxx>
---
 drivers/net/wireless/nxp/nxpwifi/util.c | 4 ++++
 1 file changed, 4 insertions(+)

Changes in v2:
- Use sizeof(*rxpd) + sizeof(ext) for the extra-header bound.
- Clarify the descriptor consistency requirement and the conditional over-read.

Hi Jeff,

Thanks for the review. My original wording was incomplete.

The new condition enforces the descriptor layout implied by
RXPD_FLAG_EXTRA_HEADER: rxpd_extra_info is immediately after rxpd, so
rx_pkt_offset must cover both structures. The existing validation only
proves that rx_pkt_offset is at least sizeof(*rxpd) and that the
802.11 header fits after rx_pkt_offset; it does not prove that the
32-byte region copied by memcpy() fits before rx_pkt_offset.

For a malformed descriptor with the extra-header flag set and
rx_pkt_offset smaller than sizeof(*rxpd) + sizeof(ext), the allowed
802.11 header length can therefore end before the copied structure. In
particular, when rx_pkt_offset equals sizeof(*rxpd), skb->len may end
at sizeof(*rxpd) + sizeof(struct ieee80211_hdr), while the copy reads
through offset sizeof(*rxpd) + sizeof(ext) - 1. For the 30-byte
ieee80211 header in this tree, the maximum logical over-read is two
bytes. The normal skb allocation has enough slack to mask this, which
is why I described it as a defensive consistency check.

A compliant device already satisfies the new condition, so valid
firmware behavior is unchanged.

I updated the check to use sizeof(*rxpd) + sizeof(ext), as suggested,
and reworded the commit message to describe the descriptor consistency
requirement and the conditional over-read explicitly.

Thanks,
Heyang

diff --git a/drivers/net/wireless/nxp/nxpwifi/util.c b/drivers/net/wireless/nxp/nxpwifi/util.c
index ae8444a14..a343df0c6 100644
--- a/drivers/net/wireless/nxp/nxpwifi/util.c
+++ b/drivers/net/wireless/nxp/nxpwifi/util.c
@@ -616,6 +616,10 @@ int nxpwifi_recv_packet_to_monif(struct nxpwifi_private *priv,
 
 	has_ext = rxpd->flags & RXPD_FLAG_EXTRA_HEADER;
 
+	if (has_ext &&
+	    rx_pkt_offset < sizeof(*rxpd) + sizeof(ext))
+		return -EINVAL;
+
 	if (has_ext)
 		memcpy((void *)&ext, (void *)(rxpd + 1), sizeof(ext));