Skip to contentSkip to conversation

wifi: cfg80211: validate common_info length in ieee80211_mle_size_ok()

PATCH In review Alexandru Hossu 2 participants 3 messages May 15

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 wifi: cfg80211: validate common_info length in ieee80211_mle_size_ok()

Hi Greg,

Here is the fix.

Alexandru

---

For MLE types with a common_info length field (BASIC, PREQ, TDLS), ieee80211_mle_size_ok() checks that variable[0] is at least as large as the mandatory fields, but it does not check that the value fits within the element itself.

cfg80211_defrag_mle() computes the sub-element area start as:

    common_size = ieee80211_mle_common_size((u8 *)res->mle);
    ie = res->data + common_size;
    ielen = mle_len - common_size;

ieee80211_mle_common_size() returns sizeof(*mle) + mle->variable[0]. If variable[0] is larger than the actual element, common_size exceeds mle_len and the size_t subtraction wraps. for_each_element_id() then iterates over arbitrary kernel heap memory past the allocation.

Add the missing upper-bound check so variable[0] cannot exceed the element size.

---
 include/linux/ieee80211-eht.h | 4 ++++
 1 file changed, 4 insertions(+)
include/linux/ieee80211-eht.h +4 -0
index 87d92fb86fab..738a581c3683 100644
@@ -908,6 +908,10 @@ static inline bool ieee80211_mle_size_ok(const u8 *data, size_t len)
 	if (!check_common_len)
 		return true;
 
+	/* common_info length must not exceed the element */
+	if (len < fixed + mle->variable[0])
+		return false;
+
 	/* if present, common length is the first octet there */
 	return mle->variable[0] >= common;
 }
#1 lore raw
Quoted 3 lines · Alexandru Hossu

Hi Greg,

Here is the fix.

Context matters :)

Also, this isn't in a mergable format, please take a look at the kernel documentation for how to do this properly.

thanks,

greg k-h

#2 lore raw
Quoted 3 lines · Greg KH

Context matters :)
Also, this isn't in a mergable format, please take a look at the kernel documentation for how to do this properly.

Sorry for both issues. The greeting was wrong, the format was wrong, and most importantly I didn't check the tree first -- the fix is already at ieee80211-eht.h:912 in linux-next. I wasted your time.

Withdrawing this.

Alexandru