My bad (was: More Boomerang Fixes)
Rob Riggs
rob@devilsthumb.com
Tue Jul 7 11:29:53 1998
This is a multi-part message in MIME format.
--------------A1C258ED4F851B861B6B696B
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
OK... Attempt #3 (three's a charm, right?)
It just goes to show, that you read the code over and over
and you still miss the important bits... until just after
you hit "send".
rx_work_limit is required, however, we put the test
*before* any work is done, rather than after, so cases
where rx_work_limit == 0 are caught before we try to
write to NULL pointers. (It is required because we
use parts of the ring buffers when small frames come
in, but leave vp->rx_skbuff[entry] alone. It still
points to a valid skb, but we can end up putting data
in a used vp->rx_ring[entry].)
Also vp->cur_rx and vp->dirty_rx are "unsigned ints"
(what was I thinking?!?) but still prone to the problem
of overflowing (or wrapping). Fixed that as well.
-Rob
--------------A1C258ED4F851B861B6B696B
Content-Type: text/plain; charset=us-ascii; name="3c59x.patch3"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="3c59x.patch3"
--- 3c59x.c.orig Mon Jul 6 12:16:25 1998
+++ 3c59x.c Tue Jul 7 13:10:08 1998
@@ -1777,6 +1777,9 @@
int pkt_len = rx_status & 0x1fff;
struct sk_buff *skb;
+ if (--rx_work_limit < 0)
+ break;
+
if (vortex_debug > 4)
printk(KERN_DEBUG "Receiving packet size %d status %4.4x.\n",
pkt_len, rx_status);
@@ -1801,6 +1804,10 @@
void *temp;
/* Pass up the skbuff already on the Rx ring. */
skb = vp->rx_skbuff[entry];
+ if (skb == NULL) {
+ printk(KERN_WARNING "boomerang: attempt to write to NULL skb caught.\n");
+ break;
+ }
vp->rx_skbuff[entry] = NULL;
#if LINUX_VERSION_CODE >= 0x10300
temp = skb_put(skb, pkt_len);
@@ -1833,8 +1840,11 @@
vp->stats.rx_packets++;
}
entry = (++vp->cur_rx) % RX_RING_SIZE;
- if (--rx_work_limit < 0)
- break;
+ /* Wrap our counters -- otherwise the next for() loop fails */
+ if (vp->cur_rx == (INT_MAX + RX_RING_SIZE + 1)) {
+ vp->cur_rx -= (INT_MAX + 1);
+ vp->dirty_rx -= (INT_MAX + 1);
+ }
}
/* Refill the Rx ring buffers. */
for (; vp->dirty_rx < vp->cur_rx; vp->dirty_rx++) {
@@ -1842,8 +1852,10 @@
entry = vp->dirty_rx % RX_RING_SIZE;
if (vp->rx_skbuff[entry] == NULL) {
skb = DEV_ALLOC_SKB(PKT_BUF_SZ);
- if (skb == NULL)
+ if (skb == NULL) {
+ printk(KERN_DEBUG "boomerang: Could not allocate skb on ring buffer.\n");
break; /* Bad news! */
+ }
skb->dev = dev; /* Mark as being used by this device. */
#if LINUX_VERSION_CODE > 0x10300
skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
--------------A1C258ED4F851B861B6B696B--