[PATCH v3] wifi: cfg80211: add cfg80211_validate_ies() helper and use it

Deepanshu Kartikey <[email protected]>
Newsgroups org.kernel.vger.linux-wireless,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
The KASAN allocation trace shows that a malformed IE buffer is
stored via SIOCSIWGENIE (cfg80211_wext_siwgenie()) without any
validation. The crash trace shows that a subsequent SIOCSIWESSID
triggers a connection attempt which calls cfg80211_sme_get_conn_ies()
to process the stored IE buffer, causing:

 - An out-of-bounds read in skip_ie() which reads ies[pos+1]
   (the length byte) past the end of the 1-byte buffer.

 - An integer underflow in the memcpy size argument when offs
   returned by ieee80211_ie_split() exceeds ies_len, causing
   unsigned subtraction to wrap to SIZE_MAX and triggering a
   fortify panic.

Fix this by adding a cfg80211_validate_ies() helper that uses
for_each_element() and for_each_element_completed() to verify
that all information elements in a buffer are well-formed.

Refactor the existing duplicate patterns in validate_beacon_head()
and validate_ie_attr() in nl80211.c to use the new helper, and
use it in cfg80211_wext_siwgenie() to validate the IE buffer
before storing it. Return -EINVAL if validation fails.

Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=cc867e537e4bd36f69bb
Signed-off-by: Deepanshu Kartikey <[email protected]>

---
v3: Refactor validation into cfg80211_validate_ies() helper and
    use it in validate_beacon_head(), validate_ie_attr(), and
    cfg80211_wext_siwgenie(), as suggested by Jeff Johnson.
    Remove redundant ie_len < 2 check since for_each_element()
    already handles that case. Fix missing semicolon after
    /* nothing */ in for_each_element() body.

v2: Use for_each_element() and for_each_element_completed()
    instead of open-coded validation loop, as suggested by
    Johannes Berg.
---
 net/wireless/nl80211.c  | 15 ++-------------
 net/wireless/nl80211.h  |  2 ++
 net/wireless/util.c     | 11 +++++++++++
 net/wireless/wext-sme.c |  3 +++
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5adcb6bd0fc5..27bbaf916574 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -228,7 +228,6 @@ static int validate_beacon_head(const struct nlattr *attr,
 {
 	const u8 *data = nla_data(attr);
 	unsigned int len = nla_len(attr);
-	const struct element *elem;
 	const struct ieee80211_mgmt *mgmt = (void *)data;
 	const struct ieee80211_ext *ext;
 	unsigned int fixedlen, hdrlen;
@@ -259,13 +258,8 @@ static int validate_beacon_head(const struct nlattr *attr,
 	data += fixedlen;
 	len -= fixedlen;
 
-	for_each_element(elem, data, len) {
-		/* nothing */
-	}
-
-	if (for_each_element_completed(elem, data, len))
+	if (cfg80211_validate_ies(data, len))
 		return 0;
-
 err:
 	NL_SET_ERR_MSG_ATTR(extack, attr, "malformed beacon head");
 	return -EINVAL;
@@ -276,13 +270,8 @@ static int validate_ie_attr(const struct nlattr *attr,
 {
 	const u8 *data = nla_data(attr);
 	unsigned int len = nla_len(attr);
-	const struct element *elem;
-
-	for_each_element(elem, data, len) {
-		/* nothing */
-	}
 
-	if (for_each_element_completed(elem, data, len))
+	if (cfg80211_validate_ies(data, len))
 		return 0;
 
 	NL_SET_ERR_MSG_ATTR(extack, attr, "malformed information elements");
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index bdb065d14054..8322ced6b551 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -129,4 +129,6 @@ int nl80211_pmsr_start(struct sk_buff *skb, struct genl_info *info);
 void nl80211_mlo_reconf_add_done(struct net_device *dev,
 				 struct cfg80211_mlo_reconf_done_data *data);
 
+bool cfg80211_validate_ies(const u8 *data, int len);
+
 #endif /* __NET_WIRELESS_NL80211_H */
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 24527bf321b2..dcaa5903bcd2 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -3083,3 +3083,14 @@ bool cfg80211_wdev_channel_allowed(struct wireless_dev *wdev,
 	return false;
 }
 EXPORT_SYMBOL(cfg80211_wdev_channel_allowed);
+
+bool cfg80211_validate_ies(const u8 *data, int len)
+{
+	const struct element *elem;
+
+	for_each_element(elem, data, len) {
+		/* nothing */
+	}
+
+	return for_each_element_completed(elem, data, len);
+}
diff --git a/net/wireless/wext-sme.c b/net/wireless/wext-sme.c
index 573b6b15a446..1476d933c7e7 100644
--- a/net/wireless/wext-sme.c
+++ b/net/wireless/wext-sme.c
@@ -319,6 +319,9 @@ int cfg80211_wext_siwgenie(struct net_device *dev,
 		return 0;
 
 	if (ie_len) {
+		if (!cfg80211_validate_ies(extra, ie_len))
+			return -EINVAL;
+
 		ie = kmemdup(extra, ie_len, GFP_KERNEL);
 		if (!ie)
 			return -ENOMEM;
-- 
2.43.0
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.