RELENG_7 crash

Robert Watson rwatson at FreeBSD.org
Tue Apr 21 22:25:41 UTC 2009


On Wed, 22 Apr 2009, Mikolaj Golub wrote:

> RW> There are several bugs here, one difficult to fix (lack of
> RW> refcounting), but also stuff like ifp being derived from an interface
> RW> number twice, but checked against NULL only the first time (line 85
> RW> checked for NULL, re-queried but no check line 88).  Fixing the top
> RW> bit of the function to only query the ifp once and check it for NULL
> RW> then would be a good idea.  More fundamentally, we do need to refcount
> RW> ifnets when used from the management path, which is not all that hard
> RW> a change, but preferably to try the easy way first given where we are
> RW> in the release cycle.
>
> I was thinking a bit on this problem (the same issue was reported in 
> kern/132734) and eliminating double call of ifnet_byindex() was the first 
> thing I did. But I decided then that the proper fix would be to wrap all 
> critical code in sysctl_ifdata in IFNET_RLOCK/IFNET_RUNLOCK (the patch is 
> attached). It looks like I am wrong and my idea about how the kernel works 
> is oversimplified? :-) Unfortunately, I didn't manage to reproduce the panic 
> in my environment so I was not able to do some experiments and tests.

The problem is that you can't hold IFNET_RLOCK() over the sysctl copyouts. 
I'm preparing a patch for 8-CURRENT that will add a refcount to struct ifnet 
to handle this case, but that isn't an MFC candidate for 7.2.  If fixing the 
return value checks for ifnet_byindex() avoids the insta-panic Mike's running 
into, it's a better 7.2 change at this point.  We can target the ifnet 
refcount changes for 7.3.

Robert N M Watson
Computer Laboratory
University of Cambridge


More information about the freebsd-stable mailing list