PERFORCE change 137106 for review
Sam Leffler
sam at FreeBSD.org
Fri Mar 7 19:51:50 UTC 2008
http://perforce.freebsd.org/chv.cgi?CH=137106
Change 137106 by sam at sam_ebb on 2008/03/07 19:51:41
Bandaid vap teardown races. On vap detach we block/reject use
from above by marking the ifnet OACTIVE (to block xmit) and
(hold breath for evil back) clearing if_softc so calls through
if_init and if_ioctl can be ignored/rejected. This fixes destroy
of a vap being controlled by wpa_supplicant--previously we would
clock the vap to the INIT state and wpa_supplicant would wakeup
and immediately try to scan which raced against the teardown.
The better way to deal with this would seem to be destroying
the ifnet before the net80211 state so nothing is visible to
user space but that is hard because much code assumes this does
not happen and changing that assumption may be costly (e.g.
additional checks in fast paths).
Affected files ...
.. //depot/projects/vap/sys/net80211/ieee80211.c#28 edit
.. //depot/projects/vap/sys/net80211/ieee80211_ioctl.c#38 edit
.. //depot/projects/vap/sys/net80211/ieee80211_proto.c#23 edit
.. //depot/projects/vap/sys/net80211/ieee80211_proto.h#17 edit
Differences ...
==== //depot/projects/vap/sys/net80211/ieee80211.c#28 (text+ko) ====
@@ -462,7 +462,10 @@
}
/*
- * Tear down vap state prior to reclaiming the ifnet.
+ * Tear down vap state and reclaim the ifnet.
+ * The driver is assumed to have prepared for
+ * this; e.g. by turning off interrupts for the
+ * underlying device.
*/
void
ieee80211_vap_detach(struct ieee80211vap *vap)
@@ -474,17 +477,24 @@
__func__, ieee80211_opmode_name[vap->iv_opmode],
ic->ic_ifp->if_xname);
+ IEEE80211_LOCK(ic);
+ /* block traffic from above */
+ ifp->if_drv_flags |= IFF_DRV_OACTIVE;
+ /*
+ * Evil hack. Clear the backpointer from the ifnet to the
+ * vap so any requests from above will return an error or
+ * be ignored. In particular this short-circuits requests
+ * by the bridge to turn off promiscuous mode as a result
+ * of calling ether_ifdetach.
+ */
+ ifp->if_softc = NULL;
/*
- * Mark interface down so we ignore calls by the bridge
- * to turn off promiscuous mode as a result of calling
- * ether_ifdetach.
+ * Stop the vap before detaching the ifnet. Ideally we'd
+ * do this in the other order so the ifnet is inaccessible
+ * while we cleanup internal state but that is hard.
*/
- ifp->if_flags &= ~IFF_UP;
- ieee80211_stop(vap);
- bpfdetach(ifp);
- ether_ifdetach(ifp);
+ ieee80211_stop_locked(vap);
- IEEE80211_LOCK(ic);
/* XXX accumulate iv_stats in ic_stats? */
TAILQ_REMOVE(&ic->ic_vaps, vap, iv_next);
ieee80211_syncflag_locked(ic, IEEE80211_F_WME);
@@ -497,6 +507,10 @@
ieee80211_syncifflag_locked(ic, IFF_ALLMULTI);
IEEE80211_UNLOCK(ic);
+ /* XXX can't hold com lock */
+ /* NB: bpfattach is called by ether_ifdetach and claims all taps */
+ ether_ifdetach(ifp);
+
ifmedia_removeall(&vap->iv_media);
ieee80211_regdomain_vdetach(vap);
==== //depot/projects/vap/sys/net80211/ieee80211_ioctl.c#38 (text+ko) ====
@@ -3127,19 +3127,33 @@
int
ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
{
- struct ieee80211vap *vap = ifp->if_softc;
- struct ieee80211com *ic = vap->iv_ic;
- struct ifnet *parent = ic->ic_ifp;
+ struct ieee80211vap *vap;
+ struct ieee80211com *ic;
+ struct ifnet *parent;
int error = 0;
struct ifreq *ifr;
struct ifaddr *ifa; /* XXX */
+ vap = ifp->if_softc;
+ if (vap == NULL) {
+ /*
+ * During detach we clear the backpointer in the softc
+ * so any ioctl request through the ifnet that arrives
+ * before teardown is ignored/rejected. In particular
+ * this hack handles destroying a vap used by an app
+ * like wpa_supplicant that will respond to the vap
+ * being forced into INIT state by immediately trying
+ * to force it back up. We can yank this hack if/when
+ * we can destroy the ifnet before cleaning up vap state.
+ */
+ return ENXIO;
+ }
switch (cmd) {
case SIOCSIFFLAGS:
+ ic = vap->iv_ic;
IEEE80211_LOCK(ic);
ieee80211_syncifflag_locked(ic, IFF_PROMISC);
ieee80211_syncifflag_locked(ic, IFF_ALLMULTI);
- IEEE80211_UNLOCK(ic);
if (ifp->if_flags & IFF_UP) {
/*
* Bring ourself up unless we're already operational.
@@ -3148,18 +3162,20 @@
* side-effect of bringing ourself up.
*/
if (vap->iv_state == IEEE80211_S_INIT)
- ieee80211_init(vap);
+ ieee80211_start_locked(vap);
} else if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
/*
* Stop ourself. If we are the last vap to be
* marked down the parent will also be taken down.
*/
- ieee80211_stop(vap);
+ ieee80211_stop_locked(vap);
}
+ IEEE80211_UNLOCK(ic);
break;
case SIOCADDMULTI:
case SIOCDELMULTI:
/* XXX merge vap lists into parent */
+ parent = vap->iv_ic->ic_ifp;
if (parent->if_drv_flags & IFF_DRV_RUNNING) {
/* XXX propagate multicast address to device */
error = parent->if_ioctl(parent, cmd, data);
==== //depot/projects/vap/sys/net80211/ieee80211_proto.c#23 (text+ko) ====
@@ -1075,7 +1075,7 @@
* set running on the underlying device then we
* automatically bring the device up.
*/
-static void
+void
ieee80211_start_locked(struct ieee80211vap *vap)
{
struct ifnet *ifp = vap->iv_ifp;
@@ -1161,14 +1161,23 @@
ieee80211_init(void *arg)
{
struct ieee80211vap *vap = arg;
- struct ieee80211com *ic = vap->iv_ic;
- IEEE80211_DPRINTF(vap,
- IEEE80211_MSG_STATE | IEEE80211_MSG_DEBUG, "%s\n", __func__);
+ /*
+ * This routine is publicly accessible through the vap's
+ * if_init method so guard against calls during detach.
+ * ieee80211_vap_detach null's the backpointer before
+ * tearing down state to signal any callback should be
+ * rejected/ignored.
+ */
+ if (vap != NULL) {
+ IEEE80211_DPRINTF(vap,
+ IEEE80211_MSG_STATE | IEEE80211_MSG_DEBUG,
+ "%s\n", __func__);
- IEEE80211_LOCK(ic);
- ieee80211_start_locked(vap);
- IEEE80211_UNLOCK(ic);
+ IEEE80211_LOCK(vap->iv_ic);
+ ieee80211_start_locked(vap);
+ IEEE80211_UNLOCK(vap->iv_ic);
+ }
}
/*
@@ -1196,16 +1205,17 @@
* next vap is brought up.
*/
void
-ieee80211_stop(struct ieee80211vap *vap)
+ieee80211_stop_locked(struct ieee80211vap *vap)
{
struct ieee80211com *ic = vap->iv_ic;
struct ifnet *ifp = vap->iv_ifp;
struct ifnet *parent = ic->ic_ifp;
+ IEEE80211_LOCK_ASSERT(ic);
+
IEEE80211_DPRINTF(vap, IEEE80211_MSG_STATE | IEEE80211_MSG_DEBUG,
"stop running, %d vaps running\n", ic->ic_nrunning);
- IEEE80211_LOCK(ic);
ieee80211_new_state_locked(vap, IEEE80211_S_INIT, -1);
if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
ifp->if_drv_flags &= ~IFF_DRV_RUNNING; /* mark us stopped */
@@ -1215,10 +1225,21 @@
IEEE80211_MSG_STATE | IEEE80211_MSG_DEBUG,
"down parent %s\n", parent->if_xname);
parent->if_flags &= ~IFF_UP;
- /* XXX holding lock */
+ /* XXX must drop lock */
+ IEEE80211_UNLOCK(ic);
parent->if_ioctl(parent, SIOCSIFFLAGS, NULL);
+ IEEE80211_LOCK(ic);
}
}
+}
+
+void
+ieee80211_stop(struct ieee80211vap *vap)
+{
+ struct ieee80211com *ic = vap->iv_ic;
+
+ IEEE80211_LOCK(ic);
+ ieee80211_stop_locked(vap);
IEEE80211_UNLOCK(ic);
}
==== //depot/projects/vap/sys/net80211/ieee80211_proto.h#17 (text+ko) ====
@@ -253,8 +253,10 @@
return tid;
}
+void ieee80211_start_locked(struct ieee80211vap *);
void ieee80211_init(void *);
void ieee80211_start_all(struct ieee80211com *);
+void ieee80211_stop_locked(struct ieee80211vap *);
void ieee80211_stop(struct ieee80211vap *);
void ieee80211_stop_all(struct ieee80211com *);
void ieee80211_dturbo_switch(struct ieee80211vap *, int newflags);
More information about the p4-projects
mailing list