From: Stefan Sperling Subject: replace ieee80211_chan2mode() To: tech@openbsd.org Date: Fri, 1 Aug 2025 13:16:19 +0200 ieee80211_chan2mode() is broken in several ways. chan2mode does not detect 11g APs properly. This can be fixed now that we are setting the ERP flag not only in hostap mode but also in client mode. If chan2mode is called while our phy mode is fixed, then it always returns our fixed phy mode. This means it wrongly selects 11g rates if our phy mode is fixed to 11g even if we are talking to an 11b-only peer which cannot receive 11g rates. chan2mode tries to handle a NULL channel which is not relevant to any of the callers. I want to replace chan2mode with a new ieee80211_node_abg_mode() function which serves the same purpose but does not have any of the above issues. Tested with qwx(4), which can now automatically associate with an AP which denies clients that use 11b data rates. Previously, 'ifconfig qwx0 mode 11g' was required to associate with such APs. ok? (The chunk in ieee80211_create_ibss() is not using node_abg_mode() because when we are starting a new hostap we only need to take our own capabilities into account. And the ERP flag won't be set on ic_bss, which causes node_abg_mode() to return 11b while in 11g mode. An 11g hostap needs 11g.) M sys/dev/ic/ath.c | 6+ 5- M sys/net80211/ieee80211.c | 0+ 49- M sys/net80211/ieee80211_input.c | 1+ 1- M sys/net80211/ieee80211_node.c | 58+ 6- M sys/net80211/ieee80211_node.h | 2+ 0- M sys/net80211/ieee80211_output.c | 1+ 1- M sys/net80211/ieee80211_proto.c | 3+ 3- M sys/net80211/ieee80211_var.h | 0+ 2- 8 files changed, 71 insertions(+), 67 deletions(-) commit - aec0df7811c350695d61f4bd9fd523d5a656341b commit + 488ec86079a70aab9c7bbd976d40b5811ea37ee1 blob - 8a991f09c1ecafe9ee705a243f15603894ec2078 blob + 4e65814965c5795a9b1a20487a0ba666e61ae7a1 --- sys/dev/ic/ath.c +++ sys/dev/ic/ath.c @@ -113,7 +113,7 @@ void ath_rx_proc(void *, int); int ath_tx_start(struct ath_softc *, struct ieee80211_node *, struct ath_buf *, struct mbuf *); void ath_tx_proc(void *, int); -int ath_chan_set(struct ath_softc *, struct ieee80211_channel *); +int ath_chan_set(struct ath_softc *, struct ieee80211_node *); void ath_draintxq(struct ath_softc *); void ath_stoprecv(struct ath_softc *); int ath_startrecv(struct ath_softc *); @@ -675,7 +675,7 @@ ath_init1(struct ath_softc *sc) */ ni = ic->ic_bss; ni->ni_chan = ic->ic_ibss_chan; - mode = ieee80211_chan2mode(ic, ni->ni_chan); + mode = ieee80211_node_abg_mode(ic, ni); if (mode != sc->sc_curmode) ath_setcurmode(sc, mode); if (ic->ic_opmode != IEEE80211_M_MONITOR) { @@ -2570,11 +2570,12 @@ ath_startrecv(struct ath_softc *sc) * ath_init. */ int -ath_chan_set(struct ath_softc *sc, struct ieee80211_channel *chan) +ath_chan_set(struct ath_softc *sc, struct ieee80211_node *ni) { struct ath_hal *ah = sc->sc_ah; struct ieee80211com *ic = &sc->sc_ic; struct ifnet *ifp = &ic->ic_if; + struct ieee80211_channel *chan = ni->ni_chan; DPRINTF(ATH_DEBUG_ANY, ("%s: %u (%u MHz) -> %u (%u MHz)\n", __func__, ieee80211_chan2ieee(ic, ic->ic_ibss_chan), @@ -2631,7 +2632,7 @@ ath_chan_set(struct ath_softc *sc, struct ieee80211_ch * if we're switching; e.g. 11a to 11b/g. */ ic->ic_ibss_chan = chan; - mode = ieee80211_chan2mode(ic, chan); + mode = ieee80211_node_abg_mode(ic, ni); if (mode != sc->sc_curmode) ath_setcurmode(sc, mode); @@ -2775,7 +2776,7 @@ ath_newstate(struct ieee80211com *ic, enum ieee80211_s return (*sc->sc_newstate)(ic, nstate, arg); } ni = ic->ic_bss; - error = ath_chan_set(sc, ni->ni_chan); + error = ath_chan_set(sc, ni); if (error != 0) goto bad; rfilt = ath_calcrxfilter(sc); blob - 8f76831260110c7d26ba6f2cb28fc333f76efa83 blob + 382c993fa6d94475165208278cc0df39d933d428 --- sys/net80211/ieee80211.c +++ sys/net80211/ieee80211.c @@ -1174,55 +1174,6 @@ ieee80211_next_mode(struct ifnet *ifp) } /* - * Return the phy mode for with the specified channel so the - * caller can select a rate set. This is problematic and the - * work here assumes how things work elsewhere in this code. - * - * Because the result of this function is ultimately used to select a - * rate from the rate set of the returned mode, it must return one of the - * legacy 11a/b/g modes; 11n and 11ac modes use MCS instead of rate sets. - */ -enum ieee80211_phymode -ieee80211_chan2mode(struct ieee80211com *ic, - const struct ieee80211_channel *chan) -{ - /* - * Are we fixed in 11a/b/g mode? - * NB: this assumes the channel would not be supplied to us - * unless it was already compatible with the current mode. - */ - if (ic->ic_curmode == IEEE80211_MODE_11A || - ic->ic_curmode == IEEE80211_MODE_11B || - ic->ic_curmode == IEEE80211_MODE_11G) - return ic->ic_curmode; - - /* If no channel was provided, return the most suitable legacy mode. */ - if (chan == IEEE80211_CHAN_ANYC) { - switch (ic->ic_curmode) { - case IEEE80211_MODE_AUTO: - case IEEE80211_MODE_11N: - if (ic->ic_modecaps & (1 << IEEE80211_MODE_11A)) - return IEEE80211_MODE_11A; - if (ic->ic_modecaps & (1 << IEEE80211_MODE_11G)) - return IEEE80211_MODE_11G; - return IEEE80211_MODE_11B; - case IEEE80211_MODE_11AC: - return IEEE80211_MODE_11A; - default: - return ic->ic_curmode; - } - } - - /* Deduce a legacy mode based on the channel characteristics. */ - if (IEEE80211_IS_CHAN_5GHZ(chan)) - return IEEE80211_MODE_11A; - else if (chan->ic_flags & (IEEE80211_CHAN_OFDM|IEEE80211_CHAN_DYN)) - return IEEE80211_MODE_11G; - else - return IEEE80211_MODE_11B; -} - -/* * Convert IEEE80211 MCS index to ifmedia subtype. */ uint64_t blob - 61ccf144a35f2ac870da130b2ae8a2f2881532b6 blob + 7400db3fd4e2ad3d8ceefff7869110f7aa007ea4 --- sys/net80211/ieee80211_input.c +++ sys/net80211/ieee80211_input.c @@ -2671,7 +2671,7 @@ ieee80211_recv_assoc_resp(struct ieee80211com *ic, str else if (ni->ni_flags & IEEE80211_NODE_HT) ieee80211_setmode(ic, IEEE80211_MODE_11N); else - ieee80211_setmode(ic, ieee80211_chan2mode(ic, ni->ni_chan)); + ieee80211_setmode(ic, ieee80211_node_abg_mode(ic, ni)); /* * Reset the erp state (mostly the slot time) now that * our operating mode has been nailed down. blob - 2e02e1fff93930e65fc955599b4f7f1d85843259 blob + a9abfb9366f3bc80f5abaaabceb9480bd06d9a5f --- sys/net80211/ieee80211_node.c +++ sys/net80211/ieee80211_node.c @@ -931,8 +931,28 @@ ieee80211_create_ibss(struct ieee80211com* ic, struct mode = IEEE80211_MODE_11AC; else if (ic->ic_flags & IEEE80211_F_HTON) mode = IEEE80211_MODE_11N; - else - mode = ieee80211_chan2mode(ic, ni->ni_chan); + else { + switch (IFM_MODE(ic->ic_media.ifm_cur->ifm_media)) { + case IFM_IEEE80211_11A: + mode = IEEE80211_MODE_11A; + break; + case IFM_IEEE80211_11G: + mode = IEEE80211_MODE_11G; + break; + case IFM_IEEE80211_11B: + mode = IEEE80211_MODE_11B; + break; + default: + if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan)) + mode = IEEE80211_MODE_11A; + else if ((ni->ni_chan->ic_flags & + (IEEE80211_CHAN_OFDM | IEEE80211_CHAN_DYN)) != 0) + mode = IEEE80211_MODE_11G; + else + mode = IEEE80211_MODE_11B; + break; + } + } ieee80211_setmode(ic, mode); /* Pick an appropriate mode for supported legacy rates. */ if (ic->ic_curmode == IEEE80211_MODE_11AC) { @@ -1303,7 +1323,7 @@ ieee80211_node_join_bss(struct ieee80211com *ic, struc uint32_t assoc_fail = 0; /* Reinitialize media mode and channels if needed. */ - mode = ieee80211_chan2mode(ic, selbs->ni_chan); + mode = ieee80211_node_abg_mode(ic, selbs); if (mode != ic->ic_curmode) ieee80211_setmode(ic, mode); @@ -1317,8 +1337,6 @@ ieee80211_node_join_bss(struct ieee80211com *ic, struc ni = ic->ic_bss; ni->ni_assoc_fail |= assoc_fail; - ic->ic_curmode = ieee80211_chan2mode(ic, ni->ni_chan); - /* Make sure we send valid rates in an association request. */ if (ic->ic_opmode == IEEE80211_M_STA) ieee80211_fix_rate(ic, ni, @@ -1561,7 +1579,7 @@ ieee80211_end_scan(struct ifnet *ifp) ieee80211_setmode(ic, IEEE80211_MODE_11N); else ieee80211_setmode(ic, - ieee80211_chan2mode(ic, ni->ni_chan)); + ieee80211_node_abg_mode(ic, ni)); return; } @@ -2641,6 +2659,40 @@ ieee80211_setup_rates(struct ieee80211com *ic, struct return ieee80211_fix_rate(ic, ni, flags); } +/* + * Return the 11a/b/g mode mutually supported for the given node. + * ni->ni_chan must be set before calling this, and ieee80211_setup_rates() + * should be called beforehand to properly differentiate 11b and 11g. + */ +enum ieee80211_phymode +ieee80211_node_abg_mode(struct ieee80211com *ic, struct ieee80211_node *ni) +{ + /* Handle the case where our own phy mode was fixed by ifconfig. */ + switch (IFM_MODE(ic->ic_media.ifm_cur->ifm_media)) { + case IFM_IEEE80211_11A: + return IEEE80211_MODE_11A; /* Peer uses 11a. */ + case IFM_IEEE80211_11B: + return IEEE80211_MODE_11B; /* Peer uses 11b. */ + case IFM_IEEE80211_11G: + /* Peer could be using either 11g or 11b, check below. */ + break; + default: + break; + } + + /* Our own phy mode is either 11G or AUTO. */ + + if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan)) + return IEEE80211_MODE_11A; + + if ((ni->ni_flags & IEEE80211_NODE_ERP) && + (ni->ni_chan->ic_flags & + (IEEE80211_CHAN_OFDM | IEEE80211_CHAN_DYN)) != 0) + return IEEE80211_MODE_11G; + + return IEEE80211_MODE_11B; +} + void ieee80211_node_trigger_addba_req(struct ieee80211_node *ni, int tid) { blob - 1544370e1d93dce970deeb793df29ecb0b58e260 blob + baebec02ff4252023680b115fc78bcdfa0ecb16b --- sys/net80211/ieee80211_node.h +++ sys/net80211/ieee80211_node.h @@ -644,6 +644,8 @@ int ieee80211_setup_vhtop(struct ieee80211_node *, con uint8_t, int); int ieee80211_setup_rates(struct ieee80211com *, struct ieee80211_node *, const u_int8_t *, const u_int8_t *, int); +enum ieee80211_phymode ieee80211_node_abg_mode(struct ieee80211com *, + struct ieee80211_node *); void ieee80211_node_trigger_addba_req(struct ieee80211_node *, int); void ieee80211_count_longslotsta(void *, struct ieee80211_node *); void ieee80211_count_nonerpsta(void *, struct ieee80211_node *); blob - c628a8b0d29698382ab9bd3fc38b8fdb4bc5f980 blob + 12665c601268bb12329555728e85f170cd1330bd --- sys/net80211/ieee80211_output.c +++ sys/net80211/ieee80211_output.c @@ -1258,7 +1258,7 @@ struct mbuf * ieee80211_get_probe_req(struct ieee80211com *ic, struct ieee80211_node *ni) { const struct ieee80211_rateset *rs = - &ic->ic_sup_rates[ieee80211_chan2mode(ic, ni->ni_chan)]; + &ic->ic_sup_rates[ieee80211_node_abg_mode(ic, ni)]; struct mbuf *m; u_int8_t *frm; blob - 3d8d649c8f0ff5e79c98fc9b6ec95d0c4d68b7f2 blob + 2463eec03cbbb297d18a40f1016e9c8800859f7f --- sys/net80211/ieee80211_proto.c +++ sys/net80211/ieee80211_proto.c @@ -221,7 +221,7 @@ ieee80211_fix_rate(struct ieee80211com *ic, struct iee error = 0; okrate = badrate = fixedrate = 0; - srs = &ic->ic_sup_rates[ieee80211_chan2mode(ic, ni->ni_chan)]; + srs = &ic->ic_sup_rates[ieee80211_node_abg_mode(ic, ni)]; nrs = &ni->ni_rates; for (i = 0; i < nrs->rs_nrates; ) { ignore = 0; @@ -1153,8 +1153,8 @@ justcleanup: /* initialize bss for probe request */ IEEE80211_ADDR_COPY(ni->ni_macaddr, etherbroadcastaddr); IEEE80211_ADDR_COPY(ni->ni_bssid, etherbroadcastaddr); - ni->ni_rates = ic->ic_sup_rates[ - ieee80211_chan2mode(ic, ni->ni_chan)]; + ni->ni_rates = ic->ic_sup_rates[ieee80211_node_abg_mode(ic, + ni)]; ni->ni_associd = 0; ni->ni_rstamp = 0; ni->ni_rsn_supp_state = RSNA_SUPP_INITIALIZE; blob - 2a427b581ccdf4a9eec573107bec2aefe4c4e06b blob + 236c37a8a241ba6228205f3902c53c0f63dea330 --- sys/net80211/ieee80211_var.h +++ sys/net80211/ieee80211_var.h @@ -496,8 +496,6 @@ int ieee80211_min_basic_rate(struct ieee80211com *); int ieee80211_max_basic_rate(struct ieee80211com *); int ieee80211_setmode(struct ieee80211com *, enum ieee80211_phymode); enum ieee80211_phymode ieee80211_next_mode(struct ifnet *); -enum ieee80211_phymode ieee80211_chan2mode(struct ieee80211com *, - const struct ieee80211_channel *); void ieee80211_disable_wep(struct ieee80211com *); void ieee80211_disable_rsn(struct ieee80211com *); int ieee80211_add_ess(struct ieee80211com *, struct ieee80211_join *);