On Thu, 15 Aug 2019, richard.n.proc...@gmail.com wrote:
> Hi Stefan,
>
> On Wed, 14 Aug 2019, Stefan Sperling wrote:
>
> > On Tue, Aug 13, 2019 at 10:47:24AM -0300, Martin Pieuchot wrote:
> > > if the crash is because of a missing key, put one :)
> >
> > Yes, you've convinced me. With this patch we install the CCMP key
> > to both firmware and net80211. Until firmware confirms that the
> > key has been installed, we do software encrypt/decrypt.
>
> I agree we should handle a missing key but suggest an alternative approach
> below.
so:
m0 = NULL;
m_free(m0);
isn't so useful; fixed patch follows.
best,
Richard.
Index: net80211/ieee80211_crypto.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_crypto.c,v
retrieving revision 1.74
diff -u -p -u -p -r1.74 ieee80211_crypto.c
--- net80211/ieee80211_crypto.c 24 Sep 2018 20:14:59 -0000 1.74
+++ net80211/ieee80211_crypto.c 15 Aug 2019 03:44:12 -0000
@@ -157,6 +157,10 @@ ieee80211_set_key(struct ieee80211com *i
/* should not get there */
error = EINVAL;
}
+
+ if (error == 0)
+ k->k_flags |= IEEE80211_KEY_SWCRYPTO;
+
return error;
}
@@ -280,6 +284,11 @@ ieee80211_decrypt(struct ieee80211com *i
}
k = &ic->ic_nw_keys[kid];
}
+
+ if ((k->k_flags & IEEE80211_KEY_SWCRYPTO) == 0) {
+ goto err;
+ }
+
switch (k->k_cipher) {
case IEEE80211_CIPHER_WEP40:
case IEEE80211_CIPHER_WEP104:
@@ -296,9 +305,15 @@ ieee80211_decrypt(struct ieee80211com *i
break;
default:
/* key not defined */
- m_freem(m0);
- m0 = NULL;
+ goto err;
}
+
+ return m0;
+
+err:
+ m_freem(m0);
+ m0 = NULL;
+
return m0;
}
Index: net80211/ieee80211_crypto.h
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_crypto.h,v
retrieving revision 1.25
diff -u -p -u -p -r1.25 ieee80211_crypto.h
--- net80211/ieee80211_crypto.h 18 Aug 2017 17:30:12 -0000 1.25
+++ net80211/ieee80211_crypto.h 15 Aug 2019 03:44:12 -0000
@@ -78,6 +78,7 @@ struct ieee80211_key {
#define IEEE80211_KEY_GROUP 0x00000001 /* group data key */
#define IEEE80211_KEY_TX 0x00000002 /* Tx+Rx */
#define IEEE80211_KEY_IGTK 0x00000004 /* integrity group key */
+#define IEEE80211_KEY_SWCRYPTO 0x00000080 /* loaded for software crypto */
u_int k_len;
u_int64_t k_rsc[IEEE80211_NUM_TID];
> >
> > Should the firmware fail to install the key for some reason, we will
> > just keep using our software fallback until the interface is reset.
> >
> > This also fixes the race with incoming frames during WPA handshake,
> > i.e. while (ni->ni_flags & IEEE80211_NODE_RXPROT) == 0.
> > Such frames can be passed to net80211 and will be dropped there.
> >
> > Lightly tested on a 7260 device.
> >
> > Ok?
> >
> > diff refs/heads/master refs/heads/ccmp-crash2
> > blob - 038e6a63dfff113b525bb1e9a30c935996535569
> > blob + 8afcce065eb0d85f4469a3f378cacba1513bc215
> > --- sys/dev/pci/if_iwm.c
> > +++ sys/dev/pci/if_iwm.c
> > @@ -3595,10 +3595,17 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct
> > iwm_rx_pac
> > rxi.rxi_rssi = rssi;
> > rxi.rxi_tstamp = device_timestamp;
> >
> > - /* Handle hardware decryption. */
> > + /*
> > + * Handle hardware decryption.
> > + *
> > + * If the WPA handshake has completed but firmware is not yet ready
> > + * to decrypt frames, fall back to software decryption in net80211.
> > + * net80211 will drop encrypted frames until the handshake completes.
> > + */
> > if (((wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) != IEEE80211_FC0_TYPE_CTL)
> > && (wh->i_fc[1] & IEEE80211_FC1_PROTECTED) &&
> > !IEEE80211_IS_MULTICAST(wh->i_addr1) &&
> > + (sc->sc_flags & IWM_FLAG_CCMP_READY) &&
> > (ni->ni_flags & IEEE80211_NODE_RXPROT) &&
> > ni->ni_pairwise_key.k_cipher == IEEE80211_CIPHER_CCMP) {
> > if ((rx_pkt_status & IWM_RX_MPDU_RES_STATUS_SEC_ENC_MSK) !=
> > @@ -4402,7 +4409,8 @@ iwm_tx(struct iwm_softc *sc, struct mbuf *m, struct ie
> >
> > if (wh->i_fc[1] & IEEE80211_FC1_PROTECTED) {
> > k = ieee80211_get_txkey(ic, wh, ni);
> > - if (k->k_cipher != IEEE80211_CIPHER_CCMP) {
> > + if (k->k_cipher != IEEE80211_CIPHER_CCMP ||
> > + (sc->sc_flags & IWM_FLAG_CCMP_READY) == 0) {
> > if ((m = ieee80211_encrypt(ic, m, k)) == NULL)
> > return ENOBUFS;
> >
> > @@ -4418,7 +4426,8 @@ iwm_tx(struct iwm_softc *sc, struct mbuf *m, struct ie
> > /* Copy 802.11 header in TX command. */
> > memcpy(((uint8_t *)tx) + sizeof(*tx), wh, hdrlen);
> >
> > - if (k != NULL && k->k_cipher == IEEE80211_CIPHER_CCMP) {
> > + if (k != NULL && k->k_cipher == IEEE80211_CIPHER_CCMP &&
> > + (sc->sc_flags & IWM_FLAG_CCMP_READY)) {
> > /* Trim 802.11 header and prepend CCMP IV. */
> > m_adj(m, hdrlen - IEEE80211_CCMP_HDRLEN);
> > ivp = mtod(m, u_int8_t *);
> > @@ -6090,6 +6099,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 +6107,8 @@ iwm_set_key(struct ieee80211com *ic, struct ieee80211_
> > return (ieee80211_set_key(ic, ni, k));
> > }
> >
> > + sc->sc_flags &= ~(IWM_FLAG_CCMP_READY | IWM_FLAG_CCMP_KEY_SENT);
> > +
> > 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 +6120,15 @@ 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,
> > - sizeof(cmd), &cmd);
> > + /* Allow for software CCMP encryption/decryption fallback. */
> > + err = ieee80211_set_key(ic, ni, k);
> > + if (!err) {
> > + 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 +6154,10 @@ 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);
> > +
> > + /* Delete software fallback CCMP key. */
> > + ieee80211_delete_key(ic, ni, k);
> > }
> >
> > void
> > @@ -6846,6 +6869,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 +7355,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;
> >
> >
>
