netdev - Re: [Bugme-new] [Bug 38102] New: BUG kmalloc-2048: Poison overwritten

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: <20110705180650.GF2959@hmsreliant.think-freely.org>
Date:	Tue, 5 Jul 2011 14:06:50 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Alexey Zaytsev <alexey.zaytsev@...il.com>,
	Michael Büsch <m@...s.ch>,
	Andrew Morton <akpm@...ux-foundation.org>,
	netdev@...r.kernel.org, Gary Zambrano <zambrano@...adcom.com>,
	bugme-daemon@...zilla.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Pekka Pietikainen <pp@...oulu.fi>,
	Florian Schirmer <jolt@...box.org>,
	Felix Fietkau <nbd@...nwrt.org>, Michael Buesch <mb@...sch.de>
Subject: Re: [Bugme-new] [Bug 38102] New: BUG kmalloc-2048: Poison overwritten

On Tue, Jul 05, 2011 at 06:47:21PM +0200, Eric Dumazet wrote:
> Le mardi 05 juillet 2011 à 12:42 -0400, Neil Horman a écrit :
> > On Tue, Jul 05, 2011 at 06:12:32PM +0200, Eric Dumazet wrote:
> 
> > > So all descriptors before prod are guaranteed to be ready for host
> > > consume... Fact that a dma access is running on 'next descriptor' should
> > > be irrelevant.
> > > 
> > But we handle more than one descriptor per b44_rx call - theres a while loop in
> > there where we do advance to the next descriptor.
> 
> Yes, but we advance up to 'prod', which is the very last safe
> descriptor.
> 
> If hardware advertises descriptor X being ready to be handled by host,
> while DMA on this X descriptor is not yet finished, this would be a
> really useless hardware ;)
> 
> 
> 
> --
> 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
> 

Something else just jumped out at me.  During b44_open, we call b44_init_rings.
This function allocates bp->rx_pending skb's and iteratively puts them in the rx
dma ring. bp->rx_pending is initalized to B44_DEF_RX_RING_PENDING, which is
defined as 200 (just about half of the 512 entries that the dma ring actually
supports in the hardware.  This is normally ok, as subsequent calls to
b44_alloc_rx_skb will fill in entries in the ring as those skbs are consumed.
The problem with this however is that b44_alloc_rx_skb only sets the
DESC_CTRL_EOT bit in the descriptor of the 512th entry, indicating that the
hardware should wrap around and reset the index counter.  If a large volume of
traffic is pushed through the adapter early on after initalization, or if the
cpu is busy during init, it would be possible that the ring buffer would fill up
prior to having additional entries added to the ring, the result being that the
dma engine would reach the end of the allocated descriptors, not see an EOT bit
set, and continue on using unallocated descriptors.

Just a theory, but it would be interesting to see if the problem subsided if you
ensured that you allocated  a full descriptor ring on b44_open
Neil

diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index 3d247f3..1b58a7c 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -57,7 +57,7 @@
 #define B44_MAX_MTU			1500

 #define B44_RX_RING_SIZE		512
-#define B44_DEF_RX_RING_PENDING		200
+#define B44_DEF_RX_RING_PENDING		512
 #define B44_RX_RING_BYTES	(sizeof(struct dma_desc) * \
 				 B44_RX_RING_SIZE)
 #define B44_TX_RING_SIZE		512
--
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