Re: [PATCH v6 05/10] s390/vfio_ccw: calculate idal length based on idaw type

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] When processing a CCW that does not transfer data (e.g., a read command with the SKIP flag set), vfio-ccw fails to translate the IDAL, passing raw guest IOVAs directly to the host channel subsystem, which causes host IOMMU faults.
--

--- Patch [5]: [PATCH v6 05/10] s390/vfio_ccw: calculate idal length based on idaw type ---
commit 6c424b4fc15dc709c90a5e16ed87383742a135f1
Author: Eric Farman <farman@linux.ibm.com>

s390/vfio_ccw: calculate idal length based on idaw type

The output of vfio-ccw is always Format-2, but the input can be either
Format-1 (31-bit addresses) or Format-2 (64-bit addresses). As a result,
the size of the guest IDAL may be incorrect and should be trimmed down.

Link: https://lore.kernel.org/r/20260720203400.7328E1F000E9@smtp.kernel.org/

> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 6275794751cb4..5ef082b8289af 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
[ ... ]
> @@ -233,6 +233,7 @@ static void convert_ccw0_to_ccw1(struct ccw1 *source, unsigned long len)
>  }
>  
>  #define idal_is_2k(_cp) (!(_cp)->orb.cmd.c64 || (_cp)->orb.cmd.i2k)
> +#define get_idaw_size(_cp) ((_cp)->orb.cmd.c64 ? sizeof(u64) : sizeof(u32))
>  
>  /*
>   * Helpers to operate ccwchain.
[ ... ]

While reviewing this, I noticed a separate issue in this file.

drivers/s390/cio/vfio_ccw_cp.c:ccwchain_fetch_ccw()
	if (ccw_does_data_transfer(ccw)) {
		ret = page_array_pin(pa, vdev, idal_is_2k(cp));
		if (ret < 0)
			goto out_unpin;
	} else {
		pa->pa_nr = 0;
	}

	ccw->cda = virt_to_dma32(idaws);
	ccw->flags |= CCW_FLAG_IDA;

	/* Populate the IDAL with pinned/translated addresses from page */
	page_array_idal_create_words(pa, idaws);

[Severity: High]
This isn't a bug introduced by this patch, but when processing a CCW that does
not transfer data, such as a read command with the SKIP flag set, does vfio-ccw
fail to translate the IDAL?

Because ccw_does_data_transfer(ccw) returns false, pa->pa_nr is set to 0. This
prevents page_array_idal_create_words() from overwriting the raw guest IOVAs
with translated host addresses.

Could a guest VM submit a channel program with a CCW where ccw_is_skip() and
ccw_is_read() are true, containing an arbitrary guest physical address?

Since the untranslated guest IOVAs remain in the idaws array, passing them
directly to the host channel subsystem might trigger a host IOMMU fault or
Channel Program Check, potentially leading to a host denial of service.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726172153.1818056-1-farman@linux.ibm.com?part=5