From owner-svn-src-head@FreeBSD.ORG Wed May 20 14:02:21 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6BC4110656A9; Wed, 20 May 2009 14:02:21 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 3D61D8FC1C; Wed, 20 May 2009 14:02:21 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id E78D646B29; Wed, 20 May 2009 10:02:20 -0400 (EDT) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 5A6B68A026; Wed, 20 May 2009 10:02:16 -0400 (EDT) From: John Baldwin To: Weongyo Jeong Date: Wed, 20 May 2009 08:26:02 -0400 User-Agent: KMail/1.9.7 References: <200905200349.n4K3nHSe018257@svn.freebsd.org> In-Reply-To: <200905200349.n4K3nHSe018257@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200905200826.03063.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Wed, 20 May 2009 10:02:16 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r192419 - head/sys/dev/usb/wlan X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 May 2009 14:02:22 -0000 On Tuesday 19 May 2009 11:49:17 pm Weongyo Jeong wrote: > Author: weongyo > Date: Wed May 20 03:49:16 2009 > New Revision: 192419 > URL: http://svn.freebsd.org/changeset/base/192419 > > Log: > try to unsetup USB xfers before calling ieee80211_ifdetach() to fix a > bug referencing a destroyed lock within TX callbacks during device > detach. > > Submitted by: hps (original version) > Tested by: Lucius Windschuh > > Modified: > head/sys/dev/usb/wlan/if_uath.c > head/sys/dev/usb/wlan/if_upgt.c > > Modified: head/sys/dev/usb/wlan/if_uath.c > ============================================================================== > --- head/sys/dev/usb/wlan/if_uath.c Wed May 20 03:33:27 2009 (r192418) > +++ head/sys/dev/usb/wlan/if_uath.c Wed May 20 03:49:16 2009 (r192419) > @@ -517,12 +517,12 @@ uath_detach(device_t dev) > > sc->sc_flags |= UATH_FLAG_INVALID; > uath_stop(ifp); > - ieee80211_ifdetach(ic); > > callout_drain(&sc->stat_ch); > callout_drain(&sc->watchdog_ch); > > usb2_transfer_unsetup(sc->sc_xfer, UATH_N_XFERS); > + ieee80211_ifdetach(ic); > > /* free buffers */ > UATH_LOCK(sc); This sequence looks very wrong. You should not be stopping the interface or draining anything until after you have detached the interface. Otherwise you have a race condition where a user 'ifconfig up' request may be processed after usb2_transfer_unsetup(). The proper order of operations should be something like: bpfdetach() ieee80211_ifdetach() UATH_LOCK(); uath_stop(); // calls callout_stop, clears IFF_DRV_RUNNING UATH_UNLOCK(); callout_drain(); usb2_transfer_unsetup(); Note that one thing this requires you to do is explicitly check in various callbacks to see if IFF_DRV_RUNNING is not set at the start of the callback (or immediately after acquiring your driver's lock if you drop it (e.g. to call if_input()). This is not needed for callout routines if you use callout_init_mtx() as in that case the callout code can effectively do that check for you as a result of the callout_stop(). However, any other callbacks from a framework that do not use the driver's lock directly will need to explicitly check IFF_DRV_RUNNING, and it sounds like that may be the correct fix here for your USB TX callbacks (either that or you need to somehow not destroy the lock until after usb_transfer_unsetup() has returned). -- John Baldwin