All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Krowiak <akrowiak@linux.ibm.com>
To: linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Cc: jjherne@linux.ibm.com, borntraeger@de.ibm.com,
	mjrosato@linux.ibm.com, pasic@linux.ibm.com, alex@shazbot.org,
	kwankhede@nvidia.com, fiuczy@linux.ibm.com, pbonzini@redhat.com,
	frankja@linux.ibm.com, imbrenda@linux.ibm.com,
	agordeev@linux.ibm.com, hca@linux.ibm.com, gor@linux.ibm.com,
	freude@linux.ibm.com
Subject: [PATCH v8 0/6] s390/vfio-ap: Fix pre-existing bugs in vfio_ap device driver
Date: Fri, 25 Sep 2026 08:45:45 -0400	[thread overview]
Message-ID: <20260925124551.665448-1-akrowiak@linux.ibm.com> (raw)

The sashiko AI reported several pre-existing bugs in the vfio_ap device
driver code while reviewing unrelated patches. This series fixes four
such bugs.

Change log v7 => v8:
~~~~~~~~~~~~~~~~~~~
Patch 1/6: Fix leaks of pinned NIB and registered GISC
* Added return codes to vfio_ap_wait_for_irqclear()
  ~ Returns 0 when the IR bit is clear confirming interrupts
    are disabled
  ~ Returns -ENODEV for response codes AP_RESPONSE_Q_NOT_AVAIL,
    AP_RESPONSE_DECONFIGURED, and AP_RESPONSE_CHECKSTOPPED to
    indicate the queue is not operational
  ~ Returns -ETIMEDOUT if retries are exhausted or TAPQ returns
    AP_RESPONSE_ASSOC_SECRET_NOT_UNIQUE or
    AP_RESPONSE_ASSOC_FAILED indicating the IR bit cannot be
    confirmed clear
  ~ Returns -EIO if TAPQ returns an invalid response code
* Updated vfio_ap_irq_disable() to handle the return code from
  vfio_ap_wait_for_irqclear()
  ~ Frees AQIC resources on return code 0 or -ENODEV
  ~ Intentionally leaks AQIC resources on -ETIMEDOUT or -EIO
    to prevent a wild DMA write that could corrupt host memory
  ~ Returns AP_RESPONSE_OTHERWISE_CHANGED to the guest on
    -ETIMEDOUT if the TAPQ response code is AP_RESPONSE_NORMAL
    or AP_RESPONSE_BUSY, to signal the disable did not complete
* Added handling for previously unhandled AQIC response codes
  ~ AP_RESPONSE_STATE_CHANGE_IN_PROGRESS: retries after 20ms,
    same as RESET_IN_PROGRESS and BUSY
  ~ AP_RESPONSE_INVALID_GISA, AP_RESPONSE_ASSOC_SECRET_NOT_
    UNIQUE, AP_RESPONSE_ASSOC_FAILED: AQIC resources are
    intentionally leaked since the disable was rejected and
    hardware still holds the NIB address
* Updated apq_status_check() to correctly interpret TAPQ status
  ~ AP_RESPONSE_NORMAL: verifies queue_empty, replies_waiting,
    irq_enabled, and async bits before returning 0; returns
    -EBUSY if async zeroization is still in progress
  ~ AP_RESPONSE_DECONFIGURED, AP_RESPONSE_CHECKSTOPPED: returns
    -ENODEV (AQIC resources freed but zeroization unconfirmed)
  ~ AP_RESPONSE_BUSY: returns -EAGAIN to re-issue ZAPQ, no
    longer falls through to ASSOC_SECRET_NOT_UNIQUE case
* Updated apq_reset_check() to handle -ENODEV from
  apq_status_check()
  ~ On -ENODEV, frees AQIC resources and records TAPQ status
    in q->reset_status to mark the queue as non-passable
  ~ On -EIO, intentionally leaks AQIC resources and records
    TAPQ status to mark the queue as non-passable
* Updated vfio_ap_mdev_reset_queue() to handle response codes
  ~ Added AP_RESPONSE_Q_NOT_AVAIL alongside DECONFIGURED and
    CHECKSTOPPED to free AQIC resources for all non-operational
    queue states
  ~ Removed AP_RESPONSE_BUSY (not a valid ZAPQ response code)
* Updated vfio_ap_mdev_remove_queue() to free AQIC resources
  when the queue is not in the host AP configuration via an
  else branch to the test_bit_inv host-config guard
* Updated unmap_iova() to fall back to a bounded queue reset
  (ZAPQ) when vfio_ap_irq_disable() leaks the NIB page, since
  the vfio core requires the page be unpinned before dma_unmap
  returns; after the reset the NIB is unconditionally freed

Patch 3/6: Fix unbounded loop in apq_reset_check()
* apq_reset_check()
  ~ On timeout, q->reset_status.response_code is set to
    AP_RESPONSE_RESET_IN_PROGRESS to signal incomplete reset
    and trigger ZAPQ re-issue on next reset attempt
  ~ AQIC resources (pinned NIB page and GISC registration)
    are intentionally leaked on timeout to prevent a wild
    DMA write that could corrupt or crash the host kernel
  ~ report_aqic_resource_leak() added to emit a ratelimited
    warning when resources are leaked
* Introduced apq_reset_finalize() to update q->reset_status
  with the confirmed TAPQ end-state and free AQIC resources
  ~ Sets response_code to AP_RESPONSE_NORMAL only when
    apq_status_check() returns 0 (zeroization confirmed)
  ~ Leaves non-zero response code intact for -ENODEV so
    _queue_passable() correctly returns false
* Fixed incorrect treatment of AP_RESPONSE_NORMAL from
  PQAP(ZAPQ) as zeroization confirmation; AP_RESPONSE_NORMAL
  only means the ZAPQ was accepted — completion requires
  all four TAPQ status bits to be verified
* Fixed AP_RESPONSE_BUSY handling in apq_status_check()
  ~ Was falling through to ASSOC_SECRET_NOT_UNIQUE; now
    returns -EAGAIN to re-issue the ZAPQ independently
* Elapsed time counter is reset to 0 when ZAPQ is re-issued
  due to -EAGAIN or RESET_IN_PROGRESS or
  STATE_CHANGE_IN_PROGRESS

Patch 5/6: Fix queue state leakage to guest and host
* Restricted _queue_passable() to return true only when
  reset_status.response_code == AP_RESPONSE_NORMAL (0)
  ~ Removed AP_RESPONSE_DECONFIGURED and
    AP_RESPONSE_CHECKSTOPPED as passable states; neither
    confirms zeroization and passing such a queue could
    leak key material from a prior guest or host operation
* Added on_qstate_transition callback to struct ap_driver
  and implemented vfio_ap_on_qstate_transition()
  ~ AP bus invokes the callback during scan when a queue
    transitions between configured/deconfigured or
    checkstopped/not-checkstopped states
  ~ On AP_QUEUE_CONFIG_ON or AP_QUEUE_CHKSTOP_OFF the queue
    is reset and zeroized; if reset succeeds the guest APCB
    is updated to plug the queue into the guest configuration
* Added ap_qstate_transition struct to ap_bus.h carrying the
  queue pointer and new_state enum
* vfio_ap_mdev_probe_queue() now calls
  vfio_ap_mdev_reset_queue() and flush_work() at probe time
  to guarantee a clean queue before assignment to a guest
* Fixed ordering of kfree(q) vs release_update_locks_for_mdev()
  in vfio_ap_mdev_remove_queue() to avoid use-after-free

Patch 6/6: replace guest-reachable WARNs with ratelimited warnings
* Replaced all WARN/WARN_ONCE calls in guest-reachable paths
  with ratelimited warning functions to prevent log flooding
  by a malicious or misbehaving guest
* Introduced five reporting helper functions:
  ~ report_tapq_rc() - invalid/unexpected PQAP(TAPQ) rc;
    used in vfio_ap_wait_for_irqclear() and
    apq_status_check()
  ~ report_irqclear_timeout() - timeout waiting for IR bit
    to clear after PQAP(AQIC) disable
  ~ report_aqic_disable_error() - failed PQAP(AQIC) disable;
    replaces three WARN_ONCE calls for non-operational queue,
    rejected disable, and retry exhaustion cases
  ~ report_zapq_rc() - invalid PQAP(ZAPQ) rc in
    vfio_ap_mdev_reset_queue()
  ~ report_gisc_unregister_failure() - failure to unregister
    guest ISC when matrix_mdev or kvm context is NULL
* Changed signatures of vfio_ap_wait_for_irqclear() and
  apq_status_check() to accept struct vfio_ap_queue * instead
  of apqn so mdev context is available for reporting
* Augmented VFIO_AP_DBF_WARN() sites in vfio_ap_irq_enable()
  and handle_pqap() with companion dev_warn_ratelimited() or
  pr_warn_ratelimited() calls for dmesg visibility 

Anthony Krowiak (6):
  s390/vfio-ap: Fix leaks of pinned NIB and registered GISC
  s390/vfio-ap: Fix failure to release IRQ notification eventfd contexts
  s390/vfio-ap: Fix unbounded loop in apq_reset_check()
  s390/vfio-ap: Use AP_DOMAINS for adm_add bitmap size in
    vfio_ap_mdev_cfg_add()
  s390/vfio-ap: fix queue state leakage to guest and host
  s390/vfio-ap: replace guest-reachable WARNs with ratelimited warnings

 drivers/s390/crypto/ap_bus.c          |  51 ++
 drivers/s390/crypto/ap_bus.h          |  29 +
 drivers/s390/crypto/vfio_ap_drv.c     |   1 +
 drivers/s390/crypto/vfio_ap_ops.c     | 739 ++++++++++++++++++++++----
 drivers/s390/crypto/vfio_ap_private.h |   2 +
 5 files changed, 704 insertions(+), 118 deletions(-)

-- 
2.53.0


             reply	other threads:[~2026-09-25 12:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25 12:45 Anthony Krowiak [this message]
2026-09-25 12:45 ` [PATCH v8 1/6] s390/vfio-ap: Fix leaks of pinned NIB and registered GISC Anthony Krowiak
2026-09-25 12:57   ` sashiko-bot
2026-09-29  9:36     ` Anthony Krowiak
2026-09-25 12:45 ` [PATCH v8 2/6] s390/vfio-ap: Fix failure to release IRQ notification eventfd contexts Anthony Krowiak
2026-09-25 12:53   ` sashiko-bot
2026-09-25 12:45 ` [PATCH v8 3/6] s390/vfio-ap: Fix unbounded loop in apq_reset_check() Anthony Krowiak
2026-09-25 13:00   ` sashiko-bot
2026-09-29 11:11     ` Anthony Krowiak
2026-09-25 12:45 ` [PATCH v8 4/6] s390/vfio-ap: Use AP_DOMAINS for adm_add bitmap size in vfio_ap_mdev_cfg_add() Anthony Krowiak
2026-09-25 12:51   ` sashiko-bot
2026-09-25 12:45 ` [PATCH v8 5/6] s390/vfio-ap: fix queue state leakage to guest and host Anthony Krowiak
2026-09-25 13:03   ` sashiko-bot
2026-09-29 11:41     ` Anthony Krowiak
2026-09-25 12:45 ` [PATCH v8 6/6] s390/vfio-ap: replace guest-reachable WARNs with ratelimited warnings Anthony Krowiak
2026-09-25 12:59   ` sashiko-bot
2026-09-25 15:55     ` Anthony Krowiak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260925124551.665448-1-akrowiak@linux.ibm.com \
    --to=akrowiak@linux.ibm.com \
    --cc=agordeev@linux.ibm.com \
    --cc=alex@shazbot.org \
    --cc=borntraeger@de.ibm.com \
    --cc=fiuczy@linux.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=freude@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=jjherne@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mjrosato@linux.ibm.com \
    --cc=pasic@linux.ibm.com \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.