netdev - Gianfar: RX Recycle skb->len error

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 for Android: free password hash cracker in your pocket

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

Message-ID: <A6A1774AFD79E346AE6D49A33CB294530DC19EB5@EX-BE-017-SFO.shared.themessagecenter.com>
Date:	Sat, 20 Mar 2010 12:54:59 -0700
From:	"Ben Menchaca (ben@...footnetworks.com)" <ben@...footnetworks.com>
To:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Gianfar: RX Recycle skb->len error

We are seeing some random skb data length errors on RX after long-running, full-gigabit traffic.  First, my debugging and solution are based on the following invariant assumption:
(skb->tail - skb->data) == skb->len

If this is wrong, please educate.

After some tracing, here is where the error packets seem to originate:
1.  We are cleaning rx, in gfar_clean_rx_ring;
2.  A new RX skb is drawn from the rx_recycle queue, and obey the above invariant (so, in gfar_new_skb(), __skb_dequeue returns an skb);
3.  At this point skb_reserve is called, which moves data and tail by the same calculated alignamount;
4.  So, newskb is not NULL.  However, !(bdp->status & RXBD_LAST) || (bdp->status & RXBD_ERR)) is evaluates to true;
5.  Since newskb is not NULL, we arrive at the else if (skb), which is true;
6.  skb->data = skb->head + NET_SKB_PAD is applied, and then the skb is requeued for recycling.

At this point, skb->data != skb->tail, but skb->len == 0.  When this skb is used for the next RX, it is causing issues later when we skb_put trailers, and then trust skb->len.

I would propose something like:

--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2540,6 +2540,7 @@ 
				 * recycle list.
 				 */
 				skb->data = skb->head + NET_SKB_PAD;
+				skb_reset_tail_pointer(skb);
				__skb_queue_head(&priv->rx_recycle, skb);
			}
		} else {

Ben Menchaca
Bigfoot Networks

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