lkml.org 
[lkml]   [2018]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject
    Date
    Subject: [PATCH v3] staging: rts5208: add error handling into rtsx_probe

    If rtsx_probe() fails to allocate dev->chip, then release_everything()
    will crash on uninitialized dev->cmnd_ready complete.

    Patch adds error handling into rtsx_probe.

    Found by Linux Driver Verification project (linuxtesting.org).

    Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
    ---
    v3: fix subject and commit message
    v2: Add error handling into rtsx_probe based on Dan Carpenter's comment.
    I do not have corresponding hardware, so patch was tested by compilation only.

    I faced with inaccuracy at rtsx_remove() and original rtsx_probe():
    there is quiesce_and_remove_host() call with scsi_remove_host() inside,
    whereas release_everything() calls scsi_host_put() after this
    scsi_remove_host() call. This is strange for me.
    Also I do not know is it require to check result value of
    rtsx_init_chip() call on rtsx_probe().
    ---
    drivers/staging/rts5208/rtsx.c | 38 +++++++++++++++++++++++-----------
    1 file changed, 26 insertions(+), 12 deletions(-)

    diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
    index 70e0b8623110..69e6abe14abf 100644
    --- a/drivers/staging/rts5208/rtsx.c
    +++ b/drivers/staging/rts5208/rtsx.c
    @@ -857,7 +857,7 @@ static int rtsx_probe(struct pci_dev *pci,
    dev->chip = kzalloc(sizeof(*dev->chip), GFP_KERNEL);
    if (!dev->chip) {
    err = -ENOMEM;
    - goto errout;
    + goto chip_alloc_fail;
    }

    spin_lock_init(&dev->reg_lock);
    @@ -879,7 +879,7 @@ static int rtsx_probe(struct pci_dev *pci,
    if (!dev->remap_addr) {
    dev_err(&pci->dev, "ioremap error\n");
    err = -ENXIO;
    - goto errout;
    + goto ioremap_fail;
    }

    /*
    @@ -894,7 +894,7 @@ static int rtsx_probe(struct pci_dev *pci,
    if (!dev->rtsx_resv_buf) {
    dev_err(&pci->dev, "alloc dma buffer fail\n");
    err = -ENXIO;
    - goto errout;
    + goto dma_alloc_fail;
    }
    dev->chip->host_cmds_ptr = dev->rtsx_resv_buf;
    dev->chip->host_cmds_addr = dev->rtsx_resv_buf_addr;
    @@ -915,7 +915,7 @@ static int rtsx_probe(struct pci_dev *pci,

    if (rtsx_acquire_irq(dev) < 0) {
    err = -EBUSY;
    - goto errout;
    + goto irq_acquire_fail;
    }

    pci_set_master(pci);
    @@ -935,14 +935,14 @@ static int rtsx_probe(struct pci_dev *pci,
    if (IS_ERR(th)) {
    dev_err(&pci->dev, "Unable to start control thread\n");
    err = PTR_ERR(th);
    - goto errout;
    + goto control_thread_fail;
    }
    dev->ctl_thread = th;

    err = scsi_add_host(host, &pci->dev);
    if (err) {
    dev_err(&pci->dev, "Unable to add the scsi host\n");
    - goto errout;
    + goto scsi_add_host_fail;
    }

    /* Start up the thread for delayed SCSI-device scanning */
    @@ -950,18 +950,16 @@ static int rtsx_probe(struct pci_dev *pci,
    if (IS_ERR(th)) {
    dev_err(&pci->dev, "Unable to start the device-scanning thread\n");
    complete(&dev->scanning_done);
    - quiesce_and_remove_host(dev);
    err = PTR_ERR(th);
    - goto errout;
    + goto scan_thread_fail;
    }

    /* Start up the thread for polling thread */
    th = kthread_run(rtsx_polling_thread, dev, "rtsx-polling");
    if (IS_ERR(th)) {
    dev_err(&pci->dev, "Unable to start the device-polling thread\n");
    - quiesce_and_remove_host(dev);
    err = PTR_ERR(th);
    - goto errout;
    + goto scan_thread_fail;
    }
    dev->polling_thread = th;

    @@ -970,9 +968,25 @@ static int rtsx_probe(struct pci_dev *pci,
    return 0;

    /* We come here if there are any problems */
    -errout:
    +scan_thread_fail:
    + quiesce_and_remove_host(dev);
    +scsi_add_host_fail:
    + complete(&dev->cmnd_ready);
    + wait_for_completion(&dev->control_exit);
    +control_thread_fail:
    + free_irq(dev->irq, (void *)dev);
    + rtsx_release_chip(dev->chip);
    +irq_acquire_fail:
    + dev->chip->host_cmds_ptr = NULL;
    + dev->chip->host_sg_tbl_ptr = NULL;
    + if (dev->chip->msi_en)
    + pci_disable_msi(dev->pci);
    +dma_alloc_fail:
    + iounmap(dev->remap_addr);
    +ioremap_fail:
    + kfree(dev->chip);
    +chip_alloc_fail:
    dev_err(&pci->dev, "%s failed\n", __func__);
    - release_everything(dev);

    return err;
    }
    --
    2.17.1
    \
     
     \ /
      Last update: 2018-06-13 19:32    [W:2.147 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site