Re: [PATCH v3 6/8] s390/vfio_ccw: ensure index for read/write regions are within range

[Matthew Rosato]

On 7/23/26 1:47 PM, Eric Farman wrote:
> 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.
> 
> Fixes: db8e5d17ac03 ("vfio-ccw: add capabilities chain")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Cornelia Huck <cohuck@xxxxxxxxxx>
> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
>  drivers/s390/cio/vfio_ccw_async.c | 4 ++++
>  drivers/s390/cio/vfio_ccw_chp.c   | 4 ++++
>  drivers/s390/cio/vfio_ccw_ops.c   | 7 +++----
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_async.c b/drivers/s390/cio/vfio_ccw_async.c
> index 420d89ba7f83..f213f62f6641 100644
> --- a/drivers/s390/cio/vfio_ccw_async.c
> +++ b/drivers/s390/cio/vfio_ccw_async.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include <linux/vfio.h>
> +#include <linux/nospec.h>
>  
>  #include "vfio_ccw_private.h"
>  
> @@ -24,6 +25,8 @@ static ssize_t vfio_ccw_async_region_read(struct vfio_ccw_private *private,
>  		return -EINVAL;
>  
>  	mutex_lock(&private->io_mutex);
> +
> +	i = array_index_nospec(i, private->num_regions);
>  	region = private->region[i].data;
>  	if (copy_to_user(buf, (void *)region + pos, count))

Couldn't using array_index_nospec cause us to accidentally operate on
region[0] if ppos were ever out of range?

By comparison, vfio_pci_ioctl_get_region_info() does an explicit check
first, so to do something similar here and elsewhere in this patch:

if (i >= private->num_regions)
   return -EINVAL;
i = array_index_nospec(i, private->num_regions);

(Note that for this case and vfio_ccw_async_region_write my pseudocode
doesn't account for io_mutex)

>  		ret = -EFAULT;
> @@ -48,6 +51,7 @@ static ssize_t vfio_ccw_async_region_write(struct vfio_ccw_private *private,
>  	if (!mutex_trylock(&private->io_mutex))
>  		return -EAGAIN;
>  
> +	i = array_index_nospec(i, private->num_regions);
>  	region = private->region[i].data;
>  	if (copy_from_user((void *)region + pos, buf, count)) {
>  		ret = -EFAULT;
> diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c
> index 38c176cf6295..bf678270c457 100644
> --- a/drivers/s390/cio/vfio_ccw_chp.c
> +++ b/drivers/s390/cio/vfio_ccw_chp.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/slab.h>
> +#include <linux/nospec.h>
>  #include <linux/vfio.h>
>  #include "vfio_ccw_private.h"
>  
> @@ -26,6 +27,8 @@ static ssize_t vfio_ccw_schib_region_read(struct vfio_ccw_private *private,
>  		return -EINVAL;
>  
>  	mutex_lock(&private->io_mutex);
> +
> +	i = array_index_nospec(i, private->num_regions);
>  	region = private->region[i].data;
>  
>  	if (cio_update_schib(sch)) {
> @@ -97,6 +100,7 @@ static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private,
>  		list_del(&crw->next);
>  
>  	mutex_lock(&private->io_mutex);
> +	i = array_index_nospec(i, private->num_regions);
>  	region = private->region[i].data;
>  
>  	if (crw)
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index 45ec722d25ea..032a1cdf4df7 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -243,6 +243,7 @@ static ssize_t vfio_ccw_mdev_read(struct vfio_device *vdev,
>  		return vfio_ccw_mdev_read_io_region(private, buf, count, ppos);
>  	default:
>  		index -= VFIO_CCW_NUM_REGIONS;
> +		index = array_index_nospec(index, private->num_regions);
>  		return private->region[index].ops->read(private, buf, count,
>  							ppos);
>  	}
> @@ -295,6 +296,7 @@ static ssize_t vfio_ccw_mdev_write(struct vfio_device *vdev,
>  		return vfio_ccw_mdev_write_io_region(private, buf, count, ppos);
>  	default:
>  		index -= VFIO_CCW_NUM_REGIONS;
> +		index = array_index_nospec(index, private->num_regions);
>  		return private->region[index].ops->write(private, buf, count,
>  							 ppos);
>  	}
> @@ -338,11 +340,8 @@ static int vfio_ccw_mdev_ioctl_get_region_info(struct vfio_device *vdev,
>  		    VFIO_CCW_NUM_REGIONS + private->num_regions)
>  			return -EINVAL;
>  
> -		info->index = array_index_nospec(info->index,
> -						 VFIO_CCW_NUM_REGIONS +
> -						 private->num_regions);
> -
>  		i = info->index - VFIO_CCW_NUM_REGIONS;
> +		i = array_index_nospec(i, private->num_regions);
>  
>  		info->offset = VFIO_CCW_INDEX_TO_OFFSET(info->index);
>  		info->size = private->region[i].size;