[PATCH] virtio: rtc: tear down old virtqueues before restore

[PATCH] virtio: rtc: tear down old virtqueues before restore

[JiaJia]

virtio_device_restore() resets the device and restores the negotiated
features before calling ->restore(). viortc_freeze() intentionally
leaves the existing virtqueues in place so the alarm queue can still
wake the system, but viortc_restore() immediately calls
viortc_init_vqs() without first deleting those old queues.

If virtqueue reinitialization fails on virtio-pci, the transport error
path runs vp_del_vqs() against a new vp_dev->vqs array while vdev->vqs
still holds the old virtqueues. vp_del_vqs() then looks up queue state
through that new array and can dereference a NULL info pointer in
vp_del_vq(), crashing the guest kernel during restore.

Delete any old virtqueues before rebuilding them, and clear the cached
queue pointers so later message requests fail cleanly if restore does
not complete.

Signed-off-by: JiaJia <physicalmtea@gmail.com>
---
Runtime evidence from a standard hibernation test_resume run on a guest
with virtio-rtc-pci (no fault injection):

  KASAN null-ptr-deref report in vp_del_vq.isra.0+0x70/0xd0

  Call trace:
    vp_del_vq.isra.0+0x70/0xd0
    vp_del_vqs+0xec/0x358
    vp_find_vqs_msix+0x24c/0x6a8
    vp_find_vqs+0x88/0x360
    vp_modern_find_vqs+0x20/0x90
    viortc_init_vqs+0xd0/0x128
    viortc_restore+0x28/0xb8
    virtio_device_restore_priv+0x184/0x288
    virtio_pci_restore+0x44/0x58

 drivers/virtio/virtio_rtc_driver.c | 38 ++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_rtc_driver.c b/drivers/virtio/virtio_rtc_driver.c
index a57d5e06e..c1adde27d 100644
--- a/drivers/virtio/virtio_rtc_driver.c
+++ b/drivers/virtio/virtio_rtc_driver.c
@@ -427,6 +427,15 @@ static int viortc_msg_xfer(struct viortc_vq *vq, struct viortc_msg *msg,
 	sg_init_one(out_sg, msg->req, msg->req_size);
 	sg_init_one(in_sg, msg->resp, msg->resp_cap);
 
+	if (!vq->vq) {
+		/*
+		 * Keep runtime interfaces in a safe failed state if restore
+		 * teardown removed the virtqueues.
+		 */
+		viortc_msg_release(msg);
+		return -ENODEV;
+	}
+
 	spin_lock_irqsave(&vq->lock, flags);
 
 	ret = virtqueue_add_sgs(vq->vq, sgs, 1, 1, msg, GFP_ATOMIC);
@@ -478,6 +487,17 @@ static int viortc_msg_xfer(struct viortc_vq *vq, struct viortc_msg *msg,
 	return 0;
 }
 
+static void viortc_del_vqs(struct viortc_dev *viortc)
+{
+	struct virtio_device *vdev = viortc->vdev;
+	unsigned int i;
+
+	vdev->config->del_vqs(vdev);
+
+	for (i = 0; i < ARRAY_SIZE(viortc->vqs); i++)
+		viortc->vqs[i].vq = NULL;
+}
+
 /*
  * common message handle macros for messages of different types
  */
@@ -1316,7 +1336,7 @@ static int viortc_probe(struct virtio_device *vdev)
 
 err_reset_vdev:
 	virtio_reset_device(vdev);
-	vdev->config->del_vqs(vdev);
+	viortc_del_vqs(viortc);
 
 	return ret;
 }
@@ -1332,7 +1352,7 @@ static void viortc_remove(struct virtio_device *vdev)
 	viortc_clocks_deinit(viortc);
 
 	virtio_reset_device(vdev);
-	vdev->config->del_vqs(vdev);
+	viortc_del_vqs(viortc);
 }
 
 static int viortc_freeze(struct virtio_device *dev)
@@ -1353,9 +1373,11 @@ static int viortc_restore(struct virtio_device *dev)
 	bool notify = false;
 	int ret;
 
+	viortc_del_vqs(viortc);
+
 	ret = viortc_init_vqs(viortc);
 	if (ret)
-		return ret;
+		goto err_del_vqs;
 
 	alarm_viortc_vq = &viortc->vqs[VIORTC_ALARMQ];
 	alarm_vq = alarm_viortc_vq->vq;
@@ -1364,16 +1386,22 @@ static int viortc_restore(struct virtio_device *dev)
 		ret = viortc_populate_vq(viortc, alarm_viortc_vq,
 					 VIORTC_ALARMQ_BUF_CAP, false);
 		if (ret)
-			return ret;
+			goto err_del_vqs;
 
 		notify = virtqueue_kick_prepare(alarm_vq);
 	}
 
 	virtio_device_ready(dev);
 
-	if (notify && !virtqueue_notify(alarm_vq))
+	if (notify && !virtqueue_notify(alarm_vq)) {
 		ret = -EIO;
+		goto err_del_vqs;
+	}
+
+	return ret;
 
+err_del_vqs:
+	viortc_del_vqs(viortc);
 	return ret;
 }
 
-- 
2.34.1