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

[syzbot <syzbot+cc867e537e4bd36f69bb@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH v3] wifi: cfg80211: add cfg80211_validate_ies() helper and use it
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci


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: syzbot+cc867e537e4bd36f69bb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=cc867e537e4bd36f69bb
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

---
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