From: Stefan Sperling Subject: Re: qwx(4) crypto offloading To: Kevin Lo , tech@openbsd.org Date: Wed, 29 May 2024 00:10:13 +0200 On Mon, May 27, 2024 at 05:28:27PM +0200, Stefan Sperling wrote: > WEP can be fixed with the change below. However, it breaks WPA2 for > some reason. I don't know if WPA2 not working in RAW mode is a driver > or a firmware issue. But I will give up now. I plan to write a new diff > which offloads all of the ciphers because that seems easier on this crazy > hardware. This diff makes WEP and TKIP work, however both types of networks still suffer from the broadcast Rx issue. I haven't found a way to avoid that. I am running out of time, so this is the best I can do. ok? ----------------------------------------------- repair qwx(4) WEP and TKIP via software crypto It is difficult to make WEP and WPA1/TKIP work with hardware crypto. Add a comment which explains why. Ensure that setkey task state is properly cleared when the interface goes down. This issue was found while trying to add WEP keys for hw crypto, but is still worth fixing in general. Also, use m_makespace to append trailing padding for the MIC when hardware crypto is used in combination with "raw" frame mode (not the default), instead of blindly adjusting m_len. diff f475ad27b09c6d0a71ec6c04c2d3bf6f36e667ce 6741a8ff1ba30932c37c7b6a84839f402ed1e969 commit - f475ad27b09c6d0a71ec6c04c2d3bf6f36e667ce commit + 6741a8ff1ba30932c37c7b6a84839f402ed1e969 blob - 91500360ed82a3c741f15f029e37b72d879210d4 blob + 4d3ed3ff5d82894fe280c95f447dff6e25c199bb --- sys/dev/ic/qwx.c +++ sys/dev/ic/qwx.c @@ -157,6 +157,7 @@ int qwx_wmi_vdev_install_key(struct qwx_softc *, struct wmi_vdev_install_key_arg *, uint8_t); int qwx_dp_peer_rx_pn_replay_config(struct qwx_softc *, struct qwx_vif *, struct ieee80211_node *, struct ieee80211_key *, int); +void qwx_setkey_clear(struct qwx_softc *); int qwx_scan(struct qwx_softc *); void qwx_scan_abort(struct qwx_softc *); @@ -183,7 +184,45 @@ qwx_init(struct ifnet *ifp) struct ieee80211com *ic = &sc->sc_ic; sc->fw_mode = ATH11K_FIRMWARE_MODE_NORMAL; - sc->crypto_mode = ATH11K_CRYPT_MODE_HW; + /* + * There are several known hardware/software crypto issues + * on wcn6855 devices, firmware 0x1106196e. It is unclear + * if these are driver or firmware bugs. + * + * 1) Broadcast/Multicast frames will only be received on + * encrypted networks if hardware crypto is used and a + * CCMP group key is used. Otherwise such frames never + * even trigger an interrupt. This breaks ARP and IPv6. + * This issue is known to affect the Linux ath11k vendor + * driver when software crypto mode is selected. + * Workaround: Use hardware crypto on WPA2 networks. + * However, even with hardware crypto broadcast frames + * are never received if TKIP is used as the WPA2 group + * cipher and we have no workaround for this. + * + * 2) Adding WEP keys for hardware crypto crashes the firmware. + * Presumably, lack of WEP support is deliberate because the + * Linux ath11k vendor driver rejects attempts to install + * WEP keys to hardware. + * Workaround: Use software crypto if WEP is enabled. + * This suffers from the broadcast issues mentioned above. + * + * 3) A WPA1 group key handshake message from the AP is never + * received if hardware crypto is used. + * Workaround: Use software crypto if WPA1 is enabled. + * This suffers from the broadcast issues mentioned above, + * even on WPA2 networks when WPA1 and WPA2 are both enabled. + * On OpenBSD, WPA1 is disabled by default. + * + * The only known fully working configurations are unencrypted + * networks, and WPA2/CCMP-only networks provided WPA1 remains + * disabled. + */ + if ((ic->ic_flags & IEEE80211_F_WEPON) || + (ic->ic_rsnprotos & IEEE80211_PROTO_WPA)) + sc->crypto_mode = ATH11K_CRYPT_MODE_SW; + else + sc->crypto_mode = ATH11K_CRYPT_MODE_HW; sc->frame_mode = ATH11K_HW_TXRX_NATIVE_WIFI; ic->ic_state = IEEE80211_S_INIT; sc->ns_nstate = IEEE80211_S_INIT; @@ -291,6 +330,8 @@ qwx_stop(struct ifnet *ifp) qwx_del_task(sc, systq, &sc->setkey_task); refcnt_finalize(&sc->task_refs, "qwxstop"); + qwx_setkey_clear(sc); + clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags); ifp->if_timer = sc->sc_tx_timer = 0; @@ -529,8 +570,8 @@ qwx_set_key(struct ieee80211com *ic, struct ieee80211_ struct qwx_softc *sc = ic->ic_softc; if (test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, sc->sc_flags) || - (k->k_cipher != IEEE80211_CIPHER_CCMP && - k->k_cipher != IEEE80211_CIPHER_TKIP)) + k->k_cipher == IEEE80211_CIPHER_WEP40 || + k->k_cipher == IEEE80211_CIPHER_WEP104) return ieee80211_set_key(ic, ni, k); return qwx_queue_setkey_cmd(ic, ni, k, QWX_ADD_KEY); @@ -543,8 +584,8 @@ qwx_delete_key(struct ieee80211com *ic, struct ieee802 struct qwx_softc *sc = ic->ic_softc; if (test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, sc->sc_flags) || - (k->k_cipher != IEEE80211_CIPHER_CCMP && - k->k_cipher != IEEE80211_CIPHER_TKIP)) { + k->k_cipher == IEEE80211_CIPHER_WEP40 || + k->k_cipher == IEEE80211_CIPHER_WEP104) { ieee80211_delete_key(ic, ni, k); return; } @@ -757,6 +798,24 @@ qwx_setkey_task(void *arg) splx(s); } +void +qwx_setkey_clear(struct qwx_softc *sc) +{ + struct ieee80211com *ic = &sc->sc_ic; + struct qwx_setkey_task_arg *a; + + while (sc->setkey_nkeys > 0) { + a = &sc->setkey_arg[sc->setkey_tail]; + ieee80211_release_node(ic, a->ni); + a->ni = NULL; + sc->setkey_tail = (sc->setkey_tail + 1) % + nitems(sc->setkey_arg); + sc->setkey_nkeys--; + } + memset(sc->setkey_arg, 0, sizeof(sc->setkey_arg)); + sc->setkey_cur = sc->setkey_tail = sc->setkey_nkeys = 0; +} + int qwx_newstate(struct ieee80211com *ic, enum ieee80211_state nstate, int arg) { @@ -773,21 +832,11 @@ qwx_newstate(struct ieee80211com *ic, enum ieee80211_s nstate != IEEE80211_S_AUTH) return 0; if (ic->ic_state == IEEE80211_S_RUN) { - struct qwx_setkey_task_arg *a; #if 0 qwx_del_task(sc, systq, &sc->ba_task); #endif qwx_del_task(sc, systq, &sc->setkey_task); - while (sc->setkey_nkeys > 0) { - a = &sc->setkey_arg[sc->setkey_tail]; - ieee80211_release_node(ic, a->ni); - a->ni = NULL; - sc->setkey_tail = (sc->setkey_tail + 1) % - nitems(sc->setkey_arg); - sc->setkey_nkeys--; - } - memset(sc->setkey_arg, 0, sizeof(sc->setkey_arg)); - sc->setkey_cur = sc->setkey_tail = sc->setkey_nkeys = 0; + qwx_setkey_clear(sc); #if 0 qwx_del_task(sc, systq, &sc->bgscan_done_task); #endif @@ -16021,13 +16070,15 @@ qwx_dp_rx_h_reo_err(struct qwx_softc *sc, struct qwx_r int qwx_dp_rx_h_rxdma_err(struct qwx_softc *sc, struct qwx_rx_msdu *msdu) { + struct ieee80211com *ic = &sc->sc_ic; int drop = 0; #if 0 ar->ab->soc_stats.rxdma_error[rxcb->err_code]++; #endif switch (msdu->err_code) { case HAL_REO_ENTR_RING_RXDMA_ECODE_TKIP_MIC_ERR: - drop = 1; /* OpenBSD uses TKIP in software crypto mode only */ + ic->ic_stats.is_rx_locmicfail++; + drop = 1; break; default: /* TODO: Review other rxdma error code to check if anything is @@ -24193,7 +24244,7 @@ qwx_dp_tx(struct qwx_softc *sc, struct qwx_vif *arvif, void *hal_tcl_desc; uint8_t pool_id; uint8_t hal_ring_id; - int ret, msdu_id; + int ret, msdu_id, off; uint32_t ring_selector = 0; uint8_t ring_map = 0; @@ -24238,22 +24289,34 @@ qwx_dp_tx(struct qwx_softc *sc, struct qwx_vif *arvif, if ((wh->i_fc[1] & IEEE80211_FC1_PROTECTED) && ti.encap_type == HAL_TCL_ENCAP_TYPE_RAW) { k = ieee80211_get_txkey(ic, wh, ni); - switch (k->k_cipher) { - case IEEE80211_CIPHER_CCMP: - ti.encrypt_type = HAL_ENCRYPT_TYPE_CCMP_128; - m->m_pkthdr.len += IEEE80211_CCMP_MICLEN; - break; - case IEEE80211_CIPHER_TKIP: - ti.encrypt_type = HAL_ENCRYPT_TYPE_TKIP_MIC; - m->m_pkthdr.len += IEEE80211_TKIP_MICLEN; - break; - default: - /* Fallback to software crypto for other ciphers. */ + if (test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, sc->sc_flags)) { ti.encrypt_type = HAL_ENCRYPT_TYPE_OPEN; - break; + } else { + switch (k->k_cipher) { + case IEEE80211_CIPHER_CCMP: + ti.encrypt_type = HAL_ENCRYPT_TYPE_CCMP_128; + if (m_makespace(m, m->m_pkthdr.len, + IEEE80211_CCMP_MICLEN, &off) == NULL) { + m_freem(m); + return ENOSPC; + } + break; + case IEEE80211_CIPHER_TKIP: + ti.encrypt_type = HAL_ENCRYPT_TYPE_TKIP_MIC; + if (m_makespace(m, m->m_pkthdr.len, + IEEE80211_TKIP_MICLEN, &off) == NULL) { + m_freem(m); + return ENOSPC; + } + break; + default: + ti.encrypt_type = HAL_ENCRYPT_TYPE_OPEN; + break; + } } if (ti.encrypt_type == HAL_ENCRYPT_TYPE_OPEN) { + /* Using software crypto. */ if ((m = ieee80211_encrypt(ic, m, k)) == NULL) return ENOBUFS; /* 802.11 header may have moved. */