netdev - Re: bnx2_poll panicking kernel

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: <1213659908.18055.26.camel@dell>
Date:	Mon, 16 Jun 2008 16:45:08 -0700
From:	"Michael Chan" <mchan@...adcom.com>
To:	"Josip Rodin" <joy@...uzijast.net>
cc:	"Ben Hutchings" <bhutchings@...arflare.com>,
	netdev <netdev@...r.kernel.org>, mirrors@...ian.org
Subject: Re: bnx2_poll panicking kernel

On Mon, 2008-06-16 at 23:48 +0200, Josip Rodin wrote:
> > Crap, it just crashed again, with 2.6.25.6.

Looking at the panic dmesg, I believe it was crashing because skb was
NULL in bnx2_tx_int().

> 
> I should mention that I might have a clue - around the time the crashes
> started, I started using HTB with the following configuration:

This rings a bell.  A similar crash was reported by a user using HTB on
a tg3 device.  After some analysis, my suspicion was that the shinfo
(skb)->nr_frags on the TX packet was corrupted before the SKB was freed
by the driver.  In both the tg3 and bnx2 drivers, we rely on the
nr_frags to locate the TX packet boundaries on the ring to free the
packets.

Please try this debug patch below.  If the theory is correct, the patch
should avoid the crash and will print something to the dmesg log every
time a crash is avoided.  Please run it again with the HTB rules and
send me the dmesg log containing:

bnx2: skb->nr_frags ...

Here's the debug patch below.  It saves the nr_frags from the SKB and
uses it to locate the packet boundaries instead.  It will also compare
the saved value with the one in SKB and print a warning when they don't
match.  Please apply to 2.6.25.6.

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 4b46e68..f7ecd07 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -2495,14 +2495,20 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
 		tx_buf = &bp->tx_buf_ring[sw_ring_cons];
 		skb = tx_buf->skb;
 
+		if (tx_buf->nr_frags != skb_shinfo(skb)->nr_frags) {
+			printk(KERN_ALERT "bnx2: skb->nr_frags=%d is corrupted,"
+					  " should be %d\n",
+					  skb_shinfo(skb)->nr_frags,
+					  tx_buf->nr_frags);
+		}
 		/* partial BD completions possible with TSO packets */
 		if (skb_is_gso(skb)) {
 			u16 last_idx, last_ring_idx;
 
 			last_idx = sw_cons +
-				skb_shinfo(skb)->nr_frags + 1;
+				tx_buf->nr_frags + 1;
 			last_ring_idx = sw_ring_cons +
-				skb_shinfo(skb)->nr_frags + 1;
+				tx_buf->nr_frags + 1;
 			if (unlikely(last_ring_idx >= MAX_TX_DESC_CNT)) {
 				last_idx++;
 			}
@@ -2515,7 +2521,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
 			skb_headlen(skb), PCI_DMA_TODEVICE);
 
 		tx_buf->skb = NULL;
-		last = skb_shinfo(skb)->nr_frags;
+		last = tx_buf->nr_frags;
 
 		for (i = 0; i < last; i++) {
 			sw_cons = NEXT_TX_BD(sw_cons);
@@ -4806,7 +4812,7 @@ bnx2_free_tx_skbs(struct bnx2 *bp)
 
 		tx_buf->skb = NULL;
 
-		last = skb_shinfo(skb)->nr_frags;
+		last = tx_buf->nr_frags;
 		for (j = 0; j < last; j++) {
 			tx_buf = &bp->tx_buf_ring[i + j + 1];
 			pci_unmap_page(bp->pdev,
@@ -5859,6 +5865,8 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	last_frag = skb_shinfo(skb)->nr_frags;
 
+	tx_buf->nr_frags = last_frag;
+
 	for (i = 0; i < last_frag; i++) {
 		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index 1eaf5bb..aa9fa6f 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6485,6 +6485,7 @@ struct l2_fhdr {
 struct sw_bd {
 	struct sk_buff		*skb;
 	DECLARE_PCI_UNMAP_ADDR(mapping)
+	unsigned short		nr_frags;
 };
 
 struct sw_pg {


--
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