| Message ID | 1360692698-24208-1-git-send-email-coelho@ti.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2013-02-12 at 20:11 +0200, Luciano Coelho wrote: > cfg80211_find_vendor_ie() was checking only that the vendor IE would > fit in the remaining IEs buffer. If a corrupt includes a vendor IE > that is too small, we could potentially overrun the IEs buffer. > > Fix this by checking that the vendor IE fits in the reported IE length > field and skip it otherwise. Applied. I changed the BUILD_BUG_ON to be != 1 since it has to be that, but if one breaks that ... johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-02-13 at 10:15 +0100, Johannes Berg wrote: > On Tue, 2013-02-12 at 20:11 +0200, Luciano Coelho wrote: > > cfg80211_find_vendor_ie() was checking only that the vendor IE would > > fit in the remaining IEs buffer. If a corrupt includes a vendor IE > > that is too small, we could potentially overrun the IEs buffer. > > > > Fix this by checking that the vendor IE fits in the reported IE length > > field and skip it otherwise. > > Applied. I changed the BUILD_BUG_ON to be != 1 since it has to be that, > but if one breaks that ... Okay. But it actually needs to be 1 in the cfg80211_find_ie() function, not here. Here we just need to make sure that ie->len is guaranteed to fit in what we got. cfg80211_find_ie() guarantees that we have at least 2 bytes, thus the >= 2 I used. Anyway, this is all *extremely* nitpicky already. :P -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/wireless/scan.c b/net/wireless/scan.c index 45f1618..4582801 100644 --- a/net/wireless/scan.c +++ b/net/wireless/scan.c @@ -277,14 +277,18 @@ const u8 *cfg80211_find_vendor_ie(unsigned int oui, u8 oui_type, if (!pos) return NULL; - if (end - pos < sizeof(*ie)) - return NULL; - ie = (struct ieee80211_vendor_ie *)pos; + + /* make sure we can access ie->len */ + BUILD_BUG_ON(offsetof(struct ieee80211_vendor_ie, len) >= 2); + + if (ie->len < sizeof(*ie)) + goto cont; + ie_oui = ie->oui[0] << 16 | ie->oui[1] << 8 | ie->oui[2]; if (ie_oui == oui && ie->oui_type == oui_type) return pos; - +cont: pos += 2 + ie->len; } return NULL;
cfg80211_find_vendor_ie() was checking only that the vendor IE would fit in the remaining IEs buffer. If a corrupt includes a vendor IE that is too small, we could potentially overrun the IEs buffer. Fix this by checking that the vendor IE fits in the reported IE length field and skip it otherwise. Reported-by: Jouni Malinen <j@w1.fi> Cc: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: Luciano Coelho <coelho@ti.com> --- net/wireless/scan.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)