d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Attempt to address some potential vap->iv_bss race conditions.

Browse Source 
There are unfortunately a number of situations where vap->iv_bss is changed
or freed by some code in net80211.  Because multiple threads can concurrently
be doing work (and the vap->iv_bss access isn't at all done behind any kind
of lock), it's quite possible that:

* a change will occur in one thread - eg, by a call through
  ieee80211_sta_join1();
* a state change occurs in another thread - eg an RX is scheduled
  in the ath tasklet and it calls ieee80211_input_mimo_all(), which
  does dereference vap->iv_bss;
* these two executing concurrently, causing things to explode.

Another instance is ath_beacon_alloc() which takes an ieee80211_node *.
It's called with the vap->iv_bss node from ath_newstate(). If the node has
changed in the meantime (say it's been freed elsewhere) the reference
that it grabbed _before_ refcounting it may be stale.

I would _prefer_ that these sorts of things were serialised somewhere but
that may be a bit much to ask.  Instead, the best we can (currently) hope
is that the underlying bss node is still (somewhat) valid.

There is a related PR (kern/164382) described by the first case above.
That should be fixed by properly serialising the RX path and reset path
so an RX can't occur at the same time as the vap free/shutdown path.

This is inspired by some related fixes in r212127.

PR: kern/165060
This commit is contained in:
Adrian Chadd 2012-02-13 00:28:41 +00:00
parent df01f319c7
commit 807675317e
1 changed files with 11 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
sys/dev/ath/if_ath.c
View File

@ -1669,6 +1669,7 @@ ath_bmiss_vap(struct ieee80211vap *vap)
struct ath_softc *sc = ifp->if_softc;
u_int64_t lastrx = sc->sc_lastrx;
u_int64_t tsf = ath_hal_gettsf64(sc->sc_ah);
/* XXX should take a locked ref to iv_bss */
u_int bmisstimeout =
vap->iv_bmissthreshold * vap->iv_bss->ni_intval * 1024;
@ -3245,7 +3246,7 @@ ath_beacon_config(struct ath_softc *sc, struct ieee80211vap *vap)
if (vap == NULL)
vap = TAILQ_FIRST(&ic->ic_vaps); /* XXX */
ni = vap->iv_bss;
ni = ieee80211_ref_node(vap->iv_bss);
/* extract tstamp from last beacon and convert to TU */
nexttbtt = TSF_TO_TU(LE_READ_4(ni->ni_tstamp.data + 4),
@ -3415,6 +3416,7 @@ ath_beacon_config(struct ath_softc *sc, struct ieee80211vap *vap)
ath_beacon_start_adhoc(sc, vap);
}
sc->sc_syncbeacon = 0;
ieee80211_free_node(ni);
#undef FUDGE
#undef TSF_TO_TU
}
@ -3853,6 +3855,7 @@ ath_recv_mgmt(struct ieee80211_node *ni, struct mbuf *m,
switch (subtype) {
case IEEE80211_FC0_SUBTYPE_BEACON:
/* update rssi statistics for use by the hal */
/* XXX unlocked check against vap->iv_bss? */
ATH_RSSI_LPF(sc->sc_halstats.ns_avgbrssi, rssi);
if (sc->sc_syncbeacon &&
ni == vap->iv_bss && vap->iv_state == IEEE80211_S_RUN) {
@ -5721,7 +5724,7 @@ ath_newstate(struct ieee80211vap *vap, enum ieee80211_state nstate, int arg)
taskqueue_unblock(sc->sc_tq);
}
ni = vap->iv_bss;
ni = ieee80211_ref_node(vap->iv_bss);
rfilt = ath_calcrxfilter(sc);
stamode = (vap->iv_opmode == IEEE80211_M_STA ||
vap->iv_opmode == IEEE80211_M_AHDEMO ||
@ -5752,7 +5755,8 @@ ath_newstate(struct ieee80211vap *vap, enum ieee80211_state nstate, int arg)
if (nstate == IEEE80211_S_RUN) {
/* NB: collect bss node again, it may have changed */
ni = vap->iv_bss;
ieee80211_free_node(ni);
ni = ieee80211_ref_node(vap->iv_bss);
DPRINTF(sc, ATH_DEBUG_STATE,
"%s(RUN): iv_flags 0x%08x bintvl %d bssid %s "
@ -5875,6 +5879,7 @@ ath_newstate(struct ieee80211vap *vap, enum ieee80211_state nstate, int arg)
#endif
}
bad:
ieee80211_free_node(ni);
return error;
}
@ -5893,6 +5898,7 @@ ath_setup_stationkey(struct ieee80211_node *ni)
struct ath_softc *sc = vap->iv_ic->ic_ifp->if_softc;
ieee80211_keyix keyix, rxkeyix;
/* XXX should take a locked ref to vap->iv_bss */
if (!ath_key_alloc(vap, &ni->ni_ucastkey, &keyix, &rxkeyix)) {
/*
* Key cache is full; we'll fall back to doing
@ -6448,6 +6454,7 @@ ath_tdma_config(struct ath_softc *sc, struct ieee80211vap *vap)
return;
}
}
/* XXX should take a locked ref to iv_bss */
tp = vap->iv_bss->ni_txparms;
/*
* Calculate the guard time for each slot. This is the
@ -6697,6 +6704,7 @@ ath_tdma_beacon_send(struct ath_softc *sc, struct ieee80211vap *vap)
* Record local TSF for our last send for use
* in arbitrating slot collisions.
*/
/* XXX should take a locked ref to iv_bss */
vap->iv_bss->ni_tstamp.tsf = ath_hal_gettsf64(ah);
}
}