wifi: nxpwifi: fix the authentication frame length handling

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 <lilinmao@kylinos.cn>
Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>

1 files changed, 16 insertions, 5 deletions

diff --git a/drivers/net/wireless/nxp/nxpwifi/cfg80211.c b/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
index 788b83356981..3221ae167c58 100644
--- a/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
+++ b/drivers/net/wireless/nxp/nxpwifi/cfg80211.c
@@ -3325,6 +3325,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;
@@ -3398,13 +3399,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 +
@@ -3450,7 +3461,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;