Re: [PATCH] scsi: core: bound the VPD page 0x83 designator walk

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing descriptor payload length check in `scsi_vpd_tpg_id()` leads to an out-of-bounds read when accessing `d[6]` and `d[7]`.
--

--- Patch [1]: [PATCH] scsi: core: bound the VPD page 0x83 designator walk ---
commit 390f380557d52bf9b1b813ffda5cbf376ee027a2
Author: Michael Bommarito <michael.bommarito@gmail.com>

scsi: core: bound the VPD page 0x83 designator walk

This commit fixes an out-of-bounds read in the VPD page 0x83 designator
walk by ensuring the designator length does not exceed the remaining bytes
in the page. It updates scsi_vpd_lun_id(), scsi_vpd_tpg_id(), and
sd_get_unique_id() to bound each iteration.

> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c

[ ... ]

> @@ -3546,8 +3554,14 @@ int scsi_vpd_tpg_id(struct scsi_device *sdev, int *rel_id)
>  		return -ENXIO;
>  	}
>  
> -	d = vpd_pg83->data + 4;
> -	while (d < vpd_pg83->data + vpd_pg83->len) {
> +	for (off = 4; off < vpd_pg83->len; off += d[3] + 4) {
> +		if (vpd_pg83->len - off < 4)
> +			break;
> +
> +		d = vpd_pg83->data + off;
> +		if (d[3] > vpd_pg83->len - off - 4)
> +			break;
> +
>  		switch (d[1] & 0xf) {
>  		case 0x4:
>  			/* Relative target port */
>  			rel_port = get_unaligned_be16(&d[6]);
>  			break;
>  		case 0x5:
>  			/* Target port group */
>  			group_id = get_unaligned_be16(&d[6]);
>  			break;

[Severity: High]
This is a pre-existing issue, but can this code read out of bounds if the
designator payload length is less than 4?

In scsi_vpd_tpg_id(), the switch statement unconditionally reads d[6] and d[7]
via get_unaligned_be16 without verifying that the designator length (d[3]) is
at least 4.

If a device or untrusted hypervisor backend provides a VPD page 0x83 containing
a designator with type 0x4 or 0x5 and a declared length (d[3]) of less than 4,
and this designator is positioned at the very end of the buffer, could the
fixed-offset read at &d[6] access memory out of bounds?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711150718.2916641-1-michael.bommarito@gmail.com?part=1