From owner-svn-src-all@FreeBSD.ORG Sun Oct 2 02:42:31 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B0850106566C; Sun, 2 Oct 2011 02:42:31 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 96E0F8FC08; Sun, 2 Oct 2011 02:42:31 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p922gVYo044003; Sun, 2 Oct 2011 02:42:31 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p922gV3S043999; Sun, 2 Oct 2011 02:42:31 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201110020242.p922gV3S043999@svn.freebsd.org> From: Adrian Chadd Date: Sun, 2 Oct 2011 02:42:31 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r225913 - head/sys/net80211 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Oct 2011 02:42:31 -0000 Author: adrian Date: Sun Oct 2 02:42:31 2011 New Revision: 225913 URL: http://svn.freebsd.org/changeset/base/225913 Log: Fix a panic in the wifi stack when a software beacon miss occurs in the wrong state. The ieee80211_swbmiss() callout is not called with the ic lock held, so it's quite possible the scheduler will run the callout during a state change. This patch: * changes the swbmiss callout to be locked by the ic lock * enforces the ic lock being held across the beacon vap functions by grabbing it inside beacon_miss() and beacon_swmiss(). This ensures that the ic lock is held (and thus the VAP state stays constant) during beacon miss and software miss processing. Since the callout is removed whilst the ic lock is held, it also ensures that the ic lock can't be called during a state change or exhibit any race conditions seen above. Both Edgar and Joel report that this patch fixes the crash and doesn't introduce new issues. Reported by: Edgar Martinez Reported by: Joel Dahl Reported by: emaste Modified: head/sys/net80211/ieee80211_proto.c head/sys/net80211/ieee80211_sta.c head/sys/net80211/ieee80211_tdma.c Modified: head/sys/net80211/ieee80211_proto.c ============================================================================== --- head/sys/net80211/ieee80211_proto.c Sat Oct 1 23:47:37 2011 (r225912) +++ head/sys/net80211/ieee80211_proto.c Sun Oct 2 02:42:31 2011 (r225913) @@ -193,7 +193,7 @@ ieee80211_proto_vattach(struct ieee80211 vap->iv_rtsthreshold = IEEE80211_RTS_DEFAULT; vap->iv_fragthreshold = IEEE80211_FRAG_DEFAULT; vap->iv_bmiss_max = IEEE80211_BMISS_MAX; - callout_init(&vap->iv_swbmiss, CALLOUT_MPSAFE); + callout_init_mtx(&vap->iv_swbmiss, IEEE80211_LOCK_OBJ(ic), 0); callout_init(&vap->iv_mgtsend, CALLOUT_MPSAFE); TASK_INIT(&vap->iv_nstate_task, 0, ieee80211_newstate_cb, vap); TASK_INIT(&vap->iv_swbmiss_task, 0, beacon_swmiss, vap); @@ -1403,7 +1403,7 @@ beacon_miss(void *arg, int npending) struct ieee80211com *ic = arg; struct ieee80211vap *vap; - /* XXX locking */ + IEEE80211_LOCK(ic); TAILQ_FOREACH(vap, &ic->ic_vaps, iv_next) { /* * We only pass events through for sta vap's in RUN state; @@ -1415,18 +1415,21 @@ beacon_miss(void *arg, int npending) vap->iv_bmiss != NULL) vap->iv_bmiss(vap); } + IEEE80211_UNLOCK(ic); } static void beacon_swmiss(void *arg, int npending) { struct ieee80211vap *vap = arg; + struct ieee80211com *ic = vap->iv_ic; - if (vap->iv_state != IEEE80211_S_RUN) - return; - - /* XXX Call multiple times if npending > zero? */ - vap->iv_bmiss(vap); + IEEE80211_LOCK(ic); + if (vap->iv_state == IEEE80211_S_RUN) { + /* XXX Call multiple times if npending > zero? */ + vap->iv_bmiss(vap); + } + IEEE80211_UNLOCK(ic); } /* @@ -1440,6 +1443,8 @@ ieee80211_swbmiss(void *arg) struct ieee80211vap *vap = arg; struct ieee80211com *ic = vap->iv_ic; + IEEE80211_LOCK_ASSERT(ic); + /* XXX sleep state? */ KASSERT(vap->iv_state == IEEE80211_S_RUN, ("wrong state %d", vap->iv_state)); Modified: head/sys/net80211/ieee80211_sta.c ============================================================================== --- head/sys/net80211/ieee80211_sta.c Sat Oct 1 23:47:37 2011 (r225912) +++ head/sys/net80211/ieee80211_sta.c Sun Oct 2 02:42:31 2011 (r225913) @@ -109,6 +109,8 @@ sta_beacon_miss(struct ieee80211vap *vap { struct ieee80211com *ic = vap->iv_ic; + IEEE80211_LOCK_ASSERT(ic); + KASSERT((ic->ic_flags & IEEE80211_F_SCAN) == 0, ("scanning")); KASSERT(vap->iv_state >= IEEE80211_S_RUN, ("wrong state %s", ieee80211_state_name[vap->iv_state])); Modified: head/sys/net80211/ieee80211_tdma.c ============================================================================== --- head/sys/net80211/ieee80211_tdma.c Sat Oct 1 23:47:37 2011 (r225912) +++ head/sys/net80211/ieee80211_tdma.c Sun Oct 2 02:42:31 2011 (r225913) @@ -285,6 +285,9 @@ static void tdma_beacon_miss(struct ieee80211vap *vap) { struct ieee80211_tdma_state *ts = vap->iv_tdma; + struct ieee80211com *ic = vap->iv_ic; + + IEEE80211_LOCK_ASSERT(ic); KASSERT((vap->iv_ic->ic_flags & IEEE80211_F_SCAN) == 0, ("scanning")); KASSERT(vap->iv_state == IEEE80211_S_RUN,