Re: [PATCH v9 07/15] s390/vfio-ap: File ops called to save the vfio device migration state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] s390/vfio-ap: Undefined hardware state is used when TAPQ returns an async error
--

commit 449bfb4f03a8737bf9d26180f1c9b9cb38b82556
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

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

This commit implements the read callback function to save the state of the
vfio-ap device during migration. It copies the guest's AP configuration
information, including queue hardware information, to userspace.

> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
> index bbd007c06a776..cf3f95e2846cb 100644
> --- a/drivers/s390/crypto/vfio_ap_migration.c
> +++ b/drivers/s390/crypto/vfio_ap_migration.c
[ ... ]
> +static int get_hardware_info_for_queue(const char *mdev_name,
> +				       struct ap_tapq_hwinfo *hwinfo,
> +				       unsigned long apqn)
> +{
> +	struct ap_queue_status status;
> +
> +	status = ap_tapq(apqn, hwinfo);
> +
> +	switch (status.response_code) {
> +	case AP_RESPONSE_NORMAL:
[ ... ]
> +	default:
> +		/*
> +		 * Without a pending async error, the tapq info should be
> +		 * available
> +		 */
> +		if (status.async)
> +			return 0;

[Severity: Medium]
When the ap_tapq function returns with an async error (status.async == 1),
both the response code and the hardware information returned in general
register 2 (hwinfo->value) are undefined.

Since we return 0 (success) here, does this cause the caller to save garbage
data into the migration stream?

Looking at the caller:

vfio_ap_store_queue_info()
    ret = get_hardware_info_for_queue(..., &source_hwinfo, ...);
    if (ret)
        return ret;
    ap_config->qinfo[num_queues].data = source_hwinfo.value;

If get_hardware_info_for_queue returns 0 when status.async is true,
source_hwinfo.value could contain uninitialized data that is then stored into
the ap_config and migrated to userspace.

Should this return an error when an async error is pending so that we don't
migrate uninitialized state data to the destination host?

> +
> +		pr_err_ratelimited("vfio_ap_mdev %s:Failed to get hwinfo for queue %02lx.%04lx: TAPQ rc=%d",
> +				   mdev_name, AP_QID_CARD(apqn), AP_QID_QUEUE(apqn),
> +				   status.response_code);
> +		return -EIO;
> +	}
> +}
[ ... ]

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