[PATCH 0/3] wifi: ipw2x00: fix management frame length handling

[PATCH 0/3] wifi: ipw2x00: fix management frame length handling

[Shmulik Cohen]

The libipw management receive helpers derive the information element
length by subtracting a fixed structure size from the reported frame
length:

	stats->len - sizeof(*beacon)	/* libipw_rx.c:1300 */
	stats->len - sizeof(*frame)	/* libipw_rx.c:1240 */

stats->len is a u16 and sizeof() has type size_t, so each subtraction is
evaluated as size_t and wraps instead of going negative. Truncated to
the u16 length parameter of libipw_parse_info_param(), a frame shorter
than its own fixed fields becomes a length near 64 KiB, and the parser
walks the receive buffer as if it held that many bytes of information
elements.

Patches 1 and 2 add the missing checks in libipw itself, so neither
handler touches its fixed fields or derives an element length from a
frame too short to contain them, whatever the caller passes.

Patch 3 bounds the reported length from above in both drivers. Neither
management path had an upper bound: ipw2100_corruption_check() does not
inspect frame_size for management frames, and ipw_rx() only rejects a
frame shorter than the header length.

All of the lengths involved are reported by the device, so per
Documentation/process/threat-model.rst this series is a set of
robustness fixes rather than a vulnerability report. I am not claiming
otherwise, and I have no evidence that any particular firmware reports
a management frame length below the fixed fields; the checks are cheap
and the arithmetic is wrong regardless of who supplies the length.

Verification
============

Built and run on arm64 under QEMU at f5bbbfec59b4e ("Merge tag
'probes-fixes-v7.2-rc7' of git://git.kernel.org/pub/scm/linux/kernel/
git/trace/linux-trace") with CONFIG_IPW2100=y, CONFIG_LIBIPW=y,
CONFIG_KUNIT=y, CONFIG_KASAN=y, CONFIG_KASAN_GENERIC=y and
CONFIG_KALLSYMS_ALL=y.

Before patches 1 and 2, KUnit cases that call the two handlers with a
2340 byte allocation, which is IPW_RX_NIC_BUFFER_LENGTH, and a reported
length of 24 report:

  BUG: KASAN: slab-out-of-bounds in libipw_parse_info_param+0x100/0xee0
  Read of size 1 at addr ffff0000021a092e by task kunit_try_catch/33
   libipw_parse_info_param+0x100/0xee0
   libipw_process_probe_response+0x354/0xc08
  The buggy address is located 10 bytes to the right of
   allocated 2340-byte region [ffff0000021a0000, ffff0000021a0924)

  BUG: KASAN: slab-out-of-bounds in libipw_parse_info_param+0x100/0xee0
   libipw_parse_info_param+0x100/0xee0
   libipw_handle_assoc_resp+0x2e8/0x3e4
  The buggy address is located 4 bytes to the right of
   allocated 2340-byte region

After patches 1 and 2 both cases pass under KASAN. The KUnit cases
exercise static functions and are not proposed for merging, so they are
not included here; I can send them on request.

Patch 3 is compile-tested only. Both drivers and CONFIG_IPW2200_QOS were
enabled and the driver directory rebuilt at W=1 with no new warnings
relative to the unpatched tree.

What was not done
=================

I do not have ipw2100 or ipw2200 hardware, so nothing here is tested on
a real device, and patch 3 in particular has no runtime test. The
reproducers are KUnit cases that call the handlers directly with the
lengths the drivers can pass them.

An unrelated observation while tracing these paths, in case it is of
interest: the CONFIG_IPW2200_QOS block in ipw_rx_notification()
(ipw2200.c:4470) looks unreachable. It is entered only under
case CMAS_ASSOCIATED, which establishes that notif->u.raw[0] is the
state byte, value 12, and it then tests
IPW_GET_PACKET_STYPE(&notif->u.raw) against IEEE80211_STYPE_ASSOC_RESP.
That masks the first byte with 0x00f0, giving 0x0000 rather than 0x0010,
so the two predicates are mutually exclusive and libipw_rx_mgt() is
never called there. The frame and its length look like they were meant
to start after the state byte. I have not sent a patch for it because I
cannot test the intended behaviour without the hardware.

Tooling
=======

Per Documentation/process/generated-content.rst: the defects were found
with AI assistance (Claude, claude-opus-5) during a review of length
arithmetic in kernel management frame parsers, prompted to look for
subtractions of a fixed header size from an unvalidated on-the-wire
length. The tool identified the call sites and the truncation, drafted
these patches and the KUnit cases, and ran the KASAN and W=1 builds. A
second model was used adversarially to attack the result; it refuted an
earlier fourth patch and an earlier version of patch 3, both of which
were dropped, and every remaining claim was rechecked against the
source by hand. checkpatch.pl --strict reports no errors, warnings or
checks on any patch in the series.

Shmulik Cohen (3):
  wifi: libipw: reject too-short beacon and probe responses
  wifi: libipw: reject too-short association responses
  wifi: ipw2x00: bound management frame length to the receive buffer

 drivers/net/wireless/intel/ipw2x00/ipw2100.c   | 4 +++-
 drivers/net/wireless/intel/ipw2x00/ipw2200.c   | 9 +++++++++
 drivers/net/wireless/intel/ipw2x00/libipw_rx.c | 6 ++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.50.1 (Apple Git-155)

[PATCH 1/3] wifi: libipw: reject too-short beacon and probe responses

[Shmulik Cohen]

libipw_process_probe_response() and the libipw_network_init() call it
makes assume the frame contains the full 36-byte beacon and probe
response prefix, but the ipw2100 and ipw2200 receive paths only
establish that a management frame carries the generic 24-byte
three-address header.

libipw_network_init() then computes the information element length as

	stats->len - sizeof(*beacon)

stats->len is a u16 and sizeof() has type size_t, so the subtraction is
evaluated as size_t and wraps instead of going negative.  Truncating
that to the u16 length parameter of libipw_parse_info_param() yields
65524 for a 24-byte beacon, and the parser then walks the receive
buffer as if it held almost 64 KiB of information elements, reading
past the allocation.

Reject the frame before any fixed field is touched.

Found by an AI-assisted review of length arithmetic in management frame
parsers.  Verified with a KUnit case under Generic KASAN on arm64 under
QEMU; I do not have the hardware, so it is not tested on a real device.

Fixes: b453872c35cf ("[NET] ieee80211 subsystem")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Shmulik Cohen <anuk909@gmail.com>
---
 drivers/net/wireless/intel/ipw2x00/libipw_rx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
index c8841f9b9ad9..2661dac6985e 100644
--- a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
+++ b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
@@ -1421,6 +1421,9 @@ static void libipw_process_probe_response(struct libipw_device
 #endif
 	unsigned long flags;
 
+	if (stats->len < sizeof(*beacon))
+		return;
+
 	LIBIPW_DEBUG_SCAN("'%*pE' (%pM): %c%c%c%c %c%c%c%c-%c%c%c%c %c%c%c%c\n",
 		     info_element->len, info_element->data,
 		     beacon->header.addr3,
-- 
2.50.1 (Apple Git-155)

[PATCH 2/3] wifi: libipw: reject too-short association responses

[Shmulik Cohen]

libipw_handle_assoc_resp() reads the capability, status and aid fields
of the 30-byte association response prefix and then computes the
information element length as

	stats->len - sizeof(*frame)

stats->len is a u16 and sizeof() has type size_t, so the subtraction is
evaluated as size_t and wraps instead of going negative.  Truncating
that to the u16 length parameter of libipw_parse_info_param() turns a
frame shorter than the fixed fields into a length near 64 KiB, and the
parser then reads past the receive buffer.

Both the ipw2100 and ipw2200 management receive paths reach this
function having established only that the frame carries the generic
24-byte three-address header.

Reject the frame before any fixed field is touched.

Found by an AI-assisted review of length arithmetic in management frame
parsers.  Verified with a KUnit case under Generic KASAN on arm64 under
QEMU; I do not have the hardware, so it is not tested on a real device.

Fixes: 9e8571affd1c ("[PATCH] ieee80211: Add QoS (WME) support to the ieee80211 subsystem")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Shmulik Cohen <anuk909@gmail.com>
---
 drivers/net/wireless/intel/ipw2x00/libipw_rx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
index 2661dac6985e..424349a6935e 100644
--- a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
+++ b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
@@ -1209,6 +1209,9 @@ static int libipw_handle_assoc_resp(struct libipw_device *ieee, struct libipw_as
 	struct libipw_network *network = &network_resp;
 	struct net_device *dev = ieee->dev;
 
+	if (stats->len < sizeof(*frame))
+		return 1;
+
 	network->flags = 0;
 	network->qos_data.active = 0;
 	network->qos_data.supported = 0;
-- 
2.50.1 (Apple Git-155)

[PATCH 3/3] wifi: ipw2x00: bound management frame length to the receive buffer

[Shmulik Cohen]

Both management receive paths establish a lower bound on the frame
length and no upper bound, even though the length originates from the
device.

ipw2100_corruption_check() returns 0 without inspecting frame_size for
management frames, and __ipw2100_rx_process() only rejects a frame
smaller than the three-address header, so any reported size up to the
u32 limit reaches libipw_rx_mgt() against a receive allocation of
IPW_RX_NIC_BUFFER_LENGTH bytes.  Check frame_size itself rather than
stats.len, which is a u16: a size of 65566 truncates to 30 on
assignment and would pass a check made afterwards.

ipw_rx() likewise only rejects a frame shorter than the header length.
Bound it against the DMA mapped receive buffer.  The size passed to
alloc_skb() is rounded up by the allocator, so skb_tailroom() can
exceed IPW_RX_BUF_SIZE and is not a usable bound here; the existing
uses of that idiom in the data paths are too permissive for the same
reason.

libipw then hands the remainder to libipw_parse_info_param(), which
walks information elements for as long as the length allows, so an
over-long reported length reads past the receive buffer without any
wraparound being involved.

The length is device-reported, so per
Documentation/process/threat-model.rst this is a robustness fix rather
than a vulnerability.

Found by an AI-assisted review of length arithmetic in management frame
parsers.  Compile-tested only for these two hunks; I do not have the
hardware, so they are not tested on a real device.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Shmulik Cohen <anuk909@gmail.com>
---
 drivers/net/wireless/intel/ipw2x00/ipw2100.c | 4 +++-
 drivers/net/wireless/intel/ipw2x00/ipw2200.c | 9 +++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2100.c b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
index 2b8a23865bfb..43b4e432956b 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
@@ -2712,7 +2712,9 @@ static void __ipw2100_rx_process(struct ipw2100_priv *priv)
 				break;
 			}
 #endif
-			if (stats.len < sizeof(struct libipw_hdr_3addr))
+			if (sq->drv[i].frame_size <
+				    sizeof(struct libipw_hdr_3addr) ||
+			    sq->drv[i].frame_size > IPW_RX_NIC_BUFFER_LENGTH)
 				break;
 			switch (WLAN_FC_GET_TYPE(le16_to_cpu(u->rx_data.header.frame_ctl))) {
 			case IEEE80211_FTYPE_MGMT:
diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
index 4bc9bb406e8e..8249d493ee22 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
@@ -8322,6 +8322,15 @@ static void ipw_rx(struct ipw_priv *priv)
 					break;
 				}
 
+				if (unlikely(le16_to_cpu(pkt->u.frame.length) >
+					     IPW_RX_BUF_SIZE -
+					     IPW_RX_FRAME_SIZE)) {
+					IPW_DEBUG_DROP("Received oversized packet. Dropping.\n");
+					priv->net_dev->stats.rx_errors++;
+					priv->wstats.discard.misc++;
+					break;
+				}
+
 				switch (WLAN_FC_GET_TYPE
 					(le16_to_cpu(header->frame_ctl))) {
 
-- 
2.50.1 (Apple Git-155)