[panic] Race in IEEE802.11 layer towards device drivers

[PseudoCylon]

----- Original Message ----
> From: Hans Petter Selasky <hselasky at c2i.net>
> To: freebsd-current at freebsd.org
> Cc: Andrew Thompson <thompsa at freebsd.org>; Sam Leffler <sam at freebsd.org>; 
>PseudoCylon <moonlightakkiy at yahoo.ca>; freebsd-usb at freebsd.org
> Sent: Mon, July 12, 2010 2:01:11 PM
> Subject: Re: [panic] Race in IEEE802.11 layer towards device drivers
> 
> Hi Andrew,
> 
> Your patch appears to be working. Can you fix this issue in  the other WLAN 
> drivers aswell? Then send an e-mail to request testing? I had  a go at it 
here:
> 
> http://p4web.freebsd.org/@@180844?ac=10
> 

Can this patch be included into testing? patch to P4 USB rev. 14 if_run.c

If not, at least please delete an extra semicolon at the end of line 3191 (must 
be an merge bug). I missed it since it wans't in diff/patch.


AK


summary of changes
* fixes bugs in rev 209144
   a shared key was written properly only on first time init, but not subsequent 
init. Make sure the key is written all the time.
* stop checking 'pending' in run_cmdq_cb().
   When loop 'pending' times, new tasks enqueued while looping won't be executed 
because 'pending' passed from taskqueue function won't be incremented.

-- patch begin --

diff --git a/dev/usb/wlan/if_run.c b/dev/usb/wlan/if_run.c
index 7a3952c..12b45ec 100644
--- a/dev/usb/wlan/if_run.c
+++ b/dev/usb/wlan/if_run.c
@@ -830,9 +830,6 @@ run_vap_create(struct ieee80211com *ic,
 if(sc->rvp_cnt++ == 0)
 ic->ic_opmode = opmode;
 
-if(opmode == IEEE80211_M_HOSTAP)
-sc->cmdq_run = RUN_CMDQ_GO;
-
 DPRINTF("rvp_id=%d bmap=%x rvp_cnt=%d\n",
     rvp->rvp_id, sc->rvp_bmap, sc->rvp_cnt);
 
@@ -891,15 +888,16 @@ run_cmdq_cb(void *arg, int pending)
 
 /* call cmdq[].func locked */
 RUN_LOCK(sc);
-for(i = sc->cmdq_exec; sc->cmdq[i].func && pending;
-    i = sc->cmdq_exec, pending--){
+for (i = sc->cmdq_exec; sc->cmdq[i].func; i = sc->cmdq_exec) {
 DPRINTFN(6, "cmdq_exec=%d pending=%d\n", i, pending);
-if(sc->cmdq_run == RUN_CMDQ_GO){
+if (sc->cmdq_run == RUN_CMDQ_GO ||
+    (sc->cmdq_key_set == RUN_CMDQ_GO &&
+    sc->cmdq[i].func == run_key_set_cb)) {
 /*
  * If arg0 is NULL, callback func needs more
  * than one arg. So, pass ptr to cmdq struct.
  */
-if(sc->cmdq[i].arg0)
+if (sc->cmdq[i].arg0)
 sc->cmdq[i].func(sc->cmdq[i].arg0);
 else
 sc->cmdq[i].func(&sc->cmdq[i]);
@@ -1771,6 +1769,19 @@ run_newstate(struct ieee80211vap *vap, enum 
ieee80211_state nstate, int arg)
 case IEEE80211_S_INIT:
 restart_ratectl = 1;
 
+/*
+ * When hostapd has set a key, don't clear it.
+ * But, when the device is being brought down, clear it.
+ */
+if (sc->cmdq_key_set != RUN_CMDQ_GO ||
+    ostate == IEEE80211_S_RUN) {
+/* clear shared key table */
+run_set_region_4(sc,
+    RT2860_SKEY(rvp->rvp_id, 0), 0, 4 * 32);
+/* clear shared key mode */
+run_set_region_4(sc, RT2860_SKEY_MODE_0_7, 0, 4);
+}
+
 if (ostate != IEEE80211_S_RUN)
 break;
 
@@ -2100,13 +2111,10 @@ run_key_set(struct ieee80211vap *vap, struct 
ieee80211_key *k,
  * To make sure key will be set when hostapd
  * calls iv_key_set() before if_init().
  */
-if(vap->iv_opmode == IEEE80211_M_HOSTAP){
-RUN_LOCK(sc);
+if (vap->iv_opmode == IEEE80211_M_HOSTAP)
 sc->cmdq_key_set = RUN_CMDQ_GO;
-RUN_UNLOCK(sc);
-}
 
-return(1);
+return (1);
 }
 
 /*
@@ -3188,7 +3196,7 @@ run_sendprot(struct run_softc *sc,
 ackrate = ieee80211_ack_rate(ic->ic_rt, rate);
 
 isshort = (ic->ic_flags & IEEE80211_F_SHPREAMBLE) != 0;
-dur = ieee80211_compute_duration(ic->ic_rt, pktlen, rate, isshort);
+dur = ieee80211_compute_duration(ic->ic_rt, pktlen, rate, isshort)
     + ieee80211_ack_duration(ic->ic_rt, rate, isshort);
 wflags = RT2860_TX_FRAG;
 
@@ -4693,14 +4701,6 @@ run_init_locked(struct run_softc *sc)
 /* clear WCID attribute table */
 run_set_region_4(sc, RT2860_WCID_ATTR(0), 0, 8 * 32);
 
-/* hostapd sets a key before init. So, don't clear it. */
-if(sc->cmdq_key_set != RUN_CMDQ_GO){
-/* clear shared key table */
-run_set_region_4(sc, RT2860_SKEY(0, 0), 0, 8 * 32);
-/* clear shared key mode */
-run_set_region_4(sc, RT2860_SKEY_MODE_0_7, 0, 4);
-}
-
 run_read(sc, RT2860_US_CYC_CNT, &tmp);
 tmp = (tmp & ~0xff) | 0x1e;
 run_write(sc, RT2860_US_CYC_CNT, tmp);
@@ -4807,7 +4807,7 @@ run_stop(void *arg)
 ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
 
 sc->ratectl_run = RUN_RATECTL_OFF;
-sc->cmdq_run = sc->cmdq_key_set;
+sc->cmdq_run = RUN_CMDQ_ABORT;
 
 RUN_UNLOCK(sc);
 
-- end patch --