[lkml]   [2001]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectRe: [PATCH] 8139too.c
On Nov 30, 2001  20:23 +0900, wrote:
> In 8139too.c, skb is used after it is freed.
> Look at the argument of RTL_W32_F() in the following patch.
> 8 % diff -u -10 8139too.c /tmp/8139too.c
> --- 8139too.c Sun Nov 25 10:46:36 2001
> +++ /tmp/8139too.c Fri Nov 30 19:50:48 2001
Please try to keep patches relative to the linux/ directory, so they can
be applied via "patch -p1" as most other kernel patches are. Also note
that you should CC the driver maintainer if you want to get the patch
into the kernel (in this case Jeff Garzik <>).

> + dev_kfree_skb(skb);
> DPRINTK ("%s: Queued Tx packet at %p size %u to slot %d.\n",
> dev->name, skb->data, skb->len, entry);
Your patch still does not fix the problems. Since we only use "skb->len"
(except for the debug case, where we also use the skb->data pointer),
rather just copy skb->len into a local variable and free skb early, so
it can be used again, rather than holding it over a spin lock.

I don't know if anyone really cares about the skb->data pointer in the
DPRINTK. If so, we could move it up before we free the skb, but I
thought it incorrect to do so, because it is not queued at that time,
and I didn't want to add a temp variable that only appears in a debug.

Cheers, Andreas
--- linux/drivers/net/8139too.c.orig Sun Nov 25 10:46:36 2001
+++ linux/drivers/net/8139too.c Fri Nov 30 19:50:48 2001
@@ -1635,35 +1635,36 @@
struct rtl8139_private *tp = dev->priv;
void *ioaddr = tp->mmio_addr;
unsigned int entry;
+ unsigned int len = skb->len;

/* Calculate the next Tx descriptor entry. */
entry = tp->cur_tx % NUM_TX_DESC;

- if (likely(skb->len < TX_BUF_SIZE)) {
+ if (likely(len < TX_BUF_SIZE)) {
skb_copy_and_csum_dev(skb, tp->tx_buf[entry]);
} else {
return 0;
- }
+ }

/* Note: the chip doesn't have auto-pad! */
RTL_W32_F (TxStatus0 + (entry * sizeof (u32)),
- tp->tx_flag | (skb->len >= ETH_ZLEN ? skb->len : ETH_ZLEN));
+ tp->tx_flag | max(len, (unsigned int)ETH_ZLEN));

dev->trans_start = jiffies;


if ((tp->cur_tx - NUM_TX_DESC) == tp->dirty_tx)
netif_stop_queue (dev);

- DPRINTK ("%s: Queued Tx packet at %p size %u to slot %d.\n",
- dev->name, skb->data, skb->len, entry);
+ DPRINTK ("%s: Queued Tx packet size %u to slot %d.\n",
+ dev->name, len, entry);

return 0;

Andreas Dilger

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2005-03-22 13:13    [W:0.461 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site