Commit be83668a authored by Nicolas Pitre's avatar Nicolas Pitre Committed by Jeff Garzik

[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: default avatarNicolas Pitre <nico@cam.org>
parent ed4030d1
...@@ -315,15 +315,25 @@ static void smc_reset(struct net_device *dev) ...@@ -315,15 +315,25 @@ static void smc_reset(struct net_device *dev)
struct smc_local *lp = netdev_priv(dev); struct smc_local *lp = netdev_priv(dev);
void __iomem *ioaddr = lp->base; void __iomem *ioaddr = lp->base;
unsigned int ctl, cfg; unsigned int ctl, cfg;
struct sk_buff *pending_skb;
DBG(2, "%s: %s\n", dev->name, __FUNCTION__); DBG(2, "%s: %s\n", dev->name, __FUNCTION__);
/* Disable all interrupts */ /* Disable all interrupts, block TX tasklet */
spin_lock(&lp->lock); spin_lock(&lp->lock);
SMC_SELECT_BANK(2); SMC_SELECT_BANK(2);
SMC_SET_INT_MASK(0); SMC_SET_INT_MASK(0);
pending_skb = lp->pending_tx_skb;
lp->pending_tx_skb = NULL;
spin_unlock(&lp->lock); 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 * This resets the registers mostly to defaults, but doesn't
* affect EEPROM. That seems unnecessary * affect EEPROM. That seems unnecessary
...@@ -389,14 +399,6 @@ static void smc_reset(struct net_device *dev) ...@@ -389,14 +399,6 @@ static void smc_reset(struct net_device *dev)
SMC_SELECT_BANK(2); SMC_SELECT_BANK(2);
SMC_SET_MMU_CMD(MC_RESET); SMC_SET_MMU_CMD(MC_RESET);
SMC_WAIT_MMU_BUSY(); 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) ...@@ -440,6 +442,7 @@ static void smc_shutdown(struct net_device *dev)
{ {
struct smc_local *lp = netdev_priv(dev); struct smc_local *lp = netdev_priv(dev);
void __iomem *ioaddr = lp->base; void __iomem *ioaddr = lp->base;
struct sk_buff *pending_skb;
DBG(2, "%s: %s\n", CARDNAME, __FUNCTION__); DBG(2, "%s: %s\n", CARDNAME, __FUNCTION__);
...@@ -447,7 +450,11 @@ static void smc_shutdown(struct net_device *dev) ...@@ -447,7 +450,11 @@ static void smc_shutdown(struct net_device *dev)
spin_lock(&lp->lock); spin_lock(&lp->lock);
SMC_SELECT_BANK(2); SMC_SELECT_BANK(2);
SMC_SET_INT_MASK(0); SMC_SET_INT_MASK(0);
pending_skb = lp->pending_tx_skb;
lp->pending_tx_skb = NULL;
spin_unlock(&lp->lock); spin_unlock(&lp->lock);
if (pending_skb)
dev_kfree_skb(pending_skb);
/* and tell the card to stay away from that nasty outside world */ /* and tell the card to stay away from that nasty outside world */
SMC_SELECT_BANK(0); SMC_SELECT_BANK(0);
...@@ -627,7 +634,12 @@ static void smc_hardware_send_pkt(unsigned long data) ...@@ -627,7 +634,12 @@ static void smc_hardware_send_pkt(unsigned long data)
} }
skb = lp->pending_tx_skb; skb = lp->pending_tx_skb;
if (unlikely(!skb)) {
smc_special_unlock(&lp->lock);
return;
}
lp->pending_tx_skb = NULL; lp->pending_tx_skb = NULL;
packet_no = SMC_GET_AR(); packet_no = SMC_GET_AR();
if (unlikely(packet_no & AR_FAILED)) { if (unlikely(packet_no & AR_FAILED)) {
printk("%s: Memory allocation failed.\n", dev->name); 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) ...@@ -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__); DBG(3, "%s: %s\n", dev->name, __FUNCTION__);
BUG_ON(lp->pending_tx_skb != NULL); 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 * 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) ...@@ -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; numPages = ((skb->len & ~1) + (6 - 1)) >> 8;
if (unlikely(numPages > 7)) { if (unlikely(numPages > 7)) {
printk("%s: Far too big packet error.\n", dev->name); printk("%s: Far too big packet error.\n", dev->name);
lp->pending_tx_skb = NULL;
lp->stats.tx_errors++; lp->stats.tx_errors++;
lp->stats.tx_dropped++; lp->stats.tx_dropped++;
dev_kfree_skb(skb); dev_kfree_skb(skb);
...@@ -745,6 +755,7 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev) ...@@ -745,6 +755,7 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
smc_special_unlock(&lp->lock); smc_special_unlock(&lp->lock);
lp->pending_tx_skb = skb;
if (!poll_count) { if (!poll_count) {
/* oh well, wait until the chip finds memory later */ /* oh well, wait until the chip finds memory later */
netif_stop_queue(dev); netif_stop_queue(dev);
...@@ -1062,7 +1073,7 @@ static void smc_phy_powerdown(struct net_device *dev) ...@@ -1062,7 +1073,7 @@ static void smc_phy_powerdown(struct net_device *dev)
above). linkwatch_event() also wants the netlink semaphore. above). linkwatch_event() also wants the netlink semaphore.
*/ */
while(lp->work_pending) while(lp->work_pending)
schedule(); yield();
bmcr = smc_phy_read(dev, phy, MII_BMCR); bmcr = smc_phy_read(dev, phy, MII_BMCR);
smc_phy_write(dev, phy, MII_BMCR, bmcr | BMCR_PDOWN); smc_phy_write(dev, phy, MII_BMCR, bmcr | BMCR_PDOWN);
...@@ -1606,14 +1617,8 @@ static int smc_close(struct net_device *dev) ...@@ -1606,14 +1617,8 @@ static int smc_close(struct net_device *dev)
/* clear everything */ /* clear everything */
smc_shutdown(dev); smc_shutdown(dev);
tasklet_kill(&lp->tx_task);
smc_phy_powerdown(dev); smc_phy_powerdown(dev);
if (lp->pending_tx_skb) {
dev_kfree_skb(lp->pending_tx_skb);
lp->pending_tx_skb = NULL;
}
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment