kern/153938: [run] [panic] [patch] Workaround for use-after-free panic

[Juergen Lock]

The following reply was made to PR kern/153938; it has been noted by GNATS.

From: Juergen Lock <nox at jelal.kn-bremen.de>
To: PseudoCylon <moonlightakkiy at yahoo.ca>
Cc: bug-followup at freebsd.org, nox at jelal.kn-bremen.de
Subject: Re: kern/153938: [run] [panic] [patch] Workaround for use-after-free
 panic
Date: Fri, 14 Jan 2011 18:36:50 +0100

 On Thu, Jan 13, 2011 at 04:47:21PM -0800, PseudoCylon wrote:
 > Hello,
 Hi!
 > 
 > Thank you for the patch.
 > 
  You're welcome! :)
 
 > I have applied it. Please try patched driver out.
 > http://gitorious.org/run/run/trees/ratectl_fix/dev/usb/wlan
 > 
 > I added locks to your patch, so saved pointers are more reliable.
 
  I see you removed the rn->wcid code, I guess I should have
 explained what it's for:  ni->ni_associd already gets zeroed before
 run_node_cleanup() is called so with your version no sc->sc_ni[wcid]
 ever gets set to NULL.
 
  New patch against git, this time with all the diagnostic code in #if 1:
 
 diff -upr run-run/dev/usb/wlan/if_run.c src/sys/dev/usb/wlan/if_run.c
 --- run-run/dev/usb/wlan/if_run.c	2011-01-14 00:35:23.000000000 +0100
 +++ src/sys/dev/usb/wlan/if_run.c	2011-01-14 17:17:50.000000000 +0100
 @@ -1694,12 +1694,12 @@ static void
  run_node_cleanup(struct ieee80211_node *ni)
  {
  	struct run_softc *sc = ni->ni_vap->iv_ic->ic_ifp->if_softc;
 +	struct run_node *rn = (void *)ni;
  	uint8_t wcid;
  
  	wcid = RUN_AID2WCID(ni->ni_associd);
  
 -	/* sc_ni[0] is not used */
 -	if ((wcid == 0) || (wcid > RT2870_WCID_MAX))
 +	if (wcid > RT2870_WCID_MAX)
  		goto done;
  
  	/*
 @@ -1707,13 +1707,27 @@ run_node_cleanup(struct ieee80211_node *
  	 * so lock conditionally
  	 */
  	if (mtx_owned(&sc->sc_mtx))
 -		sc->sc_ni[wcid] = NULL;
 +		if (wcid == 0)
 +			wcid = rn->wcid;
 +		/* sc_ni[0] is not used */
 +		if (wcid != 0 && wcid <= RT2870_WCID_MAX)
 +			sc->sc_ni[wcid] = NULL;
  	else {
  		RUN_LOCK(sc);
 -		sc->sc_ni[wcid] = NULL;
 +		if (wcid == 0)
 +			wcid = rn->wcid;
 +		/* sc_ni[0] is not used */
 +		if (wcid != 0 && wcid <= RT2870_WCID_MAX)
 +			sc->sc_ni[wcid] = NULL;
  		RUN_UNLOCK(sc);
  	}
  
 +#if 1
 +	device_printf(sc->sc_dev, "node_cleanup wcid=%d addr=%s ni=%p\n",
 +		wcid, ether_sprintf(ni->ni_vap->iv_opmode == IEEE80211_M_STA ?
 +			ni->ni_vap->iv_myaddr : ni->ni_macaddr), ni);
 +#endif
 +
  done:
  	sc->sc_node_cleanup(ni);
  }
 @@ -2279,6 +2293,13 @@ run_drain_fifo(void *arg)
  		    wcid == 0)
  			continue;
  
 +#if 1
 +		static struct ieee80211_node *lastni;
 +		if ((ni = sc->sc_ni[wcid]) == NULL && lastni)
 +			device_printf(sc->sc_dev, "drain_fifo ni=NULL wcid=%d\n",
 +				wcid);
 +		lastni = ni;
 +#endif
  		if ((sc->sc_ni[wcid] == NULL) ||
  		    (sc->sc_ni[wcid]->ni_rctls == NULL))
  			continue;
 @@ -2371,6 +2392,7 @@ run_newassoc_cb(void *arg)
  {
  	struct run_cmdq *cmdq = arg;
  	struct ieee80211_node *ni = cmdq->arg1;
 +	struct run_node *rn = (void *)ni;
  	struct run_softc *sc = ni->ni_vap->iv_ic->ic_ifp->if_softc;
  	uint8_t wcid = cmdq->wcid;
  
 @@ -2379,6 +2401,7 @@ run_newassoc_cb(void *arg)
  	run_write_region_1(sc, RT2860_WCID_ENTRY(wcid),
  	    ni->ni_macaddr, IEEE80211_ADDR_LEN);
  
 +	rn->wcid = wcid;
  	sc->sc_ni[wcid] = ni;
  }
  
 @@ -2416,8 +2439,13 @@ run_newassoc(struct ieee80211_node *ni, 
  		ieee80211_runtask(ic, &sc->cmdq_task);
  	}
  
 +#if 1
 +	device_printf(sc->sc_dev, "new assoc isnew=%d associd=%x addr=%s ni=%p\n",
 +	    isnew, ni->ni_associd, ether_sprintf(ni->ni_macaddr), ni);
 +#else
  	DPRINTF("new assoc isnew=%d associd=%x addr=%s\n",
  	    isnew, ni->ni_associd, ether_sprintf(ni->ni_macaddr));
 +#endif
  
  	ieee80211_ratectl_node_init(ni);
  
 diff -upr run-run/dev/usb/wlan/if_runvar.h src/sys/dev/usb/wlan/if_runvar.h
 --- run-run/dev/usb/wlan/if_runvar.h	2011-01-14 00:35:23.000000000 +0100
 +++ src/sys/dev/usb/wlan/if_runvar.h	2011-01-14 16:56:41.000000000 +0100
 @@ -106,6 +106,7 @@ struct run_node {
  	uint8_t			amrr_ridx;
  	uint8_t			mgt_ridx;
  	uint8_t			fix_ridx;
 +	uint8_t			wcid;
  };
  
  struct run_cmdq {