Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx

2007-12-04 Thread Peter Korsgaard 
 Wang == Wang Chen [EMAIL PROTECTED] 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 [EMAIL PROTECTED]

Acked-by: Peter Korsgaard [EMAIL PROTECTED]

-- 
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx

2007-12-04 Thread Jeff Garzik 

Wang Chen wrote:

Peter Korsgaard said the following on 2007-12-3 21:47:

Wang == Wang Chen [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
---
 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.0 
+0800
+++ linux-2.6.24.rc3/drivers/net/smc911x.c  2007-12-04 09:59:06.0 
+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


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] SMC911X: Fix using of dereferenced skb after netif_rx

2007-12-03 Thread Wang Chen 
After netif_rx(), skb will be freed. So get the
skb-len before netif_rx().

Signed-off-by: Wang Chen [EMAIL PROTECTED]
---
 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.0 
+0800
+++ linux-2.6.24.rc3/drivers/net/smc911x.c  2007-11-30 15:00:53.0 
+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;

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx

2007-12-03 Thread Wang Chen 
Herbert Xu said the following on 2007-12-3 18:11:
 Please send driver patches to Jeff Garzik [EMAIL PROTECTED].
 

Sorry. Resend the patch.

After netif_rx(), skb will be freed. So get the
skb-len before netif_rx().

Signed-off-by: Wang Chen [EMAIL PROTECTED]
---
 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.0 
+0800
+++ linux-2.6.24.rc3/drivers/net/smc911x.c  2007-11-30 15:00:53.0 
+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;

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx

2007-12-03 Thread Herbert Xu 
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 [EMAIL PROTECTED]

Please send driver patches to Jeff Garzik [EMAIL PROTECTED].

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx

2007-12-03 Thread Peter Korsgaard 
 Wang == Wang Chen [EMAIL PROTECTED] 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
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SMC911X: Fix using of dereferenced skb after netif_rx

2007-12-03 Thread Wang Chen 
Peter Korsgaard said the following on 2007-12-3 21:47:
 Wang == Wang Chen [EMAIL PROTECTED] writes:
 
 Hi,
 
  Wang +  len = skb-len;
  Wangnetif_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 [EMAIL PROTECTED]
---
 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.0 
+0800
+++ linux-2.6.24.rc3/drivers/net/smc911x.c  2007-12-04 09:59:06.0 
+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;

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html