[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