Bug 189051 - Function fnic_probe() does not set set code when the call to mempool_create_slab_pool() fails
Summary: Function fnic_probe() does not set set code when the call to mempool_create_s...
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:20 UTC by bianpan
Modified: 2016-11-25 11:20 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:20:03 UTC
In the function fnic_probe() defined in file drivers/scsi/fnic/fnic_main.c, when the call to mempool_create_slab_pool() (at line 738) returns a NULL pointer, the control flow jumps to label "err_out_free_resources", and returns variable err. Because variable err is checked at line 714, the value of err must be 0 here. As a result, function fnic_probe() returns 0 (indicates success) even if the call to mempool_create_slab_pool() fails.
There are other 2 similar bugs when the call to mempool_create_slab_pool() fail at lines 742 and 747. Though these errors may occur rarely, I think it may be better to set correct error codes (e.g. -ENOMEM) on failures. Codes related to these bugs are summarised as follows.

fnic_probe @@ drivers/scsi/fnic/fnic_main.c
 541 static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 542 {
         ...
 547     int err;
         ...
 713     err = fnic_alloc_vnic_resources(fnic);
 714     if (err) {
 715         shost_printk(KERN_ERR, fnic->lport->host,
 716                  "Failed to alloc vNIC resources, "
 717                  "aborting.\n");
 718         goto err_out_clear_intr;
 719     }
         ...
         // Insert "err = -ENOMEM;" ?
 738     fnic->io_req_pool = mempool_create_slab_pool(2, fnic_io_req_cache);
 739     if (!fnic->io_req_pool)
             // Bug (1): the value of err is 0
 740         goto err_out_free_resources;
 741 
 742     pool = mempool_create_slab_pool(2, fnic_sgl_cache[FNIC_SGL_CACHE_DFLT]);
 743     if (!pool)
             // Bug (2): the value of err is 0
 744         goto err_out_free_ioreq_pool;
 745     fnic->io_sgl_pool[FNIC_SGL_CACHE_DFLT] = pool;
 746 
 747     pool = mempool_create_slab_pool(2, fnic_sgl_cache[FNIC_SGL_CACHE_MAX]);
 748     if (!pool)
             // Bug (3): the value of err is 0
 749         goto err_out_free_dflt_pool;
         ...
 901     return 0;
         ...
 914 err_out_free_dflt_pool:
 915     mempool_destroy(fnic->io_sgl_pool[FNIC_SGL_CACHE_DFLT]);
 916 err_out_free_ioreq_pool:
 917     mempool_destroy(fnic->io_req_pool);
 918 err_out_free_resources:
 919     fnic_free_vnic_resources(fnic);
 920 err_out_clear_intr:
 921     fnic_clear_intr_mode(fnic);
 922 err_out_dev_close:
 923     vnic_dev_close(fnic->vdev);
 924 err_out_vnic_unregister:
 925     vnic_dev_unregister(fnic->vdev);
 926 err_out_iounmap:
 927     fnic_iounmap(fnic);
 928 err_out_release_regions:
 929     pci_release_regions(pdev);
 930 err_out_disable_device:
 931     pci_disable_device(pdev);
 932 err_out_free_hba:
 933     fnic_stats_debugfs_remove(fnic);
 934     scsi_host_put(lp->host);
 935 err_out:
 936     return err;
 937 }

Thanks very much!

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