[PATCH] scsi: hpsa: clamp physical LUN count in fallback report

[PATCH] scsi: hpsa: clamp physical LUN count in fallback report

[Pengpeng Hou]

When REPORT PHYS EXTENDED is not supported,
hpsa_scsi_do_report_phys_luns() falls back to REPORT LUNS and converts
each 8-byte LUN entry into the extended report buffer. The number of
entries comes from the device-provided LUNListLength field, but the loop
did not clamp it to either the source ReportLUNdata array or the
destination ReportExtendedLUNdata array.

Clamp the converted count to both arrays and update the output
LUNListLength to match the entries that were actually copied.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/scsi/hpsa.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a1b116cd..a7115efd 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3797,6 +3797,10 @@ static inline int hpsa_scsi_do_report_phys_luns(struct ctlr_info *h,
 		/* Copy ReportLUNdata header */
 		memcpy(buf, lbuf, 8);
 		nphys = be32_to_cpu(*((__be32 *)lbuf->LUNListLength)) / 8;
+		nphys = min_t(u32, nphys, ARRAY_SIZE(lbuf->LUN));
+		nphys = min_t(u32, nphys, ARRAY_SIZE(buf->LUN));
+		put_unaligned_be32(nphys * sizeof(buf->LUN[0]),
+				   buf->LUNListLength);
 		for (i = 0; i < nphys; i++)
 			memcpy(buf->LUN[i].lunid, lbuf->LUN[i], 8);
 	}

Re: [PATCH] scsi: hpsa: clamp physical LUN count in fallback report

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The fallback path fails to zero out the remaining fields of the 24-byte extended LUN entry, leaving them populated with garbage data from a prior failed DMA transfer.
--

commit 2d3d5c4cd564b5d4730f076194c9998a95c658a9
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

scsi: hpsa: clamp physical LUN count in fallback report

When REPORT PHYS EXTENDED is not supported, the driver falls back to REPORT LUNS
and converts entries. This commit clamps the converted count to array bounds and
updates the output LUNListLength accordingly.

> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index a1b116cd4723d..a7115efdde7fe 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -3797,6 +3797,10 @@ static inline int hpsa_scsi_do_report_phys_luns(struct ctlr_info *h,
>  		/* Copy ReportLUNdata header */
>  		memcpy(buf, lbuf, 8);
>  		nphys = be32_to_cpu(*((__be32 *)lbuf->LUNListLength)) / 8;
> +		nphys = min_t(u32, nphys, ARRAY_SIZE(lbuf->LUN));
> +		nphys = min_t(u32, nphys, ARRAY_SIZE(buf->LUN));
> +		put_unaligned_be32(nphys * sizeof(buf->LUN[0]),
> +				   buf->LUNListLength);
>  		for (i = 0; i < nphys; i++)
>  			memcpy(buf->LUN[i].lunid, lbuf->LUN[i], 8);

[Severity: High]
This is a pre-existing issue, but does this fallback path fail to clear the
remaining fields of the 24-byte extended LUN entry?

When the initial hpsa_scsi_do_report_luns() call fails with -EOPNOTSUPP, it
may have successfully completed a DMA transfer that populated buf with standard
8-byte LUN data.

This loop only overwrites the 8-byte lunid of the 24-byte extended LUN entry,
leaving the trailing 16 bytes populated with data from the prior failed DMA
transfer. 

Could this cause hardware property corruption? Data structures like wwid,
device_type, device_flags, and ioaccel_handle might be populated with bytes
from adjacent LUN IDs, potentially causing the driver to misidentify devices
or use invalid IOAccel handles leading to IO failures or crashes.

>  	}
>  	kfree(lbuf);
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701053626.44118-1-pengpeng@iscas.ac.cn?part=1