Re: [PATCH v4 2/9] 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:
- [High] The `ccwchain_calc_length` function can read out-of-bounds when evaluating a maximum-length CCW chain.
- [Medium] `vfio_dma_rw` unconditionally fetches exactly 2K bytes regardless of actual chain length.
--

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

This commit imposes a limit on the number of channel program segments
(linked by TIC) that can exist in a single channel program. It breaks
channel programs into segments when encountering a Transfer-In-Channel (TIC)
CCW and enforces a global limit on the number of segments to prevent unbounded
recursion.

[ ... ]
> 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;
>  
> +	/* Limit number of chains in a single channel program */
> +	if (cp->ccwchain_count >= CCWCHAIN_COUNT_MAX)
> +		return -EINVAL;
> +

[Severity: Medium]
This isn't a bug introduced by this patch, but does the unconditional read of
2K bytes in ccwchain_handle_ccw() cause valid short programs to fail?

Earlier in ccwchain_handle_ccw(), the code performs a hardcoded 2K read before
calculating the actual chain length:

ccwchain_handle_ccw() {
    ...
    ret = vfio_dma_rw(vdev, gcda, cp->guest_cp,
                      CCWCHAIN_LEN_MAX * sizeof(struct ccw1), false);
    if (ret)
        return ret;
    ...
}

If a guest submits a short channel program located near the end of a mapped
memory region, and the adjacent page is unmapped, vfio_dma_rw() will fail the
entire transaction instead of returning a partial read.

Could this lead to rejection of valid channel programs based on their memory
layout?

[Severity: High]
This is a pre-existing issue, but can the length calculation read out of
bounds?

In ccwchain_calc_length(), which is called just before the new limit check,
the code evaluates a maximum-length CCW chain:

ccwchain_calc_length() {
    ...
    if (!ccw_is_chain(ccw) && !is_tic_within_range(ccw, iova, cnt))
        break;
    ...
}

If a guest VM submits a channel program where the first 256 CCWs all have
the chain flag set, the loop increments the counter to 257. On the 257th
iteration, calling ccw_is_chain(ccw) reads the flags field from guest_cp[256],
which is one element past the end of the allocated CCWCHAIN_LEN_MAX array.

Can an unprivileged guest trigger an out-of-bounds read by providing a
specially crafted channel program?

>  	/* Need alloc a new chain for this one. */
>  	chain = ccwchain_alloc(cp, len);
>  	if (!chain)
[ ... ]

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