netdev - Re: [PATCH] sis900: Allocate rx replacement buffer before rx operation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070426233014.GB3600@localhost.localdomain>
Date:	Thu, 26 Apr 2007 19:30:14 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	netdev@...r.kernel.org, davem@...emloft.net, venza@...wnhat.org
Subject: Re: [PATCH] sis900: Allocate rx replacement buffer before rx operation

On Thu, Apr 26, 2007 at 01:47:36PM -0400, Neil Horman wrote:
> On Tue, Apr 24, 2007 at 12:43:20PM -0400, Jeff Garzik wrote:
> > Neil Horman wrote:
> > >Hey there-
> > >	The sis900 driver appears to have a bug in which the receive routine
> > >passes the skbuff holding the received frame to the network stack before
> > >refilling the buffer in the rx ring.  If a new skbuff cannot be allocated, 
> > >the
> > >driver simply leaves a hole in the rx ring, which causes the driver to stop
> > >receiving frames and become non-recoverable without an rmmod/insmod 
> > >according to
> > >reporters.  This patch reverses that order, attempting to allocate a 
> > >replacement
> > >buffer first, and receiving the new frame only if one can be allocated.  
> > >If no
> > >skbuff can be allocated, the current skbuf in the rx ring is recycled, 
> > >dropping
> > >the current frame, but keeping the NIC operational.
> > >
> > >Thanks & Regards
> > >Neil
> > 
> 
> 
> 
> Just found a hole in my last patch.  It was reported to me that shortly after we
> integrated this patch.  The report was of an oops that took place inside of
> netif_rx when using the sis900 driver.  Looking at my origional patch I noted
> that there was a spot between the new skb_alloc and the refill_rx_ring label
> where skb got reassigned to the pointer currently held in the rx_ring for the
> purposes of receiveing the frame.  The result of this is however that the buffer
> that gets passed to netif_rx (if it is called), then gets placed right back into
> the rx_ring.  So if you receive frames fast enough the skb being processed by
> the network stack can get corrupted.  The reporter is testing out the fix I've
> written for this below (I'm not near my hardware at the moment to test myself),
> but I wanted to post it for review ASAP.  I'll post test results when I hear
> them, but I think this is a pretty straightforward fix.  It just uses a separate
> pointer to do the rx operation, so that we don't improperly reassign the pointer
> that we use to refill the rx ring.
> 
> Thanks & Regards
> Neil
> 
Test results are back and appear successful.
Neil

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html