scsi: smartpqi: fix memory leak in pqi_report_phys_luns()

[PATCH] scsi: smartpqi: fix memory leak in pqi_report_phys_luns()

Posted by Zilin Guan 1 week, 1 day ago

pqi_report_phys_luns() fails to release the rpl_list buffer when
encountering an unsupported data format or when the allocation for
rpl_16byte_wwid_list fails. These early returns bypass the cleanup
logic, leading to memory leaks.

Consolidate the error handling by adding an out_free_rpl_list label
and use goto statements to ensure rpl_list is consistently freed
on failure.

Compile tested only. Issue found using a prototype static analysis tool
and code review.

Fixes: 28ca6d876c5a ("scsi: smartpqi: Add extended report physical LUNs")
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
 drivers/scsi/smartpqi/smartpqi_init.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index fe549e2b7c94..c829d9590558 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -1241,7 +1241,8 @@ static inline int pqi_report_phys_luns(struct pqi_ctrl_info *ctrl_info, void **b
 			dev_err(&ctrl_info->pci_dev->dev,
 				"RPL returned unsupported data format %u\n",
 				rpl_response_format);
-			return -EINVAL;
+			rc = -EINVAL;
+			goto out_free_rpl_list;
 		} else {
 			dev_warn(&ctrl_info->pci_dev->dev,
 				"RPL returned extended format 2 instead of 4\n");
@@ -1253,8 +1254,10 @@ static inline int pqi_report_phys_luns(struct pqi_ctrl_info *ctrl_info, void **b
 
 	rpl_16byte_wwid_list = kmalloc(struct_size(rpl_16byte_wwid_list, lun_entries,
 						   num_physicals), GFP_KERNEL);
-	if (!rpl_16byte_wwid_list)
-		return -ENOMEM;
+	if (!rpl_16byte_wwid_list) {
+		rc = -ENOMEM;
+		goto out_free_rpl_list;
+	}
 
 	put_unaligned_be32(num_physicals * sizeof(struct report_phys_lun_16byte_wwid),
 		&rpl_16byte_wwid_list->header.list_length);
@@ -1275,6 +1278,10 @@ static inline int pqi_report_phys_luns(struct pqi_ctrl_info *ctrl_info, void **b
 	*buffer = rpl_16byte_wwid_list;
 
 	return 0;
+
+out_free_rpl_list:
+	kfree(rpl_list);
+	return rc;
 }
 
 static inline int pqi_report_logical_luns(struct pqi_ctrl_info *ctrl_info, void **buffer)
-- 
2.34.1

Re: [PATCH] scsi: smartpqi: fix memory leak in pqi_report_phys_luns()

Posted by Martin K. Petersen 4 days, 11 hours ago

Zilin,

> pqi_report_phys_luns() fails to release the rpl_list buffer when
> encountering an unsupported data format or when the allocation for
> rpl_16byte_wwid_list fails. These early returns bypass the cleanup
> logic, leading to memory leaks.

Applied to 6.20/scsi-staging, thanks!

-- 
Martin K. Petersen

Re: [PATCH] scsi: smartpqi: fix memory leak in pqi_report_phys_luns()

Posted by Markus Elfring 1 week ago

…
> Fixes: 28ca6d876c5a ("scsi: smartpqi: Add extended report physical LUNs")
…

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v6.19-rc7#n34


…
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
…
> @@ -1275,6 +1278,10 @@ static inline int pqi_report_phys_luns(struct pqi_ctrl_info *ctrl_info, void **b
>  	*buffer = rpl_16byte_wwid_list;
>  
>  	return 0;
> +
> +out_free_rpl_list:
> +	kfree(rpl_list);
> +	return rc;
>  }
…

* How do you think about to refer to the local variable “rpl_8byte_wwid_list” instead?

* Would you like to refine the control flow a bit more for the end
  of this function implementation?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.19-rc7#n526


Regards,
Markus