* [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx
@ 2007-12-03 7:59 Wang Chen
2007-12-03 10:11 ` Herbert Xu
0 siblings, 1 reply; 7+ messages in thread
From: Wang Chen @ 2007-12-03 7:59 UTC (permalink / raw)
To: Dustin McIntire, Herbert Xu; +Cc: davem, netdev
After netif_rx(), skb will be freed. So get the
skb->len before netif_rx().
Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
smc911x.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
--- linux-2.6.24.rc3.org/drivers/net/smc911x.c 2007-11-19 12:38:05.000000000 +0800
+++ linux-2.6.24.rc3/drivers/net/smc911x.c 2007-11-30 15:00:53.000000000 +0800
@@ -1287,7 +1287,7 @@ smc911x_rx_dma_irq(int dma, void *data)
struct smc911x_local *lp = netdev_priv(dev);
struct sk_buff *skb = lp->current_rx_skb;
unsigned long flags;
- unsigned int pkts;
+ unsigned int pkts, len;
DBG(SMC_DEBUG_FUNC, "%s: --> %s\n", dev->name, __FUNCTION__);
DBG(SMC_DEBUG_RX | SMC_DEBUG_DMA, "%s: RX DMA irq handler\n", dev->name);
@@ -1299,9 +1299,10 @@ smc911x_rx_dma_irq(int dma, void *data)
PRINT_PKT(skb->data, skb->len);
dev->last_rx = jiffies;
skb->protocol = eth_type_trans(skb, dev);
+ len = skb->len;
netif_rx(skb);
dev->stats.rx_packets++;
- dev->stats.rx_bytes += skb->len;
+ dev->stats.rx_bytes += len;
spin_lock_irqsave(&lp->lock, flags);
pkts = (SMC_GET_RX_FIFO_INF() & RX_FIFO_INF_RXSUSED_) >> 16;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx
2007-12-03 7:59 [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx Wang Chen
@ 2007-12-03 10:11 ` Herbert Xu
2007-12-03 10:18 ` Wang Chen
0 siblings, 1 reply; 7+ messages in thread
From: Herbert Xu @ 2007-12-03 10:11 UTC (permalink / raw)
To: Wang Chen; +Cc: Dustin McIntire, davem, netdev, Jeff Garzik
On Mon, Dec 03, 2007 at 03:59:09PM +0800, Wang Chen wrote:
> After netif_rx(), skb will be freed. So get the
> skb->len before netif_rx().
>
> Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
Please send driver patches to Jeff Garzik <jgarzik@pobox.com>.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx
2007-12-03 10:11 ` Herbert Xu
@ 2007-12-03 10:18 ` Wang Chen
2007-12-03 13:47 ` Peter Korsgaard
0 siblings, 1 reply; 7+ messages in thread
From: Wang Chen @ 2007-12-03 10:18 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Herbert Xu, Dustin McIntire, davem, netdev
Herbert Xu said the following on 2007-12-3 18:11:
> Please send driver patches to Jeff Garzik <jgarzik@pobox.com>.
>
Sorry. Resend the patch.
After netif_rx(), skb will be freed. So get the
skb->len before netif_rx().
Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
smc911x.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
--- linux-2.6.24.rc3.org/drivers/net/smc911x.c 2007-11-19 12:38:05.000000000 +0800
+++ linux-2.6.24.rc3/drivers/net/smc911x.c 2007-11-30 15:00:53.000000000 +0800
@@ -1287,7 +1287,7 @@ smc911x_rx_dma_irq(int dma, void *data)
struct smc911x_local *lp = netdev_priv(dev);
struct sk_buff *skb = lp->current_rx_skb;
unsigned long flags;
- unsigned int pkts;
+ unsigned int pkts, len;
DBG(SMC_DEBUG_FUNC, "%s: --> %s\n", dev->name, __FUNCTION__);
DBG(SMC_DEBUG_RX | SMC_DEBUG_DMA, "%s: RX DMA irq handler\n", dev->name);
@@ -1299,9 +1299,10 @@ smc911x_rx_dma_irq(int dma, void *data)
PRINT_PKT(skb->data, skb->len);
dev->last_rx = jiffies;
skb->protocol = eth_type_trans(skb, dev);
+ len = skb->len;
netif_rx(skb);
dev->stats.rx_packets++;
- dev->stats.rx_bytes += skb->len;
+ dev->stats.rx_bytes += len;
spin_lock_irqsave(&lp->lock, flags);
pkts = (SMC_GET_RX_FIFO_INF() & RX_FIFO_INF_RXSUSED_) >> 16;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx
2007-12-03 10:18 ` Wang Chen
@ 2007-12-03 13:47 ` Peter Korsgaard
2007-12-04 2:01 ` Wang Chen
0 siblings, 1 reply; 7+ messages in thread
From: Peter Korsgaard @ 2007-12-03 13:47 UTC (permalink / raw)
To: Wang Chen; +Cc: Jeff Garzik, Herbert Xu, Dustin McIntire, davem, netdev
>>>>> "Wang" == Wang Chen <wangchen@cn.fujitsu.com> writes:
Hi,
Wang> + len = skb->len;
Wang> netif_rx(skb);
dev-> stats.rx_packets++;
Wang> - dev->stats.rx_bytes += skb->len;
Wang> + dev->stats.rx_bytes += len;
Why not simply update the stats before calling netif_rx as the return
value isn't checked anyway?
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx
2007-12-03 13:47 ` Peter Korsgaard
@ 2007-12-04 2:01 ` Wang Chen
2007-12-04 8:49 ` Peter Korsgaard
2007-12-04 19:54 ` Jeff Garzik
0 siblings, 2 replies; 7+ messages in thread
From: Wang Chen @ 2007-12-04 2:01 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: Jeff Garzik, Herbert Xu, Dustin McIntire, davem, netdev
Peter Korsgaard said the following on 2007-12-3 21:47:
>>>>>> "Wang" == Wang Chen <wangchen@cn.fujitsu.com> writes:
>
> Hi,
>
> Wang> + len = skb->len;
> Wang> netif_rx(skb);
> dev-> stats.rx_packets++;
> Wang> - dev->stats.rx_bytes += skb->len;
> Wang> + dev->stats.rx_bytes += len;
>
> Why not simply update the stats before calling netif_rx as the return
> value isn't checked anyway?
>
Even the return value of netif_rx isn't checked, dev->stats maybe
changed in netif_rx. But fortunately dev->stats isn't changed in
netif_rx.
So, I agree.
Here is the new patch.
Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
smc911x.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.24.rc3.org/drivers/net/smc911x.c 2007-11-19 12:38:05.000000000 +0800
+++ linux-2.6.24.rc3/drivers/net/smc911x.c 2007-12-04 09:59:06.000000000 +0800
@@ -1299,9 +1299,9 @@ smc911x_rx_dma_irq(int dma, void *data)
PRINT_PKT(skb->data, skb->len);
dev->last_rx = jiffies;
skb->protocol = eth_type_trans(skb, dev);
- netif_rx(skb);
dev->stats.rx_packets++;
dev->stats.rx_bytes += skb->len;
+ netif_rx(skb);
spin_lock_irqsave(&lp->lock, flags);
pkts = (SMC_GET_RX_FIFO_INF() & RX_FIFO_INF_RXSUSED_) >> 16;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx
2007-12-04 2:01 ` Wang Chen
@ 2007-12-04 8:49 ` Peter Korsgaard
2007-12-04 19:54 ` Jeff Garzik
1 sibling, 0 replies; 7+ messages in thread
From: Peter Korsgaard @ 2007-12-04 8:49 UTC (permalink / raw)
To: Wang Chen; +Cc: Jeff Garzik, Herbert Xu, Dustin McIntire, davem, netdev
>>>>> "Wang" == Wang Chen <wangchen@cn.fujitsu.com> writes:
Hi,
>> Why not simply update the stats before calling netif_rx as the return
>> value isn't checked anyway?
Wang> Even the return value of netif_rx isn't checked, dev->stats maybe
Wang> changed in netif_rx. But fortunately dev->stats isn't changed in
Wang> netif_rx.
Wang> So, I agree.
Wang> Here is the new patch.
Thanks.
Wang> Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx
2007-12-04 2:01 ` Wang Chen
2007-12-04 8:49 ` Peter Korsgaard
@ 2007-12-04 19:54 ` Jeff Garzik
1 sibling, 0 replies; 7+ messages in thread
From: Jeff Garzik @ 2007-12-04 19:54 UTC (permalink / raw)
To: Wang Chen; +Cc: Peter Korsgaard, Herbert Xu, Dustin McIntire, davem, netdev
Wang Chen wrote:
> Peter Korsgaard said the following on 2007-12-3 21:47:
>>>>>>> "Wang" == Wang Chen <wangchen@cn.fujitsu.com> writes:
>> Hi,
>>
>> Wang> + len = skb->len;
>> Wang> netif_rx(skb);
>> dev-> stats.rx_packets++;
>> Wang> - dev->stats.rx_bytes += skb->len;
>> Wang> + dev->stats.rx_bytes += len;
>>
>> Why not simply update the stats before calling netif_rx as the return
>> value isn't checked anyway?
>>
>
> Even the return value of netif_rx isn't checked, dev->stats maybe
> changed in netif_rx. But fortunately dev->stats isn't changed in
> netif_rx.
> So, I agree.
> Here is the new patch.
>
> Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
> ---
> smc911x.c | 2 +-
> 1 files changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-2.6.24.rc3.org/drivers/net/smc911x.c 2007-11-19 12:38:05.000000000 +0800
> +++ linux-2.6.24.rc3/drivers/net/smc911x.c 2007-12-04 09:59:06.000000000 +0800
> @@ -1299,9 +1299,9 @@ smc911x_rx_dma_irq(int dma, void *data)
> PRINT_PKT(skb->data, skb->len);
> dev->last_rx = jiffies;
> skb->protocol = eth_type_trans(skb, dev);
> - netif_rx(skb);
> dev->stats.rx_packets++;
> dev->stats.rx_bytes += skb->len;
> + netif_rx(skb);
>
applied #upstream-fixes
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-12-04 19:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-03 7:59 [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx Wang Chen
2007-12-03 10:11 ` Herbert Xu
2007-12-03 10:18 ` Wang Chen
2007-12-03 13:47 ` Peter Korsgaard
2007-12-04 2:01 ` Wang Chen
2007-12-04 8:49 ` Peter Korsgaard
2007-12-04 19:54 ` Jeff Garzik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).