From 97b3b273d7ac138b701a1fe9d0caeebea50896ad Mon Sep 17 00:00:00 2001 From: avos Date: Sun, 20 Jan 2019 13:39:18 +0000 Subject: [PATCH] net80211: resolve ioctl <-> detach race for ieee80211com structure Since r287197 ieee80211com is a part of drivers softc; as a result, after detach all pointers to it (iv_ic, ni_ic) are invalid. Most possible users (tasks, interrupt handlers) are blocked / removed when device is stopped; however, ioctl handlers were not tracked and may crash if ieee80211com structure is accessed. Since ieee80211com pointer access from ieee80211vap structure is not protected by lock (constant after interface creation) and used in many other places just use reference counting for ioctl handlers; on detach set 'detached' flag and wait until reference counter goes to 0. For HEAD ieee80211vap size was changed (__FreeBSD_version bumped); however, in stable branches I'm going to split / reuse the last iv_spare field for KBI stability. Tested with: - rsu(4), SIOCSIFCAP (-rxcsum) ioctl; - rtwn_pci(4), SIOCG80211 / IEEE80211_IOC_HTPROTMODE ioctl. MFC after: 1 week --- sys/net80211/ieee80211.c | 4 ++- sys/net80211/ieee80211_freebsd.c | 49 ++++++++++++++++++++++++++++++++ sys/net80211/ieee80211_freebsd.h | 6 +++- sys/net80211/ieee80211_ioctl.c | 10 ++++++- sys/net80211/ieee80211_var.h | 7 +++++ sys/sys/param.h | 2 +- 6 files changed, 74 insertions(+), 4 deletions(-) diff --git a/sys/net80211/ieee80211.c b/sys/net80211/ieee80211.c index 658ccc20ccc..a0b36f2866b 100644 --- a/sys/net80211/ieee80211.c +++ b/sys/net80211/ieee80211.c @@ -405,8 +405,10 @@ ieee80211_ifdetach(struct ieee80211com *ic) * The VAP is responsible for setting and clearing * the VIMAGE context. */ - while ((vap = TAILQ_FIRST(&ic->ic_vaps)) != NULL) + while ((vap = TAILQ_FIRST(&ic->ic_vaps)) != NULL) { + ieee80211_com_vdetach(vap); ieee80211_vap_destroy(vap); + } ieee80211_waitfor_parent(ic); ieee80211_sysctl_detach(ic); diff --git a/sys/net80211/ieee80211_freebsd.c b/sys/net80211/ieee80211_freebsd.c index b33969a0b11..925872593fa 100644 --- a/sys/net80211/ieee80211_freebsd.c +++ b/sys/net80211/ieee80211_freebsd.c @@ -307,6 +307,55 @@ ieee80211_sysctl_vdetach(struct ieee80211vap *vap) } } +#define MS(_v, _f) (((_v) & _f##_M) >> _f##_S) +int +ieee80211_com_vincref(struct ieee80211vap *vap) +{ + uint32_t ostate; + + ostate = atomic_fetchadd_32(&vap->iv_com_state, IEEE80211_COM_REF_ADD); + + if (ostate & IEEE80211_COM_DETACHED) { + atomic_subtract_32(&vap->iv_com_state, IEEE80211_COM_REF_ADD); + return (ENETDOWN); + } + + if (MS(ostate, IEEE80211_COM_REF) == IEEE80211_COM_REF_MAX) { + atomic_subtract_32(&vap->iv_com_state, IEEE80211_COM_REF_ADD); + return (EOVERFLOW); + } + + return (0); +} + +void +ieee80211_com_vdecref(struct ieee80211vap *vap) +{ + uint32_t ostate; + + ostate = atomic_fetchadd_32(&vap->iv_com_state, -IEEE80211_COM_REF_ADD); + + KASSERT(MS(ostate, IEEE80211_COM_REF) != 0, + ("com reference counter underflow")); + + (void) ostate; +} + +void +ieee80211_com_vdetach(struct ieee80211vap *vap) +{ + int sleep_time; + + sleep_time = msecs_to_ticks(250); + if (sleep_time == 0) + sleep_time = 1; + + atomic_set_32(&vap->iv_com_state, IEEE80211_COM_DETACHED); + while (MS(atomic_load_32(&vap->iv_com_state), IEEE80211_COM_REF) != 0) + pause("comref", sleep_time); +} +#undef MS + int ieee80211_node_dectestref(struct ieee80211_node *ni) { diff --git a/sys/net80211/ieee80211_freebsd.h b/sys/net80211/ieee80211_freebsd.h index 8395eb00896..a70de1086bf 100644 --- a/sys/net80211/ieee80211_freebsd.h +++ b/sys/net80211/ieee80211_freebsd.h @@ -224,6 +224,11 @@ typedef struct mtx ieee80211_rt_lock_t; */ #include +struct ieee80211vap; +int ieee80211_com_vincref(struct ieee80211vap *); +void ieee80211_com_vdecref(struct ieee80211vap *); +void ieee80211_com_vdetach(struct ieee80211vap *); + #define ieee80211_node_initref(_ni) \ do { ((_ni)->ni_refcnt = 1); } while (0) #define ieee80211_node_incref(_ni) \ @@ -235,7 +240,6 @@ int ieee80211_node_dectestref(struct ieee80211_node *ni); #define ieee80211_node_refcnt(_ni) (_ni)->ni_refcnt struct ifqueue; -struct ieee80211vap; void ieee80211_drain_ifq(struct ifqueue *); void ieee80211_flush_ifq(struct ifqueue *, struct ieee80211vap *); diff --git a/sys/net80211/ieee80211_ioctl.c b/sys/net80211/ieee80211_ioctl.c index e19f4fcfad3..7241a95bf8d 100644 --- a/sys/net80211/ieee80211_ioctl.c +++ b/sys/net80211/ieee80211_ioctl.c @@ -3480,10 +3480,14 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) { struct ieee80211vap *vap = ifp->if_softc; struct ieee80211com *ic = vap->iv_ic; - int error = 0, wait = 0; + int error = 0, wait = 0, ic_used; struct ifreq *ifr; struct ifaddr *ifa; /* XXX */ + ic_used = (cmd != SIOCSIFMTU && cmd != SIOCG80211STATS); + if (ic_used && (error = ieee80211_com_vincref(vap)) != 0) + return (error); + switch (cmd) { case SIOCSIFFLAGS: IEEE80211_LOCK(ic); @@ -3620,5 +3624,9 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) error = ether_ioctl(ifp, cmd, data); break; } + + if (ic_used) + ieee80211_com_vdecref(vap); + return (error); } diff --git a/sys/net80211/ieee80211_var.h b/sys/net80211/ieee80211_var.h index ee17c806dc3..0839baef885 100644 --- a/sys/net80211/ieee80211_var.h +++ b/sys/net80211/ieee80211_var.h @@ -400,6 +400,7 @@ struct ieee80211vap { uint32_t iv_caps; /* capabilities */ uint32_t iv_htcaps; /* HT capabilities */ uint32_t iv_htextcaps; /* HT extended capabilities */ + uint32_t iv_com_state; /* com usage / detached flag */ enum ieee80211_opmode iv_opmode; /* operation mode */ enum ieee80211_state iv_state; /* state machine state */ enum ieee80211_state iv_nstate; /* pending state */ @@ -685,6 +686,12 @@ MALLOC_DECLARE(M_80211_VAP); #define IEEE80211_VFHT_BITS \ "\20\1VHT\2VHT40\3VHT80\4VHT80P80\5VHT160" +#define IEEE80211_COM_DETACHED 0x00000001 /* ieee80211_ifdetach called */ +#define IEEE80211_COM_REF_ADD 0x00000002 /* add / remove reference */ +#define IEEE80211_COM_REF_M 0xfffffffe /* reference counter bits */ +#define IEEE80211_COM_REF_S 1 +#define IEEE80211_COM_REF_MAX (IEEE80211_COM_REF_M >> IEEE80211_COM_REF_S) + int ic_printf(struct ieee80211com *, const char *, ...) __printflike(2, 3); void ieee80211_ifattach(struct ieee80211com *); void ieee80211_ifdetach(struct ieee80211com *); diff --git a/sys/sys/param.h b/sys/sys/param.h index b8656ed3288..291e54da8b7 100644 --- a/sys/sys/param.h +++ b/sys/sys/param.h @@ -60,7 +60,7 @@ * in the range 5 to 9. */ #undef __FreeBSD_version -#define __FreeBSD_version 1300008 /* Master, propagated to newvers */ +#define __FreeBSD_version 1300009 /* Master, propagated to newvers */ /* * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD, -- 2.45.0