Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/net80211 Several changes:



details:   https://anonhg.NetBSD.org/src/rev/d7c6fa2bb92e
branches:  trunk
changeset: 829116:d7c6fa2bb92e
user:      maxv <maxv%NetBSD.org@localhost>
date:      Wed Jan 17 16:03:16 2018 +0000

description:
Several changes:

 * Style in several places, to make the code more readable or easier to
   understand.

 * Instead of checking m->m_pkthdr.len, check m->m_len. m_pkthdr.len is
   the total size of the packet, not the size of the current mbuf (which
   may be smaller).

 * Add a missing length check when handling QoS frames.

 * Cast the lengths passed in IEEE80211_VERIFY_LENGTH to size_t.

 * Remove the length check on scan.sp_xrates, that I added yesterday.
   xrates gets silently truncated in ieee80211_setup_rates().

 * Fix several buffer overflows in the parsers of the MANAGEMENT frames.

diffstat:

 sys/net80211/ieee80211_input.c |  248 +++++++++++++++++++++++++---------------
 1 files changed, 153 insertions(+), 95 deletions(-)

diffs (truncated from 841 to 300 lines):

diff -r 7c5f3bc15ee4 -r d7c6fa2bb92e sys/net80211/ieee80211_input.c
--- a/sys/net80211/ieee80211_input.c    Wed Jan 17 13:52:12 2018 +0000
+++ b/sys/net80211/ieee80211_input.c    Wed Jan 17 16:03:16 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ieee80211_input.c,v 1.108 2018/01/16 18:53:32 maxv Exp $       */
+/*     $NetBSD: ieee80211_input.c,v 1.109 2018/01/17 16:03:16 maxv Exp $       */
 
 /*
  * Copyright (c) 2001 Atsushi Onoe
@@ -37,7 +37,7 @@
 __FBSDID("$FreeBSD: src/sys/net80211/ieee80211_input.c,v 1.81 2005/08/10 16:22:29 sam Exp $");
 #endif
 #ifdef __NetBSD__
-__KERNEL_RCSID(0, "$NetBSD: ieee80211_input.c,v 1.108 2018/01/16 18:53:32 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ieee80211_input.c,v 1.109 2018/01/17 16:03:16 maxv Exp $");
 #endif
 
 #ifdef _KERNEL_OPT
@@ -354,8 +354,7 @@
                 * any non-PAE frames received without encryption.
                 */
                if ((ic->ic_flags & IEEE80211_F_DROPUNENC) &&
-                   key == NULL &&
-                   eh->ether_type != htons(ETHERTYPE_PAE)) {
+                   key == NULL && eh->ether_type != htons(ETHERTYPE_PAE)) {
                        /*
                         * Drop unencrypted frames.
                         */
@@ -407,10 +406,9 @@
                ic->ic_stats.is_rx_wrongdir++;
                goto err;
        }
-       if (m->m_pkthdr.len < sizeof(struct ieee80211_frame)) {
-               IEEE80211_DISCARD_MAC(ic, IEEE80211_MSG_ANY,
-                   ni->ni_macaddr, "mgt", "too short: len %u",
-                   m->m_pkthdr.len);
+       if (m->m_len < sizeof(struct ieee80211_frame)) {
+               IEEE80211_DISCARD_MAC(ic, IEEE80211_MSG_ANY, ni->ni_macaddr,
+                   "mgt", "too short: len %u", m->m_len);
                ic->ic_stats.is_rx_tooshort++;
                goto out;
        }
@@ -542,10 +540,10 @@
        if (ic->ic_opmode == IEEE80211_M_MONITOR)
                goto out;
 
-       if (m->m_pkthdr.len < sizeof(struct ieee80211_frame_min)) {
+       if (m->m_len < sizeof(struct ieee80211_frame_min)) {
                IEEE80211_DISCARD_MAC(ic, IEEE80211_MSG_ANY,
                    ni->ni_macaddr, NULL,
-                   "too short (1): len %u", m->m_pkthdr.len);
+                   "too short (1): len %u", m->m_len);
                ic->ic_stats.is_rx_tooshort++;
                goto out;
        }
@@ -607,11 +605,11 @@
                        else if (type == IEEE80211_FC0_TYPE_CTL)
                                bssid = wh->i_addr1;
                        else {
-                               if (m->m_pkthdr.len < sizeof(struct ieee80211_frame)) {
+                               if (m->m_len < sizeof(struct ieee80211_frame)) {
                                        IEEE80211_DISCARD_MAC(ic,
                                            IEEE80211_MSG_ANY, ni->ni_macaddr,
                                            NULL, "too short (2): len %u",
-                                           m->m_pkthdr.len);
+                                           m->m_len);
                                        ic->ic_stats.is_rx_tooshort++;
                                        goto out;
                                }
@@ -674,7 +672,14 @@
                        if (ieee80211_has_qos(wh)) {
                                struct ieee80211_qosframe *qosf;
 
-                               /* XXX mbuf length check */
+                               if (m->m_len < sizeof(struct ieee80211_qosframe)) {
+                                       IEEE80211_DISCARD_MAC(ic,
+                                           IEEE80211_MSG_ANY,
+                                           ni->ni_macaddr, NULL,
+                                           "too short (1): len %u", m->m_len);
+                                       ic->ic_stats.is_rx_tooshort++;
+                                       goto out;
+                               }
                                qosf = mtod(m, struct ieee80211_qosframe *);
 
                                tid = qosf->i_qos[0] & IEEE80211_QOS_TID;
@@ -1089,6 +1094,7 @@
                    ni->ni_macaddr, "open auth",
                    "bad sta auth mode %u", ni->ni_authmode);
                ic->ic_stats.is_rx_bad_auth++;  /* XXX */
+
                if (ic->ic_opmode == IEEE80211_M_HOSTAP) {
                        /* XXX hack to workaround calling convention */
                        ieee80211_send_error(ic, ni, wh->i_addr2,
@@ -1097,6 +1103,7 @@
                }
                return;
        }
+
        switch (ic->ic_opmode) {
        case IEEE80211_M_IBSS:
        case IEEE80211_M_AHDEMO:
@@ -1114,13 +1121,16 @@
                        ic->ic_stats.is_rx_bad_auth++;
                        return;
                }
+
                /* always accept open authentication requests */
                if (ni == ic->ic_bss) {
                        ni = ieee80211_dup_bss(&ic->ic_sta, wh->i_addr2);
                        if (ni == NULL)
                                return;
-               } else if ((ni->ni_flags & IEEE80211_NODE_AREF) == 0)
-                       (void) ieee80211_ref_node(ni);
+               } else if ((ni->ni_flags & IEEE80211_NODE_AREF) == 0) {
+                       (void)ieee80211_ref_node(ni);
+               }
+
                /*
                 * Mark the node as referenced to reflect that its
                 * reference count has been bumped to insure it remains
@@ -1128,11 +1138,12 @@
                 */
                ni->ni_flags |= IEEE80211_NODE_AREF;
 
-               IEEE80211_SEND_MGMT(ic, ni,
-                       IEEE80211_FC0_SUBTYPE_AUTH, seq + 1);
+               IEEE80211_SEND_MGMT(ic, ni, IEEE80211_FC0_SUBTYPE_AUTH,
+                   seq + 1);
                IEEE80211_DPRINTF(ic, IEEE80211_MSG_DEBUG | IEEE80211_MSG_AUTH,
                    "[%s] station authenticated (open)\n",
                    ether_snprintf(ebuf, sizeof(ebuf), ni->ni_macaddr));
+
                /*
                 * When 802.1x is not in use mark the port
                 * authorized at this point so traffic can flow.
@@ -1149,20 +1160,22 @@
                        return;
                }
                if (status != 0) {
-
                        IEEE80211_DPRINTF(ic,
                            IEEE80211_MSG_DEBUG | IEEE80211_MSG_AUTH,
                            "[%s] open auth failed (reason %d)\n",
                            ether_snprintf(ebuf, sizeof(ebuf), ni->ni_macaddr),
                            status);
+
                        /* XXX can this happen? */
                        if (ni != ic->ic_bss)
                                ni->ni_fails++;
+
                        ic->ic_stats.is_rx_auth_fail++;
                        ieee80211_new_state(ic, IEEE80211_S_SCAN, 0);
-               } else
+               } else {
                        ieee80211_new_state(ic, IEEE80211_S_ASSOC,
                            wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK);
+               }
                break;
        }
 }
@@ -1176,9 +1189,9 @@
  */
 static void
 ieee80211_send_error(struct ieee80211com *ic, struct ieee80211_node *ni,
-       const u_int8_t *mac, int subtype, int arg)
+    const u_int8_t *mac, int subtype, int arg)
 {
-       int istmp;
+       bool istmp;
 
        if (ni == ic->ic_bss) {
                ni = ieee80211_tmp_node(ic, mac);
@@ -1186,9 +1199,11 @@
                        /* XXX msg */
                        return;
                }
-               istmp = 1;
-       } else
-               istmp = 0;
+               istmp = true;
+       } else {
+               istmp = false;
+       }
+
        IEEE80211_SEND_MGMT(ic, ni, subtype, arg);
        if (istmp)
                ieee80211_free_node(ni);
@@ -1197,11 +1212,12 @@
 static int
 alloc_challenge(struct ieee80211com *ic, struct ieee80211_node *ni)
 {
-       if (ni->ni_challenge == NULL)
+       if (ni->ni_challenge == NULL) {
                ni->ni_challenge = malloc(IEEE80211_CHALLENGE_LEN,
                    M_DEVBUF, M_NOWAIT);
+       }
        if (ni->ni_challenge == NULL) {
-                IEEE80211_DEBUGVAR(char ebuf[3 * ETHER_ADDR_LEN]);
+               IEEE80211_DEBUGVAR(char ebuf[3 * ETHER_ADDR_LEN]);
 
                IEEE80211_DPRINTF(ic, IEEE80211_MSG_DEBUG | IEEE80211_MSG_AUTH,
                    "[%s] shared key challenge alloc failed\n",
@@ -1236,6 +1252,7 @@
                estatus = IEEE80211_STATUS_ALG;
                goto bad;
        }
+
        /*
         * Pre-shared key authentication is evil; accept
         * it only if explicitly configured (it is supported
@@ -1266,6 +1283,7 @@
                        challenge = frm;
                frm += frm[1] + 2;
        }
+
        switch (seq) {
        case IEEE80211_AUTH_SHARED_CHALLENGE:
        case IEEE80211_AUTH_SHARED_RESPONSE:
@@ -1288,6 +1306,7 @@
        default:
                break;
        }
+
        switch (ic->ic_opmode) {
        case IEEE80211_M_MONITOR:
        case IEEE80211_M_AHDEMO:
@@ -1296,6 +1315,7 @@
                    ni->ni_macaddr, "shared key auth",
                    "bad operating mode %u", ic->ic_opmode);
                return;
+
        case IEEE80211_M_HOSTAP:
 #ifndef IEEE80211_NO_HOSTAP
        {
@@ -1318,10 +1338,11 @@
                                allocbs = 1;
                        } else {
                                if ((ni->ni_flags & IEEE80211_NODE_AREF) == 0)
-                                       (void) ieee80211_ref_node(ni);
+                                       (void)ieee80211_ref_node(ni);
                                allocbs = 0;
                        }
                        __USE(allocbs);
+
                        /*
                         * Mark the node as referenced to reflect that its
                         * reference count has been bumped to insure it remains
@@ -1334,8 +1355,10 @@
                                /* NB: don't return error so they rexmit */
                                return;
                        }
+
                        get_random_bytes(ni->ni_challenge,
                                IEEE80211_CHALLENGE_LEN);
+
                        IEEE80211_DPRINTF(ic,
                                IEEE80211_MSG_DEBUG | IEEE80211_MSG_AUTH,
                                "[%s] shared key %sauth request\n",
@@ -1343,6 +1366,7 @@
                                ni->ni_macaddr),
                                allocbs ? "" : "re");
                        break;
+
                case IEEE80211_AUTH_SHARED_RESPONSE:
                        if (ni == ic->ic_bss) {
                                IEEE80211_DISCARD_MAC(ic, IEEE80211_MSG_AUTH,
@@ -1351,6 +1375,7 @@
                                /* NB: don't send a response */
                                return;
                        }
+
                        if (ni->ni_challenge == NULL) {
                                IEEE80211_DISCARD_MAC(ic, IEEE80211_MSG_AUTH,
                                    ni->ni_macaddr, "shared key response",
@@ -1359,8 +1384,9 @@
                                estatus = IEEE80211_STATUS_CHALLENGE;
                                goto bad;
                        }
+
                        if (memcmp(ni->ni_challenge, &challenge[2],
-                                  challenge[1]) != 0) {
+                           challenge[1]) != 0) {
                                IEEE80211_DISCARD_MAC(ic, IEEE80211_MSG_AUTH,
                                    ni->ni_macaddr, "shared key response",
                                    "%s", "challenge mismatch");
@@ -1368,12 +1394,15 @@
                                estatus = IEEE80211_STATUS_CHALLENGE;
                                goto bad;
                        }
+
                        IEEE80211_DPRINTF(ic,
                            IEEE80211_MSG_DEBUG | IEEE80211_MSG_AUTH,
                            "[%s] station authenticated (shared key)\n",
                            ether_snprintf(ebuf, sizeof(ebuf), ni->ni_macaddr));
+
                        ieee80211_node_authorize(ni);
                        break;
+
                default:
                        IEEE80211_DISCARD_MAC(ic, IEEE80211_MSG_AUTH,



Home | Main Index | Thread Index | Old Index