[PATCH] hisi_sas: Fix unreasonable exit processing of hisi_sas_v3_probe

[PATCH] hisi_sas: Fix unreasonable exit processing of hisi_sas_v3_probe

[Ye Bin]

In this function, the exception return missed some processing.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index fa05e612d85a..394e20b8f622 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3166,8 +3166,8 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct Scsi_Host *shost;
 	struct hisi_hba *hisi_hba;
 	struct device *dev = &pdev->dev;
-	struct asd_sas_phy **arr_phy;
-	struct asd_sas_port **arr_port;
+	struct asd_sas_phy **arr_phy = NULL;
+	struct asd_sas_port **arr_port = NULL;
 	struct sas_ha_struct *sha;
 	int rc, phy_nr, port_nr, i;
 
@@ -3213,7 +3213,7 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	arr_port = devm_kcalloc(dev, port_nr, sizeof(void *), GFP_KERNEL);
 	if (!arr_phy || !arr_port) {
 		rc = -ENOMEM;
-		goto err_out_ha;
+		goto err_out_iomap;
 	}
 
 	sha->sas_phy = arr_phy;
@@ -3254,7 +3254,7 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	rc = scsi_add_host(shost, dev);
 	if (rc)
-		goto err_out_ha;
+		goto err_out_iomap;
 
 	rc = sas_register_ha(sha);
 	if (rc)
@@ -3262,14 +3262,20 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	rc = hisi_hba->hw->hw_init(hisi_hba);
 	if (rc)
-		goto err_out_register_ha;
+		goto err_out_hw_init;
 
 	scsi_scan_host(shost);
 
 	return 0;
 
+err_out_hw_init:
+	sas_unregister_ha(sha);
 err_out_register_ha:
 	scsi_remove_host(shost);
+err_out_iomap:
+	devm_kfree(dev, arr_phy);
+	devm_kfree(dev, arr_port);
+	pcim_iounmap(pdev, hisi_hba->regs);
 err_out_ha:
 	hisi_sas_debugfs_exit(hisi_hba);
 	scsi_host_put(shost);
-- 
2.17.2

Re: [PATCH] hisi_sas: Fix unreasonable exit processing of hisi_sas_v3_probe

[John Garry]

On 23/01/2020 06:12, Ye Bin wrote:

subject: scsi: hisi_sas: Fix...

/s/unreasonable/broken/

> In this function, the exception return missed some processing.

This is the same as hisi_sas_probe(), so, if this is wrong, then that is 
wrong also, i.e. needs to be fixed.

> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>   drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index fa05e612d85a..394e20b8f622 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> @@ -3166,8 +3166,8 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	struct Scsi_Host *shost;
>   	struct hisi_hba *hisi_hba;
>   	struct device *dev = &pdev->dev;
> -	struct asd_sas_phy **arr_phy;
> -	struct asd_sas_port **arr_port;
> +	struct asd_sas_phy **arr_phy = NULL;
> +	struct asd_sas_port **arr_port = NULL;
>   	struct sas_ha_struct *sha;
>   	int rc, phy_nr, port_nr, i;
>   
> @@ -3213,7 +3213,7 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	arr_port = devm_kcalloc(dev, port_nr, sizeof(void *), GFP_KERNEL);
>   	if (!arr_phy || !arr_port) {
>   		rc = -ENOMEM;
> -		goto err_out_ha;
> +		goto err_out_iomap;
>   	}
>   
>   	sha->sas_phy = arr_phy;
> @@ -3254,7 +3254,7 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   
>   	rc = scsi_add_host(shost, dev);
>   	if (rc)
> -		goto err_out_ha;
> +		goto err_out_iomap;
>   
>   	rc = sas_register_ha(sha);
>   	if (rc)
> @@ -3262,14 +3262,20 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   
>   	rc = hisi_hba->hw->hw_init(hisi_hba);
>   	if (rc)
> -		goto err_out_register_ha;
> +		goto err_out_hw_init;
>   
>   	scsi_scan_host(shost);
>   
>   	return 0;
>   
> +err_out_hw_init:
> +	sas_unregister_ha(sha);
>   err_out_register_ha:
>   	scsi_remove_host(shost);
> +err_out_iomap:
> +	devm_kfree(dev, arr_phy);
> +	devm_kfree(dev, arr_port);

why do you need this? Surely these memories will be freed automatically 
for probe failure.

> +	pcim_iounmap(pdev, hisi_hba->regs);

This seems to be missing from hisi_sas_v3_remove() also.

>   err_out_ha:
>   	hisi_sas_debugfs_exit(hisi_hba);
>   	scsi_host_put(shost);
> 


Thanks,
John