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;