From owner-freebsd-net@FreeBSD.ORG Fri Jan 14 18:00:24 2011 Return-Path: Delivered-To: freebsd-net@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 06913106566C for ; Fri, 14 Jan 2011 18:00:24 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id CE5B28FC15 for ; Fri, 14 Jan 2011 18:00:23 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p0EI0NIr056873 for ; Fri, 14 Jan 2011 18:00:23 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p0EI0Nv6056850; Fri, 14 Jan 2011 18:00:23 GMT (envelope-from gnats) Date: Fri, 14 Jan 2011 18:00:23 GMT Message-Id: <201101141800.p0EI0Nv6056850@freefall.freebsd.org> To: freebsd-net@FreeBSD.org From: Juergen Lock Cc: Subject: Re: kern/153938: [run] [panic] [patch] Workaround for use-after-free panic X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Juergen Lock List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Jan 2011 18:00:24 -0000 The following reply was made to PR kern/153938; it has been noted by GNATS. From: Juergen Lock To: PseudoCylon Cc: bug-followup@freebsd.org, nox@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 {