staging: slicoss: bug fixes to stabilize driver
cover staging: slicoss: bug fixes to stabilize driver
The following patch series fixes multiple small but critical bugs in the slicoss driver that can cause kernel oopses, panics, and lots of memory leaking.
The intent of this patch series is to provide the ground work so that proper work (NAPI, locking re-design, ... see TODO) can be done.
All patches were tested on the "Mojave" (single port, PCI) card.
David Matlack (7):
staging: slicoss: fix use-after-free in slic_entry_probe
staging: slicoss: fix multiple free-after-free in slic_entry_remove
staging: slicoss: remove unused members of struct adapter
staging: slicoss: remove gratuitous debug infrastructure
staging: slicoss: fix dma memory leak
staging: slicoss: fix 64-bit isr address bug
staging: slicoss: fix use-after-free bug in slic_entry_remove
drivers/staging/slicoss/TODO | 1 - drivers/staging/slicoss/slic.h | 5 - drivers/staging/slicoss/slicoss.c | 487 ++------------------------------------ 3 files changed, 16 insertions(+), 477 deletions(-)
Signature · 1 line
1.9.2
PATCH 1/7 staging: slicoss: fix use-after-free in slic_entry_probe
This patch fixes a use-after-free bug that can cause a kernel oops. If slic_card_init fails then slic_entry_probe (the pci probe() function for this device) will return error without cleaning up memory.
--- drivers/staging/slicoss/slicoss.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
drivers/staging/slicoss/slicoss.c +5 -11
@@ -3595,7 +3595,6 @@ static int slic_entry_probe(struct pci_dev *pcidev,struct net_device *netdev;struct adapter *adapter;void __iomem *memmapped_ioaddr =NULL ; -u32 status =0 ;ulong mmio_start =0 ;ulong mmio_len =0 ;struct sliccard *card =NULL ;
@@ -3686,16 +3685,11 @@ static int slic_entry_probe(struct pci_dev *pcidev, adapter->allocated =1 ; } - status =slic_card_init (card, adapter); + err =slic_card_init (card, adapter); +if (err) + goto err_out_unmap; -if (status !=0 ) { - card->state =CARD_FAIL ; - adapter->state =ADAPT_FAIL ; - adapter->linkstate =LINK_DOWN ; -dev_err (&pcidev->dev,"FAILED status[%x]\n" , status); - }else { -slic_adapter_set_hwaddr (adapter); - } +slic_adapter_set_hwaddr (adapter); netdev->base_addr = (unsigned long )adapter->memorybase; netdev->irq = adapter->irq;
@@ -3712,7 +3706,7 @@ static int slic_entry_probe(struct pci_dev *pcidev, cards_found++; -return status; +return 0 ; err_out_unmap:iounmap (memmapped_ioaddr);
Signature · 1 line
1.9.2
PATCH 2/7 staging: slicoss: fix multiple free-after-free in slic_entry_remove
In reply to David MatlackThis patch fixes two free-after-free bugs in slic_entry_remove. Specifically, slic_unmap_mmio_space() iounmaps adapter->slic_regs, which is the same region of memory as dev->base_addr (iounmap-ed a few lines later).
Next, both release_mem_region() and pci_release_regions() are called on the same pci_dev struct.
--- drivers/staging/slicoss/slicoss.c | 8 -------- 1 file changed, 8 deletions(-)
drivers/staging/slicoss/slicoss.c +0 -8
@@ -2955,8 +2955,6 @@ static void slic_card_cleanup(struct sliccard *card)static void slic_entry_remove (struct pci_dev *pcidev) {struct net_device *dev =pci_get_drvdata (pcidev); -u32 mmio_start =0 ; -uint mmio_len =0 ;struct adapter *adapter =netdev_priv (dev);struct sliccard *card;struct mcast_address *mcaddr, *mlist;
@@ -2965,12 +2963,6 @@ static void slic_entry_remove(struct pci_dev *pcidev)slic_unmap_mmio_space (adapter);unregister_netdev (dev); - mmio_start =pci_resource_start (pcidev,0 ); - mmio_len =pci_resource_len (pcidev,0 ); - -release_mem_region (mmio_start, mmio_len); - -iounmap ((void __iomem *)dev->base_addr);/* free multicast addresses */ mlist = adapter->mcastaddrs;while (mlist) {
Signature · 1 line
1.9.2
PATCH 3/7 staging: slicoss: remove unused members of struct adapter
In reply to David MatlackThis patch removes two fields from the private "struct adapter". Specifically,
memorybase duplicate of slic_regs
memorylength written once and never read. This field is trivially
computed with pci_resource_len if it's ever needed in the future.
--- drivers/staging/slicoss/slic.h | 2 -- drivers/staging/slicoss/slicoss.c | 4 +--- 2 files changed, 1 insertion(+), 5 deletions(-)
drivers/staging/slicoss/slic.h +0 -2
@@ -420,8 +420,6 @@ struct adapter {ushort devid;ushort subsysid;u32 irq; -void __iomem *memorybase; -u32 memorylength;u32 drambase;u32 dramlength;uint queues_initialized;
drivers/staging/slicoss/slicoss.c +1 -3
@@ -3422,7 +3422,6 @@ static void slic_init_adapter(struct net_device *netdev, adapter->busnumber = pcidev->bus->number; adapter->slotnumber = ((pcidev->devfn >>3 ) &0x1F ); adapter->functionnumber = (pcidev->devfn &0x7 ); - adapter->memorylength =pci_resource_len (pcidev,0 ); adapter->slic_regs = (__iomem struct slic_regs *)memaddr; adapter->irq = pcidev->irq;/* adapter->netdev = netdev;*/
@@ -3431,7 +3430,6 @@ static void slic_init_adapter(struct net_device *netdev, adapter->chipid = chip_idx; adapter->port =0 ;/*adapter->functionnumber;*/ adapter->cardindex = adapter->port; - adapter->memorybase = memaddr;spin_lock_init (&adapter->upr_lock.lock);spin_lock_init (&adapter->bit64reglock.lock);spin_lock_init (&adapter->adapter_lock.lock);
@@ -3683,7 +3681,7 @@ static int slic_entry_probe(struct pci_dev *pcidev,slic_adapter_set_hwaddr (adapter); - netdev->base_addr = (unsigned long )adapter->memorybase; + netdev->base_addr = (unsigned long ) memmapped_ioaddr; netdev->irq = adapter->irq; netdev->netdev_ops = &slic_netdev_ops;
Signature · 1 line
1.9.2
PATCH 4/7 staging: slicoss: remove gratuitous debug infrastructure
In reply to David MatlackAs per the TODO file, this patch removes the gratuitous debug infrastructure. As an extra incentive for removing this code, the debugfs files are not cleaned up properly. For example, if register_netdev() fails in slic_entry_probe() then all debugfs files get left behind, even after the driver module is unloaded. Touching these files quickly leads to an oops.
--- drivers/staging/slicoss/TODO | 1 - drivers/staging/slicoss/slic.h | 3 - drivers/staging/slicoss/slicoss.c | 450 +------------------------------------- 3 files changed, 3 insertions(+), 451 deletions(-)
drivers/staging/slicoss/TODO +0 -1
@@ -18,7 +18,6 @@ TODO: use ethtool instead - reorder code to elminate use of forward declarations - don't keep private linked list of drivers. - - remove all the gratiutous debug infrastructure - use PCI_DEVICE() - do ethtool correctly using ethtool_ops - NAPI?
drivers/staging/slicoss/slic.h +0 -3
@@ -310,8 +310,6 @@ struct sliccard {u32 loadtimerset;uint config_set;struct slic_config config; -struct dentry *debugfs_dir; -struct dentry *debugfs_cardinfo;struct adapter *master;struct adapter *adapter[SLIC_MAX_PORTS ];struct sliccard *next;
@@ -450,7 +448,6 @@ struct adapter {u32 pingtimerset;struct timer_list loadtimer;u32 loadtimerset; -struct dentry *debugfs_entry;struct slic_spinlock upr_lock;struct slic_spinlock bit64reglock;struct slic_rspqueue rspqueue;
drivers/staging/slicoss/slicoss.c +3 -447
517 lines not on this page · raw message
Signature · 1 line
1.9.2
PATCH 5/7 staging: slicoss: fix dma memory leak
In reply to David MatlackThis patch fixes a memory leak in slic_card_init. If the driver fails to poll for an interrupt after requesting config data from the device the dma memory is never freed.
--- drivers/staging/slicoss/slicoss.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/staging/slicoss/slicoss.c +3 -0
@@ -2852,6 +2852,9 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter) &slic_regs->slic_isp,0 , &slic_regs->slic_addr_upper, 0, FLUSH); +pci_free_consistent (adapter->pcidev, +sizeof (struct slic_eeprom ), + peeprom, phys_config);return -EINVAL ; } }
Signature · 1 line
1.9.2
PATCH 6/7 staging: slicoss: fix 64-bit isr address bug
In reply to David MatlackThis patch fixes a bug that only manifests when the physical address of the interrupt status register is >4GB. Specifically, the driver was only telling the device about the lower 32 bits of the ISR. This patch adds the upper 32 bits.
--- drivers/staging/slicoss/slicoss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/slicoss/slicoss.c +2 -1
@@ -2815,7 +2815,8 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)spin_lock_irqsave (&adapter->bit64reglock.lock, adapter->bit64reglock.flags); -slic_reg32_write (&slic_regs->slic_addr_upper,0 ,DONT_FLUSH ); +slic_reg32_write (&slic_regs->slic_addr_upper, +SLIC_GET_ADDR_HIGH (&pshmem->isr),DONT_FLUSH );slic_reg32_write (&slic_regs->slic_isp,SLIC_GET_ADDR_LOW (&pshmem->isr),FLUSH ); spin_unlock_irqrestore(&adapter->bit64reglock.lock,
Signature · 1 line
1.9.2
PATCH 7/7 staging: slicoss: fix use-after-free bug in slic_entry_remove
In reply to David MatlackThis patch fixes a use-after-free bug that causes a null pointer dereference in slic_entry_halt.
Since unregister_netdev() will ultimately call slic_entry_halt (the net_device ndo_stop() virtual function for this device), we should call it before freeing the memory used by slic_entry_halt.
--- drivers/staging/slicoss/slicoss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/slicoss/slicoss.c +2 -1
@@ -2526,9 +2526,10 @@ static void slic_entry_remove(struct pci_dev *pcidev)struct sliccard *card;struct mcast_address *mcaddr, *mlist; +unregister_netdev (dev); +slic_adapter_freeresources (adapter);slic_unmap_mmio_space (adapter); -unregister_netdev (dev);/* free multicast addresses */ mlist = adapter->mcastaddrs;
Signature · 1 line
1.9.2