Re: [PATCH v2] vhost-scsi: do not count PI bytes in the data length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] vhost-scsi: missing validation of prot_bytes alignment allows OOB access
--

commit a2683cd412935b122ac2e93ce16af9655a892639
Author: Jia Jia <physicalmtea@gmail.com>

vhost-scsi: do not count PI bytes in the data length

This commit updates the expected data length passed to the target core
by omitting the protection information (PI) bytes. It adjusts the length
so that protected read and write operations take their length directly
from the CDB, preventing issues when the data scatterlist is smaller
than the total length.

> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 885fac5ed042e..8b32caa7ec294 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1534,9 +1534,13 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
>  		 * vhost_scsi_queue_data_in() and vhost_scsi_queue_status()
>  		 */
>  		cmd->tvc_vq_desc = vc.head;
> +		/*
> +		 * exp_data_len is the data sgl. prot_bytes already sit in
> +		 * the protection sgl. Protected READ and WRITE replace
> +		 * data_length from the CDB in sbc_check_prot().
> +		 */
>  		vhost_scsi_target_queue_cmd(nexus, cmd, cdb, lun, task_attr,
> -					    data_direction,
> -					    exp_data_len + prot_bytes);
> +					    data_direction, exp_data_len);

[Severity: Critical]
Does this code allow an out-of-bounds access when prot_bytes is not aligned?

In vhost_scsi_handle_vq(), prot_bytes is extracted from the virtqueue element:

    prot_bytes = vhost32_to_cpu(vq, v_req_pi.pi_bytesin);

There doesn't appear to be validation ensuring it is a multiple of the 8-byte
T10 PI tuple size before it dictates the length of the protection scatterlist.

When the target core later processes this command, functions like
sbc_dif_generate() in drivers/target/target_core_sbc.c loop over the
scatterlist length using 8-byte increments:

    for (j = 0; j < psg->length; j += sizeof(*sdt)) {

If psg->length is not a multiple of 8, does the last iteration read or write
past the end of the mapped scatterlist element? A malicious guest could provide
an unaligned pi_bytesin or pi_bytesout length, potentially causing an
out-of-bounds access into an adjacent physical page on the host.

>  		ret = 0;
>  err:

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