Message ID | 1442439049-49970-1-git-send-email-mrochs@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
On 09/16/2015 04:30 PM, Matthew R. Ochs wrote: > Currently, scsi_host_put() is being called prematurely in the > remove path and is missing entirely in an error cleanup path. > The former can lead to memory being freed too early with > subsequent access potentially corrupting data whilst the former > would result in a memory leak. > > Move the usage on remove to be the last cleanup action taken > and introduce a call to scsi_host_put() in the one initialization > error path that does not use remove to cleanup. > > Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com> > Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com> > --- > drivers/scsi/cxlflash/main.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c > index fc77cd4..1856a73 100644 > --- a/drivers/scsi/cxlflash/main.c > +++ b/drivers/scsi/cxlflash/main.c > @@ -734,7 +734,6 @@ static void cxlflash_remove(struct pci_dev *pdev) > case INIT_STATE_SCSI: > cxlflash_term_local_luns(cfg); > scsi_remove_host(cfg->host); > - scsi_host_put(cfg->host); > /* Fall through */ > case INIT_STATE_AFU: > term_afu(cfg); > @@ -744,6 +743,7 @@ static void cxlflash_remove(struct pci_dev *pdev) > case INIT_STATE_NONE: > flush_work(&cfg->work_q); > free_mem(cfg); > + scsi_host_put(cfg->host); > break; > } > > @@ -2415,6 +2415,7 @@ static int cxlflash_probe(struct pci_dev *pdev, > dev_err(&pdev->dev, "%s: call to scsi_host_alloc failed!\n", This message text is wrong. Its the call to alloc_mem that has failed in this leg, not the call to scsi_host_alloc. > __func__); > rc = -ENOMEM; > + scsi_host_put(cfg->host); > goto out; > } >
> On Sep 22, 2015, at 3:53 PM, Brian King <brking@linux.vnet.ibm.com> wrote: > On 09/16/2015 04:30 PM, Matthew R. Ochs wrote: >> Currently, scsi_host_put() is being called prematurely in the >> remove path and is missing entirely in an error cleanup path. >> The former can lead to memory being freed too early with >> subsequent access potentially corrupting data whilst the former >> would result in a memory leak. >> >> Move the usage on remove to be the last cleanup action taken >> and introduce a call to scsi_host_put() in the one initialization >> error path that does not use remove to cleanup. >> >> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com> >> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com> >> --- >> drivers/scsi/cxlflash/main.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c >> index fc77cd4..1856a73 100644 >> --- a/drivers/scsi/cxlflash/main.c >> +++ b/drivers/scsi/cxlflash/main.c >> @@ -734,7 +734,6 @@ static void cxlflash_remove(struct pci_dev *pdev) >> case INIT_STATE_SCSI: >> cxlflash_term_local_luns(cfg); >> scsi_remove_host(cfg->host); >> - scsi_host_put(cfg->host); >> /* Fall through */ >> case INIT_STATE_AFU: >> term_afu(cfg); >> @@ -744,6 +743,7 @@ static void cxlflash_remove(struct pci_dev *pdev) >> case INIT_STATE_NONE: >> flush_work(&cfg->work_q); >> free_mem(cfg); >> + scsi_host_put(cfg->host); >> break; >> } >> >> @@ -2415,6 +2415,7 @@ static int cxlflash_probe(struct pci_dev *pdev, >> dev_err(&pdev->dev, "%s: call to scsi_host_alloc failed!\n", > > This message text is wrong. Its the call to alloc_mem that has failed in this > leg, not the call to scsi_host_alloc. Good find. I'll fix this in a separate patch. -matt
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index fc77cd4..1856a73 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -734,7 +734,6 @@ static void cxlflash_remove(struct pci_dev *pdev) case INIT_STATE_SCSI: cxlflash_term_local_luns(cfg); scsi_remove_host(cfg->host); - scsi_host_put(cfg->host); /* Fall through */ case INIT_STATE_AFU: term_afu(cfg); @@ -744,6 +743,7 @@ static void cxlflash_remove(struct pci_dev *pdev) case INIT_STATE_NONE: flush_work(&cfg->work_q); free_mem(cfg); + scsi_host_put(cfg->host); break; } @@ -2415,6 +2415,7 @@ static int cxlflash_probe(struct pci_dev *pdev, dev_err(&pdev->dev, "%s: call to scsi_host_alloc failed!\n", __func__); rc = -ENOMEM; + scsi_host_put(cfg->host); goto out; }