cvs commit: src/sys/dev/re if_re.c

[Ruslan Ermilov]

On Thu, Sep 15, 2005 at 03:21:12PM -0400, John Baldwin wrote:
> On Thursday 15 September 2005 02:59 pm, Ruslan Ermilov wrote:
> > ru          2005-09-15 18:59:34 UTC
> >
> >   FreeBSD src repository
> >
> >   Modified files:
> >     sys/dev/re           if_re.c
> >   Log:
> >   re_detach() fixes:
> >
> >   - Fixed if_free() logic screw-up that can either result
> >     in freeing a NULL pointer or leaking "struct ifnet".
> >   - Move if_free() after re_stop(); the latter accesses
> >     "struct ifnet".  This bug was masked by a previous bug.
> >   - Restore the fix for a panic on detach caused by racing
> >     with BPF detach code by Bill by moving ether_ifdetach()
> >     after re_stop() and resetting IFF_UP; this got screwed
> >     up in revs. 1.30 and 1.36.
> 
> Device drivers should not modify IFF_UP.  Instead, the interrupt handler 
> should be checking IFF_DRV_RUNNING rather than IFF_UP (foo_stop() clears 
> IFF_DRV_RUNNING).
> 
Ideally they shoudn't, yes.  But there are at least two scenarios
where resetting IFF_UP is currently used to attack bugs.

The first is the BPF detach bad interaction with foo_detach(),
as described in re_detach().  This panic is real with (I think)
all drivers.  And testing IFF_DRV_RUNNING here doesn't seem to
be able to prevent the panic.  Perhaps the fix would be to
move ether_ifdetach() before foo_stop() in foo_detach(), I'm
not yet sure.

Another panic sometimes seen on SMP machines on shutdown is when
foo_intr() is called after foo_shutdown() has already been called.
Some drivers (if_re, if_vr, if_ath) attack this problem by clearing
IFF_UP so that foo_intr() bails out quickly.  I don't know if
anybody diagnosed the roots of this problem, but I have the
following idea: foo_shutdown() calls foo_stop() which disables
device interrupts.  After foo_stop() has freed resources but
before interrupt has been teared down, another device that
shares the same IRQ generated an interrupt, and foo_intr() is
called again.  It checks ISR register, and it has some interrupt
active, and the code calls some function that accesses already
freed memory.  I don't have any real SMP hardware to play with,
so the above is pure theoretical.  Do you think this is possible?
If so, checking IFF_DRV_RUNNING in foo_intr() should fix this.

If we can also fix the "BPF detach MII tick reactivation" panic
without involving resetting IFF_UP, all drivers can be cleaned
up to not much with IFF_UP.


Cheers,
-- 
Ruslan Ermilov
ru at FreeBSD.org
FreeBSD committer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-all/attachments/20050915/41abb4c1/attachment.bin