This should hopefully prevent a crash reported to me by Jesper Wallin,
where net80211 crashes when it attempts to decrypt a CCMP-encrypted
frame which iwm passed up without decrypting it first.

By code inspection I have determined that this problem could happen
in case a CCMP frame is received peer before the WPA handshake has
completed (e.g. during re-association). Note how the hardware decryption
code path depends on IEEE80211_NODE_RXPROT being set, which is set only
once the handshake has been completed.
We have to be careful here to avoid breaking non-CCMP ciphers (WEP, TKIP). 

It also attempts to close a small race where we're handing the key to
firmware and eventually get an interrupt which confirms key installation,
in parallel to sending/receiving unicast data frames.
Until the key has been confirmed by firmware, don't transmit data frames
and drop received encrypted frames.
This part of the diff has poor error handling; if firmware never confirms
they key then iwm will get stuck and the user will have to recover.
But that should be better than the current situation where we'd be
sending unencrypted frames or perhaps crash in net80211.

These fixes could be separated but a combined diff is easier to test.

Once we've got this working for iwm(4), the iwn(4) driver will need
a similar fix.

diff refs/heads/master refs/heads/ccmp-crash
blob - 038e6a63dfff113b525bb1e9a30c935996535569
blob + 0571a3a042c0097002241e2accc026c55af205ed
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -3601,6 +3601,13 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac
            !IEEE80211_IS_MULTICAST(wh->i_addr1) &&
            (ni->ni_flags & IEEE80211_NODE_RXPROT) &&
            ni->ni_pairwise_key.k_cipher == IEEE80211_CIPHER_CCMP) {
+               if ((sc->sc_flags & IWM_FLAG_CCMP_READY) == 0) {
+                       /* Firmware has not installed the key yet. */
+                       ic->ic_stats.is_ccmp_dec_errs++;
+                       ifp->if_ierrors++;
+                       m_freem(m);
+                       goto done;
+               }
                if ((rx_pkt_status & IWM_RX_MPDU_RES_STATUS_SEC_ENC_MSK) !=
                    IWM_RX_MPDU_RES_STATUS_SEC_CCM_ENC) {
                        ic->ic_stats.is_ccmp_dec_errs++;
@@ -3625,6 +3632,22 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac
                        goto done;
                }
                rxi.rxi_flags |= IEEE80211_RXI_HWDEC;
+       } else if ((wh->i_fc[1] & IEEE80211_FC1_PROTECTED) &&
+           !IEEE80211_IS_MULTICAST(wh->i_addr1) &&
+           (ic->ic_flags & IEEE80211_F_RSNON) &&
+           (ic->ic_rsnprotos & IEEE80211_PROTO_RSN) &&
+           (ic->ic_rsnciphers & IEEE80211_CIPHER_CCMP) &&
+           (ni->ni_rsnciphers & IEEE80211_CIPHER_CCMP) &&
+           (ni->ni_flags & IEEE80211_NODE_RXPROT) == 0) {
+               /*
+                * We're doing CCMP and have received an encrypted unicast
+                * frame before 4-way handshake has completed. Don't pass it
+                * up the stack; net80211 would crash trying to decrypt it.
+                */
+               ic->ic_stats.is_ccmp_dec_errs++;
+               ifp->if_ierrors++;
+               m_freem(m);
+               goto done;
        }
 
 #if NBPFILTER > 0
@@ -6090,6 +6113,7 @@ iwm_set_key(struct ieee80211com *ic, struct ieee80211_
 {
        struct iwm_softc *sc = ic->ic_softc;
        struct iwm_add_sta_key_cmd cmd = { 0 };
+       int err;
 
        if ((k->k_flags & IEEE80211_KEY_GROUP) ||
            k->k_cipher != IEEE80211_CIPHER_CCMP)  {
@@ -6097,6 +6121,8 @@ iwm_set_key(struct ieee80211com *ic, struct ieee80211_
                return (ieee80211_set_key(ic, ni, k));
        }
 
+       sc->sc_flags &= ~IWM_FLAG_CCMP_READY;
+
        cmd.key_flags = htole16(IWM_STA_KEY_FLG_CCM |
            IWM_STA_KEY_FLG_WEP_KEY_MAP |
            ((k->k_id << IWM_STA_KEY_FLG_KEYID_POS) &
@@ -6108,8 +6134,11 @@ iwm_set_key(struct ieee80211com *ic, struct ieee80211_
        cmd.key_offset = 0;
        cmd.sta_id = IWM_STATION_ID;
 
-       return iwm_send_cmd_pdu(sc, IWM_ADD_STA_KEY, IWM_CMD_ASYNC,
+       err = iwm_send_cmd_pdu(sc, IWM_ADD_STA_KEY, IWM_CMD_ASYNC,
            sizeof(cmd), &cmd);
+       if (!err)
+               sc->sc_flags |= IWM_FLAG_CCMP_KEY_SENT;
+       return err;
 }
 
 void
@@ -6135,6 +6164,7 @@ iwm_delete_key(struct ieee80211com *ic, struct ieee802
        cmd.sta_id = IWM_STATION_ID;
 
        iwm_send_cmd_pdu(sc, IWM_ADD_STA_KEY, IWM_CMD_ASYNC, sizeof(cmd), &cmd);
+       sc->sc_flags &= ~(IWM_FLAG_CCMP_KEY_SENT | IWM_FLAG_CCMP_READY);
 }
 
 void
@@ -6765,6 +6795,14 @@ iwm_start(struct ifnet *ifp)
                    (ic->ic_xflags & IEEE80211_F_TX_MGMT_ONLY))
                        break;
 
+               if (sc->sc_flags & IWM_FLAG_CCMP_KEY_SENT) {
+                       /*
+                        * Can't send data frames before firmware is
+                        * ready to encrypt them.
+                        */
+                       break;
+               }
+
                IFQ_DEQUEUE(&ifp->if_snd, m);
                if (!m)
                        break;
@@ -6846,6 +6884,7 @@ iwm_stop(struct ifnet *ifp)
        sc->sc_flags &= ~IWM_FLAG_TE_ACTIVE;
        sc->sc_flags &= ~IWM_FLAG_HW_ERR;
        sc->sc_flags &= ~IWM_FLAG_SHUTDOWN;
+       sc->sc_flags &= ~(IWM_FLAG_CCMP_KEY_SENT | IWM_FLAG_CCMP_READY);
 
        sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
 
@@ -7331,6 +7370,12 @@ iwm_notif_intr(struct iwm_softc *sc)
                        break;
 
                case IWM_ADD_STA_KEY:
+                       if (sc->sc_flags & IWM_FLAG_CCMP_KEY_SENT) {
+                               sc->sc_flags |= IWM_FLAG_CCMP_READY;
+                               sc->sc_flags &= ~IWM_FLAG_CCMP_KEY_SENT;
+                       }
+                       break;
+
                case IWM_PHY_CONFIGURATION_CMD:
                case IWM_TX_ANT_CONFIGURATION_CMD:
                case IWM_ADD_STA:
blob - e6593fbc840833a36d5a792042faaa85e3ac9719
blob + 747616074f868d8679f9f01232dec4ce09474f93
--- sys/dev/pci/if_iwmvar.h
+++ sys/dev/pci/if_iwmvar.h
@@ -289,6 +289,8 @@ struct iwm_rx_ring {
 #define IWM_FLAG_HW_ERR                0x80    /* hardware error occurred */
 #define IWM_FLAG_SHUTDOWN      0x100   /* shutting down; new tasks forbidden */
 #define IWM_FLAG_BGSCAN                0x200   /* background scan in progress 
*/
+#define IWM_FLAG_CCMP_KEY_SENT 0x400   /* CCMP key sent to firmware */
+#define IWM_FLAG_CCMP_READY    0x800   /* CCMP key installed in firmware */
 
 struct iwm_ucode_status {
        uint32_t uc_error_event_table;

Reply via email to