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

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 <[email protected]>

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/[email protected]?part=1