From: Jesper Juhl In a low memory situation, if you are very unlucky, the speedo_init_rx_ring() function may cause a NULL pointer deref. The problem is in the case where we can't allocate even a single skb for the RX ring. In this case 'last_rxf' will be NULL when we break out of the loop and the line last_rxf->status = cpu_to_le32(0xC0000002); /* '2' is flag value only. */ will cause a NULL pointer dereference. To fix this properly we need to be return an error from speedo_init_rx_ring() and have the caller (speedo_open()) catch and propagate the error, as well as undo anything done to setup the device so far. This patch adds a check to catch the unlucky case of not even a single skb being available and adds code in the caller to catch the error and release the device properly. For a user who hits this problem, this makes the difference between her device not being opened and a kernel crash. Clearly a non functional NIC if preferable to a kernel crash - especially since setting up the device can easily be retried later after freeing up some memory; a kernel crash is not as easy to recover from. The problem was initially spotted by the Coverity checker. Signed-off-by: Jesper Juhl Signed-off-by: Andrew Morton --- drivers/net/eepro100.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff -puN drivers/net/eepro100.c~eepro100-avoid-potential-null-pointer-deref-in-speedo_init_rx_ring drivers/net/eepro100.c --- a/drivers/net/eepro100.c~eepro100-avoid-potential-null-pointer-deref-in-speedo_init_rx_ring +++ a/drivers/net/eepro100.c @@ -482,7 +482,7 @@ static void mdio_write(struct net_device static int speedo_open(struct net_device *dev); static void speedo_resume(struct net_device *dev); static void speedo_timer(unsigned long data); -static void speedo_init_rx_ring(struct net_device *dev); +static int speedo_init_rx_ring(struct net_device *dev); static void speedo_tx_timeout(struct net_device *dev); static int speedo_start_xmit(struct sk_buff *skb, struct net_device *dev); static void speedo_refill_rx_buffers(struct net_device *dev, int force); @@ -977,9 +977,8 @@ speedo_open(struct net_device *dev) /* .. we can safely take handler calls during init. */ retval = request_irq(dev->irq, &speedo_interrupt, IRQF_SHARED, dev->name, dev); - if (retval) { - return retval; - } + if (retval) + goto out; dev->if_port = sp->default_port; @@ -1001,7 +1000,9 @@ speedo_open(struct net_device *dev) } #endif - speedo_init_rx_ring(dev); + retval = speedo_init_rx_ring(dev); + if (retval) + goto out_rx_init_err; /* Fire up the hardware. */ iowrite16(SCBMaskAll, ioaddr + SCBCmd); @@ -1040,7 +1041,12 @@ speedo_open(struct net_device *dev) if ((sp->phy[0] & 0x8000) == 0) mdio_read(dev, sp->phy[0] & 0x1f, MII_BMCR); - return 0; + out: + return retval; + out_rx_init_err: + free_irq(dev->irq, dev); + pci_set_power_state(sp->pdev, PCI_D2); + goto out; } /* Start the chip hardware after a full reset. */ @@ -1243,7 +1249,7 @@ static void speedo_show_state(struct net } /* Initialize the Rx and Tx rings, along with various 'dev' bits. */ -static void +static int speedo_init_rx_ring(struct net_device *dev) { struct speedo_private *sp = netdev_priv(dev); @@ -1283,6 +1289,10 @@ speedo_init_rx_ring(struct net_device *d pci_dma_sync_single_for_device(sp->pdev, sp->rx_ring_dma[i], sizeof(struct RxFD), PCI_DMA_TODEVICE); } + /* If we failed to allocate even a single skb we need to bail out. */ + if (!last_rxf) + return -ENOMEM; + sp->dirty_rx = (unsigned int)(i - RX_RING_SIZE); /* Mark the last entry as end-of-list. */ last_rxf->status = cpu_to_le32(0xC0000002); /* '2' is flag value only. */ @@ -1290,6 +1300,8 @@ speedo_init_rx_ring(struct net_device *d sizeof(struct RxFD), PCI_DMA_TODEVICE); sp->last_rxf = last_rxf; sp->last_rxf_dma = last_rxf_dma; + + return 0; } static void speedo_purge_tx(struct net_device *dev) _