[PATCH 1/2] wifi: nxpwifi: fix the authentication frame length handling
Linmao Li <[email protected]>
| Newsgroups | org.kernel.vger.linux-wireless,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
nxpwifi_cfg80211_authenticate() builds a 3-address management frame in a
temporary buffer and hands it to nxpwifi_form_mgmt_frame(), which copies
it into the skb and splices in address4. The frame length is tracked in
a single u16 that conflates the two lengths, and it gets both wrong.
Truncation. req->ie_len and req->auth_data_len are both size_t. The IE
policy caps req->ie_len at IEEE80211_MAX_DATA_LEN, but
NL80211_ATTR_AUTH_DATA only has a four-byte minimum length policy. Since
nla_len is a u16, a single authentication data attribute can carry up to
65531 bytes, enough for the combined frame length to exceed U16_MAX
before it is assigned to pkt_len. With auth_data_len == 65510 and no IEs
the length wraps to 6, kzalloc(6) succeeds, and the memcpy() below then
writes 65506 user-provided bytes past a 6-byte heap object.
Geometry. NXPWIFI_MGMT_HEADER_LEN is 30, which is the 24-byte 3-address
header plus the address4 that the firmware expects. The temporary buffer
only holds a struct ieee80211_hdr_3addr, so six of the bytes accounted
for are never used, and passing that length to nxpwifi_form_mgmt_frame()
makes it append ETH_ALEN on top of a length that already included it.
The skb is sized without those six bytes, so the last skb_put_data()
overruns the tailroom by exactly ETH_ALEN. It usually goes unnoticed
because SKB_DATA_ALIGN() rounding leaves slack. The driver's other
caller of the helper, nxpwifi_cfg80211_mgmt_tx(), adds ETH_ALEN to the
skb size before allocating and passes the 3-address length, which is what
this path should do too.
Keep the two lengths apart. frame_len is the 3-address frame the driver
builds: it sizes the temporary buffer and is what the helper is told.
pkt_len is what the firmware sees, frame_len plus address4: it sizes the
skb and goes into tx_info. Computing frame_len in size_t and rejecting
anything that cannot be represented once ETH_ALEN is added removes the
wrap.
The value handed to the firmware is unchanged, so this is not a
behavioural change for frames that were already valid. A tighter 802.11
bound may make sense, but that depends on the firmware's frame format and
is a separate decision.
Fixes: 73b01e57ed3e ("wifi: nxp: add nxpwifi driver for IW61x")
Signed-off-by: Linmao Li <[email protected]>
---
drivers/net/wireless/nxp/nxpwifi/cfg80211.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/nxp/nxpwifi/cfg80211.c b/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
index 5cc8cdf594d3e..1f46e4f0157e7 100644
--- a/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
+++ b/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
@@ -3323,6 +3323,7 @@ nxpwifi_cfg80211_authenticate(struct wiphy *wiphy,
struct nxpwifi_adapter *adapter = priv->adapter;
struct sk_buff *skb;
u16 pkt_len, auth_alg;
+ size_t frame_len;
int ret;
struct ieee80211_mgmt *mgmt;
struct nxpwifi_txinfo *tx_info;
@@ -3396,13 +3397,23 @@ nxpwifi_cfg80211_authenticate(struct wiphy *wiphy,
nxpwifi_cancel_scan(adapter);
- pkt_len = (u16)req->ie_len + req->auth_data_len +
- NXPWIFI_MGMT_HEADER_LEN + NXPWIFI_AUTH_BODY_LEN;
+ frame_len = req->ie_len + req->auth_data_len +
+ sizeof(struct ieee80211_hdr_3addr) + NXPWIFI_AUTH_BODY_LEN;
if (req->auth_data_len >= 4)
- pkt_len -= 4;
+ frame_len -= 4;
- mgmt = kzalloc(pkt_len, GFP_KERNEL);
+ /* nxpwifi_form_mgmt_frame() inserts address4, so the frame handed to
+ * the firmware is ETH_ALEN longer than the one built here.
+ */
+ if (frame_len > U16_MAX - ETH_ALEN) {
+ nxpwifi_dbg(adapter, ERROR,
+ "auth frame too long: %zu bytes\n", frame_len);
+ return -EINVAL;
+ }
+ pkt_len = frame_len + ETH_ALEN;
+
+ mgmt = kzalloc(frame_len, GFP_KERNEL);
skb = dev_alloc_skb(NXPWIFI_MIN_DATA_HEADER_LEN +
NXPWIFI_MGMT_FRAME_HEADER_SIZE +
@@ -3448,7 +3459,7 @@ nxpwifi_cfg80211_authenticate(struct wiphy *wiphy,
memcpy((u8 *)varptr, req->ie, req->ie_len);
}
- nxpwifi_form_mgmt_frame(skb, (const u8 *)mgmt, pkt_len);
+ nxpwifi_form_mgmt_frame(skb, (const u8 *)mgmt, frame_len);
kfree(mgmt);
priv->auth_flag = HOST_MLME_AUTH_PENDING;
priv->auth_alg = auth_alg;
--
2.25.1