diff options
author | Nicolas Pitre <nico@cam.org> | 2005-06-19 23:56:21 -0400 |
---|---|---|
committer | Jeff Garzik <jgarzik@pobox.com> | 2005-06-27 00:40:33 -0400 |
commit | be83668a253149d99085ca4afe6cd8dc8a43fcd0 (patch) | |
tree | 8a70198e809a6f0abd70031d3f2d6d7460e76d4f /drivers | |
parent | ed4030d114efff53e2605ea4d07d39835b68b605 (diff) | |
download | op-kernel-dev-be83668a253149d99085ca4afe6cd8dc8a43fcd0.zip op-kernel-dev-be83668a253149d99085ca4afe6cd8dc8a43fcd0.tar.gz |
[PATCH] smc91x: plug race between TX tasklet and driver reset
The race causes a kernel oops when smc_hardware_send_pkt() tries to
dereference pending_tx_skb which would have been freed from one of the
driver reset paths just after the tx_task tasklet has been scheduled.
This race is possible on SMP but was uncovered by the kernel RT work.
Signed-off-by: Nicolas Pitre <nico@cam.org>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/net/smc91x.c | 43 |
1 files changed, 24 insertions, 19 deletions
diff --git a/drivers/net/smc91x.c b/drivers/net/smc91x.c index fd80048..cfb9d3c 100644 --- a/drivers/net/smc91x.c +++ b/drivers/net/smc91x.c @@ -315,15 +315,25 @@ static void smc_reset(struct net_device *dev) struct smc_local *lp = netdev_priv(dev); void __iomem *ioaddr = lp->base; unsigned int ctl, cfg; + struct sk_buff *pending_skb; DBG(2, "%s: %s\n", dev->name, __FUNCTION__); - /* Disable all interrupts */ + /* Disable all interrupts, block TX tasklet */ spin_lock(&lp->lock); SMC_SELECT_BANK(2); SMC_SET_INT_MASK(0); + pending_skb = lp->pending_tx_skb; + lp->pending_tx_skb = NULL; spin_unlock(&lp->lock); + /* free any pending tx skb */ + if (pending_skb) { + dev_kfree_skb(pending_skb); + lp->stats.tx_errors++; + lp->stats.tx_aborted_errors++; + } + /* * This resets the registers mostly to defaults, but doesn't * affect EEPROM. That seems unnecessary @@ -389,14 +399,6 @@ static void smc_reset(struct net_device *dev) SMC_SELECT_BANK(2); SMC_SET_MMU_CMD(MC_RESET); SMC_WAIT_MMU_BUSY(); - - /* clear anything saved */ - if (lp->pending_tx_skb != NULL) { - dev_kfree_skb (lp->pending_tx_skb); - lp->pending_tx_skb = NULL; - lp->stats.tx_errors++; - lp->stats.tx_aborted_errors++; - } } /* @@ -440,6 +442,7 @@ static void smc_shutdown(struct net_device *dev) { struct smc_local *lp = netdev_priv(dev); void __iomem *ioaddr = lp->base; + struct sk_buff *pending_skb; DBG(2, "%s: %s\n", CARDNAME, __FUNCTION__); @@ -447,7 +450,11 @@ static void smc_shutdown(struct net_device *dev) spin_lock(&lp->lock); SMC_SELECT_BANK(2); SMC_SET_INT_MASK(0); + pending_skb = lp->pending_tx_skb; + lp->pending_tx_skb = NULL; spin_unlock(&lp->lock); + if (pending_skb) + dev_kfree_skb(pending_skb); /* and tell the card to stay away from that nasty outside world */ SMC_SELECT_BANK(0); @@ -627,7 +634,12 @@ static void smc_hardware_send_pkt(unsigned long data) } skb = lp->pending_tx_skb; + if (unlikely(!skb)) { + smc_special_unlock(&lp->lock); + return; + } lp->pending_tx_skb = NULL; + packet_no = SMC_GET_AR(); if (unlikely(packet_no & AR_FAILED)) { printk("%s: Memory allocation failed.\n", dev->name); @@ -702,7 +714,6 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev) DBG(3, "%s: %s\n", dev->name, __FUNCTION__); BUG_ON(lp->pending_tx_skb != NULL); - lp->pending_tx_skb = skb; /* * The MMU wants the number of pages to be the number of 256 bytes @@ -718,7 +729,6 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev) numPages = ((skb->len & ~1) + (6 - 1)) >> 8; if (unlikely(numPages > 7)) { printk("%s: Far too big packet error.\n", dev->name); - lp->pending_tx_skb = NULL; lp->stats.tx_errors++; lp->stats.tx_dropped++; dev_kfree_skb(skb); @@ -745,6 +755,7 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev) smc_special_unlock(&lp->lock); + lp->pending_tx_skb = skb; if (!poll_count) { /* oh well, wait until the chip finds memory later */ netif_stop_queue(dev); @@ -1062,7 +1073,7 @@ static void smc_phy_powerdown(struct net_device *dev) above). linkwatch_event() also wants the netlink semaphore. */ while(lp->work_pending) - schedule(); + yield(); bmcr = smc_phy_read(dev, phy, MII_BMCR); smc_phy_write(dev, phy, MII_BMCR, bmcr | BMCR_PDOWN); @@ -1606,14 +1617,8 @@ static int smc_close(struct net_device *dev) /* clear everything */ smc_shutdown(dev); - + tasklet_kill(&lp->tx_task); smc_phy_powerdown(dev); - - if (lp->pending_tx_skb) { - dev_kfree_skb(lp->pending_tx_skb); - lp->pending_tx_skb = NULL; - } - return 0; } |