Re: [PATCH v6 1/1] s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config

On Tue, 18 Aug 2026 15:33:49 -0400
Anthony Krowiak <[email protected]> wrote:

> Three related problems exist in the handling of KVM interrupt and page
> resources when a queue is removed from the host's AP configuration
> while assigned to a mediated device (mdev).
> 
> Problem 1:
> ~~~~~~~~~
> AP_RESPONSE_Q_NOT_AVAIL not handled in vfio_ap_mdev_reset_queue()
> 
> When the AP bus removes a queue device whose adapter or domain has
> been removed from the host's AP configuration,
> vfio_ap_mdev_remove_queue() is called. If the queue is still in the
> host's AP configuration at that point, it calls
> vfio_ap_mdev_reset_queue(), which issues a PQAP(ZAPQ). Since the
> adapter is already gone from the host configuration, ap_zapq() returns
> AP_RESPONSE_Q_NOT_AVAIL (0x01). This response code is not handled in
> vfio_ap_mdev_reset_queue()'s switch statement and falls through to
> the default case, which issues a WARN but does not call
> vfio_ap_free_aqic_resources(). As a result, if IRQ handling was
> enabled for the queue by the guest, the KVM GISC registration and
> the pinned guest page holding the notification indicator byte (NIB)
> are both leaked.
> 
> This is fixed by adding AP_RESPONSE_Q_NOT_AVAIL to the same case as
> AP_RESPONSE_DECONFIGURED and AP_RESPONSE_CHECKSTOPPED in
> vfio_ap_mdev_reset_queue(). Like those response codes, Q_NOT_AVAIL
> indicates the queue is not operational and no further reset attempts
> are possible; the correct action is to free the IRQ resources
> immediately.
> 
> Problem 2:
> ~~~~~~~~~
>  AP_RESPONSE_Q_NOT_AVAIL not handled in apq_status_check()
> 
> In vfio_ap_mdev_reset_queue(), there are four cases that indicate a queue
> reset has not yet completed, in which case apq_reset_check() is queued to
> a work queue to verify completion of the reset operation. This function
> uses the PQAP(TAPQ) function to get the queue's status and calls
> apq_status_check() to verify whether the reset has completed, failed or
> needs to be executed again. As described in Problem #1 above,
> apq_reset_check() does not specifically check for AP_RESPONSE_Q_NOT_AVAIL,
> thereby potentially leaking KVM GISC registration and the pinned guest page
> holding the NIB.
> 
> This is fixed by adding a case statement for AP_RESPONSE_Q_NOT_AVAIL to
> apq_status_check() and returning -ENODEV for that case. The caller,
> apq_reset_check() will then check for this return code and call
> vfio_ap_free_aqic_resources() to prevent the leak.
> 
> Problem 3:
> ~~~~~~~~~
> vfio_ap_free_aqic_resources() leaks saved_isc when kvm is NULL
> 
> vfio_ap_free_aqic_resources() guards the call to
> kvm_s390_gisc_unregister() with:
> 
>     if (q->saved_isc != VFIO_AP_ISC_INVALID &&
>         !WARN_ON(!(q->matrix_mdev && q->matrix_mdev->kvm)))
> 
> If matrix_mdev->kvm is NULL -- which can happen when
> vfio_ap_mdev_unset_kvm() has already run and cleared kvm before a
> subsequent cleanup path reaches this function -- the WARN_ON fires
> and the entire block is skipped. This leaves q->saved_isc set to a
> non-invalid value, creating a potential double-free on any subsequent
> call to this function.
> 
> When kvm is NULL the KVM guest is already torn down, so
> kvm_s390_gisc_unregister() need not and cannot be called; however,
> q->saved_isc must always be cleared. Fix this by separating the
> kvm_s390_gisc_unregister() call from the q->saved_isc reset. The
> WARN_ON now guards only the genuinely impossible case of matrix_mdev
> being NULL. A NULL kvm is handled gracefully by skipping only the
> unregister call, and q->saved_isc = VFIO_AP_ISC_INVALID is set
> unconditionally whenever saved_isc was not already invalid.
> 
> Additionally, add an else clause to the host-config check in
> vfio_ap_mdev_remove_queue() to call vfio_ap_free_aqic_resources()
> directly when the queue is not in the host's AP configuration. This
> serves as a backstop: when the AP bus fires the driver .remove
> callback after an adapter is removed from the host config, the queue
> is by definition no longer addressable, so vfio_ap_mdev_reset_queue()
> would always return Q_NOT_AVAIL. The else clause handles this case
> directly without the unnecessary ap_zapq() call, and ensures cleanup
> occurs even if kvm has already been set to NULL by a prior call to
> vfio_ap_mdev_unset_kvm().
> 
> Fixes: b9bd10c43456d ("s390/vfio-ap: do not reset queue removed from host config")
> Cc: [email protected]
> Signed-off-by: Anthony Krowiak <[email protected]>
> Reviewed-by: Matthew Rosato <[email protected]>

Acked-by: Halil Pasic <[email protected]>

Looks good to me functionally. Style-wise there are things we could talk
about, but we don't have to :)