[PATCH 3/4] s390/vfio-ap: Fix unbounded loop in apq_reset_check()
Anthony Krowiak <[email protected]>
| Newsgroups | org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-s390,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
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_INTERVAL (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_TIMEOUT (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. vfio_ap_free_aqic_resources() is called before returning
to release any KVM ISC registration and pinned NIB page,
consistent with all other early-exit paths in the function.
Fixes: dd174833e44e ("s390/vfio-ap: remove upper limit on wait for queue reset to complete")
Cc: [email protected]
Signed-off-by: Anthony Krowiak <[email protected]>
---
drivers/s390/crypto/vfio_ap_ops.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 6e4569d6b975..c7eebbd0ed40 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);
@@ -1973,6 +1974,12 @@ static void apq_reset_check(struct work_struct *reset_work)
status.response_code,
status.queue_empty,
status.irq_enabled);
+ if (elapsed >= AP_RESET_MAX_WAIT) {
+ /* Timed out waiting for reset to complete */
+ memcpy(&q->reset_status, &status, sizeof(status));
+ vfio_ap_free_aqic_resources(q);
+ return;
+ }
} else {
if (q->reset_status.response_code == AP_RESPONSE_RESET_IN_PROGRESS ||
q->reset_status.response_code == AP_RESPONSE_BUSY ||
--
2.53.0