Re: [PATCH v5 01/10] s390/vfio_ccw: free all memory if cp_init() fails

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbounded mutual recursion between `ccwchain_handle_ccw` and `ccwchain_loop_tic` can lead to kernel stack exhaustion.
- [High] Out-of-bounds read of `cp->guest_cp` in `ccwchain_calc_length`.
--

--- Patch [1]: [PATCH v5 01/10] s390/vfio_ccw: free all memory if cp_init() fails ---
commit fee68998110983d79a8ac5ec5666d53fcd26abad
Author: Eric Farman <farman@linux.ibm.com>

s390/vfio_ccw: free all memory if cp_init() fails

The commit addresses a memory leak in the channel program initialization
process. It moves the cleanup of allocated CCW segments out of the
recursive ccwchain_handle_ccw() logic into a new wrapper function
ccwchain_build_ccws(). This ensures that all segments are freed if an
error occurs.

> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 7561aa7d3e017..086d1b54bdb05 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -455,9 +455,6 @@ static int ccwchain_handle_ccw(dma32_t cda, struct channel_program *cp)
>  	/* Loop for tics on this new chain. */
>  	ret = ccwchain_loop_tic(chain, cp);

[Severity: High]
This is a pre-existing issue, but can this unbounded mutual recursion lead to
kernel stack exhaustion?

If a malicious guest constructs a channel program consisting of a long sequence
of forward-jumping Transfer in Channel (TIC) commands that do not loop back to
previously processed chains, it could trigger unchecked mutual recursion.

When the guest issues an SSCH instruction, the host kernel intercepts it and
evaluates the program. ccwchain_handle_ccw() calls ccwchain_loop_tic(), which
in turn calls ccwchain_handle_ccw() for any new segments:

drivers/s390/cio/vfio_ccw_cp.c:ccwchain_loop_tic() {
	...
		/* Build a ccwchain for the next segment */
		ret = ccwchain_handle_ccw(tic->cda, cp);
		if (ret)
			return ret;
	...
}

Since there is no enforced limit on the recursion depth or the total number of
chains, a long sequence of non-cyclic TICs could cause uncontrolled mutual
recursion, exhausting the kernel stack and causing a host panic.

[Severity: High]
This is also a pre-existing issue, but does ccwchain_calc_length() have an
out-of-bounds read vulnerability?

When ccwchain_handle_ccw() allocates cp->guest_cp, it copies a maximum of
CCWCHAIN_LEN_MAX (256) elements:

drivers/s390/cio/vfio_ccw_cp.c:ccwchain_handle_ccw() {
	...
	ret = vfio_dma_rw(vdev, gcda, cp->guest_cp,
			  CCWCHAIN_LEN_MAX * sizeof(struct ccw1), false);
	...
	/* Count the CCWs in the current chain */
	len = ccwchain_calc_length(gcda, cp);
	...
}

However, in ccwchain_calc_length(), the do-while loop condition allows up to
CCWCHAIN_LEN_MAX + 1 iterations:

drivers/s390/cio/vfio_ccw_cp.c:ccwchain_calc_length() {
	...
	do {
		...
		ccw = cp->guest_cp + cnt;
		...
		if (!ccw_is_chain(ccw) && !is_tic_within_range(ccw, iova, cnt))
			break;
		...
	} while (cnt < CCWCHAIN_LEN_MAX + 1);
	...
}

If a malicious guest submits a channel program where the first 256 CCWs all
have the command-chaining or data-chaining flags enabled, the loop will not
break early and will execute a 257th iteration.

During this iteration, ccw points to the 257th element (index 256), and
ccw_is_chain(ccw) reads its flags byte. This reads past the 2K allocation
boundary, potentially triggering a host kernel panic or memory safety tooling.

>  
> -	if (ret)
> -		ccwchain_free(chain);
> -
>  	return ret;
>  }

[ ... ]

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