Re: [GIT PULL v1 06/23] s390/vfio_ccw: Ensure index for read/write regions are within range

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The `private->crw` linked list is accessed and modified concurrently without any synchronization, leading to list corruption.
--

--- Patch [6]: [GIT PULL v1 06/23] s390/vfio_ccw: Ensure index for read/write regions are within range ---
commit e1d18d0b96d197063d1125d864eec65df5351f67
Author: Eric Farman <farman@linux.ibm.com>

    s390/vfio_ccw: Ensure index for read/write regions are within range
    
    The introduction of the capability chain rightly clamped the
    region indexes to the range of the capabilities itself, but
    neglected to do so for the existing read/write regions which
    should also be enforced.

[ ... ]

> diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c
> index 38c176cf62957..f3015132d4b5f 100644
> --- a/drivers/s390/cio/vfio_ccw_chp.c
> +++ b/drivers/s390/cio/vfio_ccw_chp.c

[ ... ]

> @@ -97,6 +105,12 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private,
>  		list_del(&crw->next);
>  

[Severity: Critical]
This is a pre-existing issue, but does this code corrupt the private->crw list?

CRW events are dequeued from the list here without any lock protection, allowing
concurrent read() syscalls to race and corrupt the list.

It looks like events are also appended to the list locklessly by
vfio_ccw_queue_crw():

drivers/s390/cio/vfio_ccw_drv.c:vfio_ccw_queue_crw() {
	...
	list_add_tail(&crw->next, &private->crw);
	queue_work(vfio_ccw_work_q, &private->crw_work);
}

This means a single read() call can race against an incoming channel path event
handled by the asynchronous CSS callback vfio_ccw_chp_event(), resulting in list
corruption.

>  	mutex_lock(&private->io_mutex);
> +	if (i >= private->num_regions) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	i = array_index_nospec(i, private->num_regions);
>  	region = private->region[i].data;

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