Re: [PATCH] s390/vfio-ap: Fix leak of KVM GISC resources

[Anthony Krowiak]

On 8/18/26 1:32 PM, Matthew Rosato wrote:
> On 8/18/26 7:58 AM, Anthony Krowiak wrote:
> > s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config
> >
> > Two 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: 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: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Anthony Krowiak <akrowiak@xxxxxxxxxxxxx>
> > ---
> Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
>
> For archive purposes, this is a continuation of
>
> [PATCH v5 9/9] s390/vfio-ap: Fix memory leak when queue removed from host AP config
>
> https://lore.kernel.org/linux-s390/20260812200240.818004-10-akrowiak@xxxxxxxxxxxxx/

I just noticed that I had a commit editor open for this patch when I got 
back
from an appointment for which I had to run out the door. I added a case for
the AP_RESPONSE_Q_NOT_AVAIL  to apq_reset_check() for the same reason
a case was added to vfio_ap_mdev_reset_queue(). When 
vfio_ap_mdev_reset_queue()
has to wait for an asynch reset to complete, it queues the 
apq_status_check()
function to a work queue. That function calls apq_reset_check() to check 
the
status response code which also did not check for AP_RESPONSE_Q_NOT_AVAIL
and leaked AQIC resources. That patch is forthcoming today.

>