Re: apparently missing locking in if_bnx.c

[Beverly Schwartz <bschwart%bbn.com@localhost>]

We were able to produce the same behavior with the intel device driver (wm).  
It appears this problem is in multiple drivers.

-Bev

On Mar 6, 2012, at 3:31 AM, Manuel Bouyer wrote:

> On Mon, Mar 05, 2012 at 03:45:10PM -0500, Greg Troxel wrote:
>> 
>> My colleague Beverly Schwartz has found a problem in if_bnx.c where
>> bnx_start takes packets off ifp->if_snd without a mutex.  bnx_start can
>> be called from multiple places (the output routine, the tx complete
>> routine, and the allocate-more-tx-buffers routine).  We have in fact
>> seen calls to bnx_start while it is running, resulting in the packet
>> being queued twice and a double free.  With pure netbsd-6, we can lock
>> up a machine (i386) with multiple bnx interfaces by running multiple
>> transfers in both directions on multiple interfaces at once.  With the
>> following patch, it is solid.  (I am pretty sure we are running with
>> only 1 core.)
>> 
>> I'm sending this as early as I can, because whatever change we converge
>> on will need to be pulled up to netbsd-6 and I'd like some more eyes on
>> it.
>> 
>> There are a splnet calls around ifq_enqueue.  So perhaps the bug is
>> instead that bnx_start is somehow called not at splnet.  But bnx_intr
>> should be running at splnet, which should serialize access.
> 
> Yes, bnx_start() should be called at splnet(), or raise to splnet() itself.
> This driver runs under the KERNEL_LOCK() anyway, so no mutex is required,
> ony splnet().
> 
> -- 
> Manuel Bouyer <bouyer%antioche.eu.org@localhost>
>     NetBSD: 26 ans d'experience feront toujours la difference
> --