svn commit: r192419 - head/sys/dev/usb/wlan
John Baldwin
jhb at freebsd.org
Wed May 20 14:02:22 UTC 2009
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 <lwindschuh at googlemail.com>
>
> 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
More information about the svn-src-all
mailing list