Re: [PATCH v2 3/7] s390/vfio_ccw: fix out of bounds check on CCW array

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver does not explicitly cancel asynchronous background works (io_work and crw_work) during device removal or release, which can lead to a Use-After-Free if a work executes after the private device structure is freed.
--

--- Patch [3]: [PATCH v2 3/7] s390/vfio_ccw: fix out of bounds check on CCW array ---
commit c4537e2a2ac0b540c3111dbbbb37763f251c94df
Author: Eric Farman <farman@linux.ibm.com>

s390/vfio_ccw: fix out of bounds check on CCW array

This commit fixes an out-of-bounds check in ccwchain_calc_length() when
counting CCWs. It restructures the loop to break as soon as the maximum of
256 CCWs are examined, preventing memory reads outside the array range.

> diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> index 7abdebd8f7c3d..ce61858c82eb6 100644
> --- a/drivers/s390/cio/vfio_ccw_cp.c
> +++ b/drivers/s390/cio/vfio_ccw_cp.c
> @@ -393,11 +393,14 @@ static int ccwchain_calc_length(u64 iova, struct channel_program *cp)
>  		if (!ccw_is_chain(ccw) && !is_tic_within_range(ccw, iova, cnt))
>  			break;
>  
> -		ccw++;
> -	} while (cnt < CCWCHAIN_LEN_MAX + 1);
> +		/* Exit the loop when we reach the maximum */
> +		if (cnt >= CCWCHAIN_LEN_MAX) {
> +			cnt = -EINVAL;
> +			break;
> +		}
>  
> -	if (cnt == CCWCHAIN_LEN_MAX + 1)
> -		cnt = -EINVAL;
> +		ccw++;
> +	} while (1);
>  
>  	return cnt;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
during device teardown in drivers/s390/cio/vfio_ccw_ops.c.

Looking at vfio_ccw_mdev_remove() and vfio_ccw_mdev_release_dev(), the
driver tears down the device and the VFIO core frees the private
structure without explicitly cancelling asynchronous background works
(io_work and crw_work).

While flush_workqueue() is called in vfio_ccw_sch_quiesce(), this only
flushes currently queued work and does not prevent concurrent queuing.

For instance, if vfio_ccw_chp_event() reads the private structure and
queues private->crw_work right before or during teardown, the work item
can remain on vfio_ccw_work_q and execute after private has been freed.

Does this teardown path need a cancel_work_sync() for these work items
to prevent them from accessing freed memory?

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