Re: [PATCH v7 05/15] s390/vfio-ap: Callback to get/set vfio device mig state during guest migration

[Anthony Krowiak <akrowiak@linux.ibm.com>]

On 8/7/26 6:45 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] vfio_ap_set_state falsely reports success when attempting to transition out of the VFIO_DEVICE_STATE_ERROR state.
> --
>
> --- Patch [5]: [PATCH v7 05/15] s390/vfio-ap: Callback to get/set vfio device mig state during guest migration ---
> commit 4d4bb0a626dab0e0e55ea92759151b23e4fe2cdf
> Author: Anthony Krowiak <akrowiak@linux.ibm.com>
>
> s390/vfio-ap: Callback to get/set vfio device mig state during guest migration
>
> This commit implements two callbacks for the vfio-ap device related to its
> current migration state. It provides the mandatory callbacks to get and set
> the migration state during live migration of guests with pass-through access.
>
>> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
>> index 1613813124b6f..af740b177c97c 100644
>> --- a/drivers/s390/crypto/vfio_ap_migration.c
>> +++ b/drivers/s390/crypto/vfio_ap_migration.c
> [ ... ]
>
>> +static struct file *vfio_ap_set_state(struct vfio_device *vdev,
>> +				      enum vfio_device_mig_state  new_state)
>> +{
>> +	int ret;
>> +	struct file *filp = NULL;
>> +	struct ap_matrix_mdev *matrix_mdev;
> [ ... ]
>
>> +	while (mig_data->mig_state != VFIO_DEVICE_STATE_ERROR &&
>> +	       mig_data->mig_state != new_state) {
>> +		ret = vfio_mig_get_next_state(vdev, mig_data->mig_state,
>> +					      new_state, &next_state);
>> +		if (ret) {
>> +			filp = ERR_PTR(ret);
>> +			break;
>> +		}
>> +
>> +		filp = vfio_ap_transition_to_state(matrix_mdev, next_state);
>> +		if (IS_ERR(filp))
>> +			break;
>> +
>> +		mig_data->mig_state = next_state;
>> +
>> +		if (WARN_ON(filp && new_state != next_state)) {
>> +			fput(filp);
>> +			filp = ERR_PTR(-EINVAL);
>> +			break;
>> +		}
>> +	}
>> +
>> +	mutex_unlock(&matrix_dev->mdevs_lock);
>> +
>> +	return filp;
> [Severity: High]
> If the device is already in VFIO_DEVICE_STATE_ERROR when vfio_ap_set_state()
> is called, the while loop condition evaluates to false and is skipped entirely.
>
> Since filp is initialized to NULL, does returning NULL here incorrectly
> signal a successful state transition to the VFIO core? Should an attempt to
> transition out of the error state return an explicit error pointer (such as
> ERR_PTR(-EIO)) instead to properly reject the transition?

An ERROR check after the transition loop in vfio_ap_set_state() will be
added: filp will be set to ERR_PTR(-EIO).

>
>> +}