Re: [PATCH v3 2/8] s390/vfio_ccw: limit the number of channel program segments

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Off-by-one error in ccwchain_calc_length allows a guest-triggered out-of-bounds read.
--

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

This commit limits the number of channel program segments that can exist
in a single channel program. It breaks channel programs into segments
when it encounters a Transfer-In-Channel (TIC) CCW, and adds a check to
ensure the segment count does not exceed a global limit. This prevents
hitting an arbitrary limit due to recursive processing.

> 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 isn't a bug introduced by this patch, but does ccwchain_calc_length()
contain an off-by-one error that allows a guest-triggered out-of-bounds read?

When a guest VM submits a channel program containing a sequence of 256 CCWs
with the command-chaining or data-chaining flag set, the driver handles
this request and calls ccwchain_calc_length() to parse the chain.

The array cp->guest_cp is allocated to hold exactly CCWCHAIN_LEN_MAX (256)
elements. However, the loop in ccwchain_calc_length() uses the condition
while (cnt < CCWCHAIN_LEN_MAX + 1):

drivers/s390/cio/vfio_ccw_cp.c:ccwchain_calc_length() {
	do {
		cnt++;
		...
		if (!ccw_is_chain(ccw) && !is_tic_within_range(ccw, iova, cnt))
			break;

		ccw++;
	} while (cnt < CCWCHAIN_LEN_MAX + 1);
}

Does this allow the loop to iterate up to 257 times? During the 257th
iteration (cnt = 257), does the pointer ccw advance past the end of the
cp->guest_cp array, causing the ccw_is_chain(ccw) macro to dereference
an out-of-bounds pointer to read ccw->flags?

Could this lead to a kernel heap out-of-bounds read and subsequent kernel
panic if KASAN is enabled, allowing an unprivileged guest to impact host
stability?

> +	/* 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/20260723174751.1180334-1-farman@linux.ibm.com?part=2