[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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.