Re: [PATCH v8 02/10] s390/vfio_ccw: limit the number of channel program segments

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] A malicious guest can trigger a host kernel out-of-bounds read and potentially execute DMA to arbitrary host physical addresses by concurrently modifying the first IDAW in a channel program.
- [High] An off-by-one error in `ccwchain_calc_length` causes a deterministic out-of-bounds read when processing a channel program with exactly `CCWCHAIN_LEN_MAX` chained CCWs, which can crash the host if KASAN is enabled.
--

--- Patch [2]: [PATCH v8 02/10] s390/vfio_ccw: limit the number of channel program segments ---
commit 8d46ac3adbb3bfeb184e8057b932faa35fe79719
Author: Eric Farman <farman@linux.ibm.com>

s390/vfio_ccw: limit the number of channel program segments

The processing of channel programs, and the CCWs within them, is done
recursively. As such, there is an arbitrary (but not architectural)
limit to the number of CCWs that can exist in a single channel program.
The vfio-ccw logic breaks these channel programs into segments whenever
it encounters a Transfer-In-Channel (TIC) CCW, and the combined number
of segments count towards the global limit. Impose an equivalent limit
to the number of segments until such logic can be made non-recursive.

> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 086d1b54bdb05..1c2890d139c6b 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c

[ ... ]

> @@ -441,6 +442,10 @@ static int ccwchain_handle_ccw(dma32_t cda, struct channel_program *cp)
>  	if (len < 0)
>  		return len;
>  

[Severity: High]
This is a pre-existing issue, but does ccwchain_calc_length() have an
off-by-one error leading to an out-of-bounds read?

When a guest provides a channel program with exactly CCWCHAIN_LEN_MAX chained
CCWs, the do-while loop in ccwchain_calc_length() executes 256 times:

drivers/s390/cio/vfio_ccw_cp.c:ccwchain_calc_length() {
    ...
    } while (cnt < CCWCHAIN_LEN_MAX + 1);
    ...
}

This increments cnt to 256 and advances ccw to point to guest_cp[256].
Since the loop condition (256 < 257) evaluates to true, the loop executes
a 257th time, dereferencing ccw in !ccw_is_chain(ccw). Since guest_cp is
allocated with exactly 256 elements, this read crosses the slab boundary.

Could this crash the host if KASAN is enabled?

[Severity: Critical]
This is a pre-existing issue, but can a malicious guest trigger an
out-of-bounds read and potentially execute DMA to arbitrary host physical
addresses by concurrently modifying the first IDAW in a channel program?

When fetching a guest CCW, ccw_count_idaws() performs the first fetch from
guest memory to calculate the required array size:

drivers/s390/cio/vfio_ccw_cp.c:ccw_count_idaws() {
    ...
    ret = vfio_dma_rw(vdev, dma32_to_u32(ccw->cda), &iova, size, false);
    ...
}

It then calls get_guest_idal(), which allocates a kernel buffer and reads the
entire IDAL from guest memory again into the undersized kernel buffer:

drivers/s390/cio/vfio_ccw_cp.c:get_guest_idal() {
    ...
    ret = vfio_dma_rw(vdev, dma32_to_u32(ccw->cda), idaws, idal_len, false);
    ...
}

If a guest provides a page-aligned first IDAW during the first fetch, and
concurrently modifies its memory so the second fetch reads a misaligned first
IDAW, the hardware will require more IDAWs than originally allocated.

Does this cause the hardware to read past the end of the idaws kernel buffer
and interpret adjacent memory as host physical addresses for DMA?

> +	/* Limit number of chains in a single channel program */
> +	if (cp->ccwchain_count >= CCWCHAIN_COUNT_MAX)
> +		return -EINVAL;
> +
>  	/* Need alloc a new chain for this one. */
>  	chain = ccwchain_alloc(cp, len);

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