cvs commit: src/sys/dev/an if_an.c

[Nate Lawson]

Maksim Yevmenkin wrote:
> emax        2005-08-02 16:03:51 UTC
> 
>   FreeBSD src repository
> 
>   Modified files:
>     sys/dev/an           if_an.c 
>   Log:
>   Do not lock an to check gone flag. Only need to hold the lock to set
>   the gone flag.
>   
>   Reviewed by:    imp
>   MFC after:      1 day
>   
>   Revision  Changes    Path
>   1.69      +1 -2      src/sys/dev/an/if_an.c
> 
> 
> Index: src/sys/dev/an/if_an.c
> diff -u src/sys/dev/an/if_an.c:1.68 src/sys/dev/an/if_an.c:1.69
> --- src/sys/dev/an/if_an.c:1.68	Wed Jul 27 21:03:35 2005
> +++ src/sys/dev/an/if_an.c	Tue Aug  2 16:03:51 2005
> @@ -826,12 +826,11 @@
>  	struct an_softc		*sc = device_get_softc(dev);
>  	struct ifnet		*ifp = sc->an_ifp;
>  
> -	AN_LOCK(sc);
>  	if (sc->an_gone) {
> -		AN_UNLOCK(sc);
>  		device_printf(dev,"already unloaded\n");
>  		return(0);
>  	}
> +	AN_LOCK(sc);
>  	an_stop(sc);
>  	sc->an_gone = 1;
>  	ifmedia_removeall(&sc->an_ifmedia);

This commit is wrong.  The race is:

Process 1              Process 2
an_detach()
if (gone) ...
                        an_detach()
                        if (gone) ...
                        LOCK
                        an_stop()
                        gone = 1
                        UNLOCK
LOCK
an_stop() !!! called twice
gone = 1

You really need to hold the lock over both the check and set, otherwise 
it's not atomic.

This driver also needs some mtx_asserts in an_reset() and other places 
that must be called with the an lock held.

-- 
Nate