Re: [PATCH v9 11/15] s390/vfio-ap: File ops called to resume the vfio device migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] fortify: __counted_by() traps on memcpy() of ap_config causing kernel panic
- [Low] build: incompatible pointer type warning for num_queues in vfio_ap_get_config()
--

commit 1f3606fec30cc37e82a55e4d85b768771f43fc2f
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

s390/vfio-ap: File ops called to resume the vfio device migration

This commit implements the write callback function for the file stream
created to restore the state of the vfio-ap device on the destination system.
Userspace reads the source guest's saved vfio-ap device state and writes it
to this file stream. The write callback copies each chunk into a pre-allocated
vfio_ap_config structure and validates it against the destination host's
configuration.

> --- a/drivers/s390/crypto/vfio_ap_migration.c
> +++ b/drivers/s390/crypto/vfio_ap_migration.c

[ ... ]

> @@ -284,7 +340,7 @@ static int vfio_ap_get_config(struct ap_matrix_mdev *matrix_mdev)
>  
>  	lockdep_assert_held(&matrix_dev->mdevs_lock);
>  
> -	ap_config_size = vfio_ap_config_size(matrix_mdev, (int *)&num_queues);
> +	ap_config_size = vfio_ap_config_size(matrix_mdev, &num_queues);

[Severity: Low]
Is it possible this causes a build failure? vfio_ap_config_size() expects
an int pointer for its second argument, but num_queues is declared as an
unsigned int. Passing an unsigned int pointer here introduces an
incompatible pointer type warning which breaks the build under
CONFIG_WERROR.

[ ... ]

> +static ssize_t vfio_ap_resuming_write(struct file *filp, const char __user *buf,
> +				      size_t len, loff_t *pos)
> +{

[ ... ]

> +	ret = validate_resuming_write_parms(filp, len);
> +	if (ret)
> +		goto out_unlock;
> +
> +	ap_config = matrix_mdev->mig_data->resuming_mig_file.ap_config;
> +
> +	memcpy((char *)ap_config + *pos, kbuf, len);

[Severity: High]
Does this memcpy() trigger a fortify panic? The struct vfio_ap_config
utilizes the __counted_by(num_queues) annotation for its flexible array.
During the resuming phase, ap_config is allocated with num_queues initialized
to 0.

Because memcpy() is fortified, it evaluates the maximum size of the
destination object using __builtin_dynamic_object_size(). At the time of
this memcpy(), ap_config->num_queues is still 0, so the compiler evaluates
the destination capacity strictly as the struct header size.

If userspace writes the migration data in a chunk larger than the header
size, the bounds check will fail before num_queues can be overwritten,
triggering a panic and crashing the system.

> +	*pos += len;
> +
> +	/*
> +	 * Check whether we have received all the data.  We know the total
> +	 * expected size once num_queues has arrived, i.e. once *pos is past
> +	 * the fixed header.  Until then keep accumulating.
> +	 */
> +	if (*pos >= offsetofend(struct vfio_ap_config, num_queues)) {

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