Re: [PATCH v4 1/9] 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 recursion between `ccwchain_handle_ccw()` and `ccwchain_loop_tic()` can exhaust the host kernel stack.
- [High] An off-by-one error in `ccwchain_calc_length()` causes a heap out-of-bounds read past the end of the `cp->guest_cp` array.
--

--- Patch [1]: [PATCH v4 1/9] s390/vfio_ccw: free all memory if cp_init() fails ---
commit 8ee92e1dc4c9d7d0f67c0ba543dd404b37f81dcc
Author: Eric Farman <farman@linux.ibm.com>

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

This commit addresses a memory leak where CCW segments created as a result
of a Transfer in Channel would not be properly freed if cp_init() fails.
It moves the cleanup logic outside the recursive ccwchain_handle_ccw() and
ccwchain_loop_tic() loops.

> 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 isn't a bug introduced by this patch, but can this unbounded recursion
between ccwchain_handle_ccw() and ccwchain_loop_tic() exhaust the host kernel
stack?

If a guest constructs a channel program consisting of a long linear sequence
of TICs (Transfer in Channel), where each TIC points to a distinct, previously
unvisited address, could this lead to a kernel panic from stack overflow?

Since there is no limit on the recursion depth, ccwchain_handle_ccw() and
ccwchain_loop_tic() recursively call each other until the stack overflows:

ccwchain_handle_ccw() -> ccwchain_loop_tic() -> ccwchain_handle_ccw()

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

[Severity: High]
This is a pre-existing issue, but does an off-by-one error in
ccwchain_calc_length() cause a heap out-of-bounds read past the end of the
cp->guest_cp array?

If the guest provides a CCW chain of exactly CCWCHAIN_LEN_MAX (256) elements,
with all elements having the chain flag set, the loop logic permits
evaluating the chain flag on the out-of-bounds index before realizing
the maximum limit is reached:

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

Could this lead to a kernel Oops if it crosses a page boundary into an
unmapped page, or trigger KASAN panics?

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