Bug 189001 - Function twl_probe() does not set error codes on some failures
Summary: Function twl_probe() does not set error codes on some failures
Status: NEW
Alias: None
Product: SCSI Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: scsi_drivers-other
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 11:16 UTC by bianpan
Modified: 2016-11-25 11:16 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description bianpan 2016-11-25 11:16:16 UTC
In function twl_probe() defined in file drivers/scsi/3w-sas.c, because variable retval is checked at line 1608, its value must be 0 when pci_iomap() is called (at line 1614). If pci_iomap() returns a NULL pointer, the control flow jumps to label "out_release_mem_region", cleans and returns the value of retval (i.e. 0). As a result, function twl_probe() returns 0 (indicates success) even if there are errors. The behavior of its caller may be misled.
There are other 2 similar bugs when the function calls fail at lines 1601 and 1624. Though these errors may occur rarely, I think it is better to set the correct error codes on failures. Codes are summarised as follows.

twl_probe @@ drivers/scsi/3w-sas.c
1564 static int twl_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
1565 {
1566     struct Scsi_Host *host = NULL;
1567     TW_Device_Extension *tw_dev;
1568     int retval = -ENODEV;
1569     int *ptr_phycount, phycount=0;
1570 
1571     retval = pci_enable_device(pdev);
1572     if (retval) {
1573         TW_PRINTK(host, TW_DRIVER, 0x17, "Failed to enable pci device");
1574         goto out_disable_device;
1575     }
         ...
1601     if (twl_initialize_device_extension(tw_dev)) {
1602         TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1a, "Failed to initialize device extension");
             // Bug (1): retval takes value 0. Insert "retval = -ENODEV;"?
1603         goto out_free_device_extension;
1604     }
1605 
1606     /* Request IO regions */
1607     retval = pci_request_regions(pdev, "3w-sas");
1608     if (retval) {
1609         TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1b, "Failed to get mem region");
1610         goto out_free_device_extension;
1611     }
1612 
1613     /* Save base address, use region 1 */
1614     tw_dev->base_addr = pci_iomap(pdev, 1, 0);
1615     if (!tw_dev->base_addr) {
1616         TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1c, "Failed to ioremap");
             // Bug (2): retval takes value 0. Insert "retval = -ENOMEM;"?
1617         goto out_release_mem_region;
1618     }
1619 
1620     /* Disable interrupts on the card */
1621     TWL_MASK_INTERRUPTS(tw_dev);
1622 
1623     /* Initialize the card */
1624     if (twl_reset_sequence(tw_dev, 0)) {
1625         TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1d, "Controller reset failed during probe");
             // Bug (3): retval takes value 0. Insert "retval = -ENODEV;"?
1626         goto out_iounmap;
1627     }

Thanks very much!

Note You need to log in before you can comment on or make changes to this bug.