The apq_reset_check() worker polls ap_tapq() in a while(true) loop waiting for a queue reset to complete. When ap_tapq() returns AP_RESPONSE_BUSY or AP_RESPONSE_RESET_IN_PROGRESS, apq_status_check() returns -EBUSY and the loop continues after sleeping AP_RESET_MAX_WAIT (20ms). There is no upper bound on how many times the loop iterates, so if the hardware continuously returns a busy response the worker runs indefinitely. This is particularly harmful because several callers of vfio_ap_mdev_reset_queues() and vfio_ap_mdev_reset_qlist() call flush_work() on each queue's reset_work while holding one or more of the global matrix_dev locks (guests_lock, mdevs_lock) or the KVM lock. An indefinitely spinning worker permanently blocks all of those locks, hanging mdev removal, KVM guest teardown, and the VFIO_DEVICE_RESET ioctl path. Fix this by introducing AP_RESET_MAX_WAIT (2000ms) and breaking out of the poll loop when elapsed time reaches that threshold. On timeout the final busy status is written back to q->reset_status so that callers inspecting reset_status.response_code after flush_work() see a non-zero value and can return an appropriate error. The AQIC resources associated with this queue - the pinned page containing the NIB and the registered guest ISC - cannot be freed in this case. The NIB is the active DMA target for AP interrupt delivery until the reset completes; freeing the pinned page while the hardware may still write to it would result in a use-after-free kernel crash. If the reset eventually completes, interrupts will be terminated, but the pinned NIB page and ISC registration will be leaked. This is preferable to either a use-after-free or waiting indefinitely. Fixes: dd174833e44e ("s390/vfio-ap: remove upper limit on wait for queue reset to complete") Cc: stable@vger.kernel.org Signed-off-by: Anthony Krowiak --- drivers/s390/crypto/vfio_ap_ops.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 4c32fd6eaaa6..a9a33f4949a0 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -31,6 +31,7 @@ #define AP_QUEUE_IN_USE "in use" #define AP_RESET_INTERVAL 20 /* Reset sleep interval (20ms) */ +#define AP_RESET_MAX_WAIT 2000 /* Maximum wait for reset (2000ms) */ static int vfio_ap_mdev_reset_queues(struct ap_matrix_mdev *matrix_mdev); static int vfio_ap_mdev_reset_qlist(struct list_head *qlist); @@ -2016,8 +2017,32 @@ static void apq_reset_check(struct work_struct *reset_work) elapsed += AP_RESET_INTERVAL; status = ap_tapq(q->apqn, NULL); ret = apq_status_check(q->apqn, &status); - if (ret == -EIO) + if (ret == -EIO) { + memcpy(&q->reset_status, &status, sizeof(status)); return; + } + if (elapsed >= AP_RESET_MAX_WAIT) { + /* + * Timed out waiting for reset to complete. + * + * The AQIC resources associated with this queue - the pinned page + * containing the NIB and the registered guest ISC - cannot be freed + * here. The NIB is the active DMA target for AP interrupt delivery + * until the reset completes; freeing the pinned page while the + * hardware may still write to it would result in a use-after-free + * kernel crash. + * + * If the reset eventually completes, interrupts will be terminated + * and the pinned NIB page and ISC registration will be leaked. This + * is preferable to either a use-after-free or waiting indefinitely: + * the caller of apq_reset_check() holds mdevs_lock while flush_work() + * blocks holds the matrix_dev->mdevs_lock mutex, which + * serializes access to all mdev objects system-wide, so blocking + * here would stall all other guests using AP queues. + */ + memcpy(&q->reset_status, &status, sizeof(status)); + return; + } if (ret == -EBUSY) { pr_notice_ratelimited(WAIT_MSG, elapsed, AP_QID_CARD(q->apqn), -- 2.53.0