[PATCH] scsi: ipr: fix out-of-bounds read in __ipr_format_res_path()

[PATCH] scsi: ipr: fix out-of-bounds read in __ipr_format_res_path()

[Haotian Zhang]

The loop in __ipr_format_res_path() reads res_path[i] before checking
i against IPR_RES_PATH_BYTES, so an adapter-supplied resource path
without a 0xff terminator in its 8 bytes causes a read one byte past
the array.

Move the index check before the element access so the loop stops at
the array boundary.

Fixes: 3e7ebdfa58dd ("[SCSI] ipr: update the configuration table code for the next generation chip")
Cc: stable@vger.kernel.org
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/scsi/ipr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index dbd58a7e7bc1..3cb8fb02d706 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -1217,7 +1217,7 @@ static char *__ipr_format_res_path(u8 *res_path, char *buffer, int len)
 
 	*p = '\0';
 	p += scnprintf(p, buffer + len - p, "%02X", res_path[0]);
-	for (i = 1; res_path[i] != 0xff && i < IPR_RES_PATH_BYTES; i++)
+	for (i = 1; i < IPR_RES_PATH_BYTES && res_path[i] != 0xff; i++)
 		p += scnprintf(p, buffer + len - p, "-%02X", res_path[i]);
 
 	return buffer;
-- 
2.43.0

Re: [PATCH] scsi: ipr: fix out-of-bounds read in __ipr_format_res_path()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing bounds check on hardware-supplied errors_logged causes out-of-bounds array reads.
--

--- Patch [1]: [PATCH] scsi: ipr: fix out-of-bounds read in __ipr_format_res_path() ---
commit bc3f2f125a3eaeb0638d8a5ff264fd2cd1c9634a
Author: Haotian Zhang <vulab@iscas.ac.cn>

scsi: ipr: fix out-of-bounds read in __ipr_format_res_path()

The loop in __ipr_format_res_path() reads res_path[i] before checking
i against IPR_RES_PATH_BYTES. This commit moves the index check before 
the element access so the loop stops at the array boundary.

> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index d207e5e81afe0..26e967783d952 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -1217,7 +1217,7 @@ static char *__ipr_format_res_path(u8 *res_path, char *buffer, int len)
>  
>  	*p = '\0';
>  	p += scnprintf(p, buffer + len - p, "%02X", res_path[0]);
> -	for (i = 1; res_path[i] != 0xff && i < IPR_RES_PATH_BYTES; i++)
> +	for (i = 1; i < IPR_RES_PATH_BYTES && res_path[i] != 0xff; i++)

[Severity: High]
This is a pre-existing issue, but since you are fixing bounds checking in
this file, could there be similar out-of-bounds array reads in
ipr_log_config_error(), ipr_log_enhanced_config_error(), and
ipr_log_sis64_config_error()?

In all three functions, the hardware-supplied error->errors_logged is read
and used unconditionally as a loop bound over the error->dev array:

drivers/scsi/ipr.c:ipr_log_config_error() {
	...
	errors_logged = be32_to_cpu(error->errors_logged);
	...
	for (i = 0; i < errors_logged; i++, dev_entry++) {
	...
}

If a malfunctioning or malicious PCIe device supplies an arbitrarily large
value for errors_logged via DMA, couldn't this loop read out of bounds past
the 3-element error->dev array and potentially cause a kernel panic or leak
kernel memory to the system log?

Should errors_logged be bounded against ARRAY_SIZE(error->dev) in these
functions?

>  		p += scnprintf(p, buffer + len - p, "-%02X", res_path[i]);
>  
>  	return buffer;

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