Re: [PATCH v3 04/15] s390/vfio-ap: Reset migration state in VFIO_DEVICE_RESET ioctl handler

[sashiko-bot@kernel.org]

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

New issues:
- [Low] Typo in the function comment block for `vfio_ap_reset_migration_state`.

Pre-existing issues:
- [High] Preexisting memory leak of `ap_config` in `vfio_ap_release_migration_data`.
--

commit 814157f788ce7725968457dca0dde1e8bb23d18c
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

s390/vfio-ap: Reset migration state in VFIO_DEVICE_RESET ioctl handler

This patch adds a handler to reset the vfio-ap migration state during a
VFIO_DEVICE_RESET ioctl. It releases migration files and frees the storage
allocated for maintaining the migration state to return it to RUNNING.

> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
> index 5b6d48ccf332c..c7c71980bdbb8 100644
> --- a/drivers/s390/crypto/vfio_ap_migration.c
> +++ b/drivers/s390/crypto/vfio_ap_migration.c
> @@ -129,3 +129,23 @@ void vfio_ap_release_migration_data(struct ap_matrix_mdev *matrix_mdev)
>  	kfree(matrix_mdev->mig_data);
>  	matrix_mdev->mig_data = NULL;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does this function leak the
dynamically allocated ap_config pointer stored within mig_data?

If the VFIO device file descriptor is closed before the migration file
descriptor, vfio_ap_mdev_close_device() calls this function which frees
matrix_mdev->mig_data without first freeing
mig_data->resuming_mig_state.ap_config.

When the migration file descriptor is subsequently closed, its release
callback vfio_ap_release_mig_file() checks matrix_mdev->mig_data. Since it
is now NULL, it returns -ENODEV and skips cleanup, which seems to permanently
leak the ap_config memory. Could this be triggered repeatedly by userspace?

> +
> +/**
> + * vfio_ap_reset_migration_state - Reset the vfio-ap migration state
> + *
> + * @matrix_mdev: pointer to the object maintaining the vfio-ap device state
> + *
> + * Called during VFIO_DEVICE_RESET to clean up any active migration
> + * stte and reset the device to RUNNING state as required by the VFIO

[Severity: Low]
This isn't a bug, but there is a typo in the comment block. Did you mean
"state" instead of "stte"?

> + * migration specification.
> + */

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