Re: [PATCH 2/3] wifi: cfg80211: validate assoc response length before status and IE access
From: Zhao Li
Date: Mon Jul 06 2026 - 23:18:16 EST
On Mon, 2026-07-06 at 12:39 +0200, Johannes Berg wrote:
> What makes you claim "unsafe"?
"Unsafe" as in accessed before data->len is validated against the
fixed-field layout, not as in known-exploitable on valid traffic. In
the original struct initialiser, unconditionally:
- .status = le16_to_cpu(mgmt->u.assoc_resp.status_code) reads two bytes
at offset 26, needing data->len >= 28;
- .resp_ie = mgmt->u.assoc_resp.variable points at offset 30 (non-S1G);
- .resp_ie_len = data->len - 30 is a size_t, so it underflows to near
SIZE_MAX when data->len < 30.
resp_ie/resp_ie_len are not a harmless local intermediate: they are
consumed by __cfg80211_connect_result() and nl80211_send_connect_result(),
including the kzalloc() sizing and the memcpy()/nla_put() of resp_ie_len
bytes. So the underflow can feed a later copy/nla_put length rather than
remaining a harmless local value.
The mac80211 caller is fine: ieee80211_rx_mgmt_assoc_resp() gates on
len < 24 + 6 (== offsetof(..., u.assoc_resp.variable)) first. The
mwifiex handoff to cfg80211_rx_assoc_resp() only checks that assoc_rsp_size
is non-zero.
For S1G the IE offset is 28, not 30; the old code filled the regular
assoc-response fields first and only patched resp_ie later once it saw
S1G. v2 makes both layouts explicit and validates data->len before
assigning the fields.
If "unsafe" is too loaded here, I can reword it to "unvalidated" or
"unchecked" in a v3.
Thanks,
Zhao Li