Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 20 Sep 2005 15:51:09 -0400
From:      John Baldwin <jhb@FreeBSD.org>
To:        "M. Warner Losh" <imp@bsdimp.com>
Cc:        cvs-src@FreeBSD.org, src-committers@FreeBSD.org, ru@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/dev/an if_an.c src/sys/dev/arl if_arl_isa.c src/sys/dev/awi if_awi_pccard.c src/sys/dev/cm if_cm_isa.c src/sys/dev/cnw if_cnw.c src/sys/dev/cp if_cp.c src/sys/dev/cs if_cs.c src/sys/dev/ed if_ed.c src/sys/dev/em if_em.c ...
Message-ID:  <200509201551.11396.jhb@FreeBSD.org>
In-Reply-To: <20050919.083146.105425670.imp@bsdimp.com>
References:  <200509190310.j8J3ALgt095979@repoman.freebsd.org> <20050919055028.GC65954@ip.net.ua> <20050919.083146.105425670.imp@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 19 September 2005 10:31 am, M. Warner Losh wrote:
> In message: <20050919055028.GC65954@ip.net.ua>
>
>             Ruslan Ermilov <ru@freebsd.org> writes:
> : Hi,
> :
> : On Mon, Sep 19, 2005 at 03:10:21AM +0000, Warner Losh wrote:
> : > imp         2005-09-19 03:10:21 UTC
> : >
> : >   FreeBSD src repository
> : >
> : >   Modified files:
> : >     sys/dev/an           if_an.c
> :
> : [...]
> :
> : >     sys/dev/xe           if_xe_pccard.c
> : >   Log:
> : >   Make sure that we call if_free(ifp) after bus_teardown_intr.  Since
> : > we could get an interrupt after we free the ifp, and the interrupt
> : > handler depended on the ifp being still alive, this could, in theory,
> : > cause a crash.  Eliminate this possibility by moving the if_free to
> : > after the bus_teardown_intr() call.
> :
> : I'm going into more details in my other reply to your ed(4) commit,
> : but I'm pretty sure you're attacking a wrong problem here.  Most
> : drivers don' assume and will behave badly (read: panic) if foo_intr()
> : is called after foo_stop() (foo_stop() disables interrupts and
> : usually frees some resources needed by code called from foo_intr()).
> : A better fix that I had in mind (and that I think jhb@ has suggested)
> : would be to return from foo_intr() quickly if IFF_DRV_RUNNING is not
> : set.  Can you reproduce the problem that you mention on real hardware?
>
> You can't test IFF_DRV_RUNNING in memory that's already been freed.
>
> Warner

Then do this:

	foo_stop();
	callout_drain();
	ether_ifdetach();
	bus_teardowin_intr();
	if_free();

Regarding other comments I saw today on some e-mail or another, I do think 
that to make the locking sane, we might should push the checks for 
IFF_DRV_RUNNING down into the foo_start() routines rather than doing it in 
the network layer where the driver lock isn't held.

-- 
John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200509201551.11396.jhb>