* [PATCH] virtio: rtc: tear down old virtqueues before restore
@ 2026-04-17 9:08 JiaJia
2026-04-23 18:02 ` Peter Hilber
0 siblings, 1 reply; 8+ messages in thread
From: JiaJia @ 2026-04-17 9:08 UTC (permalink / raw)
To: Peter Hilber, Michael S. Tsirkin, Jason Wang
Cc: Xuan Zhuo, Eugenio Pérez, virtualization, linux-kernel,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] virtio: rtc: tear down old virtqueues before restore
2026-04-17 9:08 [PATCH] virtio: rtc: tear down old virtqueues before restore JiaJia
@ 2026-04-23 18:02 ` Peter Hilber
2026-04-24 3:14 ` [PATCH v2] " JiaJia
0 siblings, 1 reply; 8+ messages in thread
From: Peter Hilber @ 2026-04-23 18:02 UTC (permalink / raw)
To: JiaJia
Cc: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
virtualization, linux-kernel
On Fri, Apr 17, 2026 at 05:08:31PM +0800, JiaJia wrote:
> 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>
Thanks for identifying the bug and providing a fix! I had a look at the
bugfix and I added some comments below on how I think the fix could be
improved.
> ---
> 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;
> +}
> +
If keeping this (see my comment at the bottom), it should go further
down, e.g. above viortc_probe().
> /*
> * common message handle macros for messages of different types
> */
> @@ -1316,7 +1336,7 @@ static int viortc_probe(struct virtio_device *vdev)
In order to delete virtqueues consistently everywhere, I think it makes
sense to also "goto err_reset_vdev;" if viortc_init_vqs() fails in
viortc_probe().
>
> 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);
This looks racy to me, since we can end up here after
virtio_device_ready(), when viortc_cb_alarmq() could run. But after
virtio_device_ready(), deleting the virtqueues does not seem to be
required. So directly returning an error after virtio_device_ready()
should work.
Alternatively, I'd suggest calling the logic from viortc_remove()
instead of just viortc_del_vqs() (factored out as __viortc_remove()).
This would have the benefit of using the same logic to stop the device
in both cases.
This should also make the viortc_del_vqs() wrapper and vq->vq check
unnecessary due to viortc_clocks_deinit() having been called.
The tradeoff would be that self-healing on another restore would not be
possible any more.
Best regards,
Peter
> return ret;
> }
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] virtio: rtc: tear down old virtqueues before restore
2026-04-23 18:02 ` Peter Hilber
@ 2026-04-24 3:14 ` JiaJia
2026-04-24 13:54 ` Peter Hilber
0 siblings, 1 reply; 8+ messages in thread
From: JiaJia @ 2026-04-24 3:14 UTC (permalink / raw)
To: Peter Hilber
Cc: mst, jasowang, xuanzhuo, eperezma, virtualization, linux-kernel,
physicalmtea
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 can run vp_del_vqs() against a newly allocated vp_dev->vqs array
while vdev->vqs still contains the old virtqueues. vp_del_vqs() then
looks up queue state through the new array and can dereference a NULL
info pointer in vp_del_vq(), crashing the guest kernel during restore.
Delete the stale virtqueues before rebuilding them. If restore fails
before virtio_device_ready(), reuse the remove path to stop the device.
Once the device is ready, return errors directly instead of deleting the
virtqueues again.
Signed-off-by: JiaJia <physicalmtea@gmail.com>
---
Hi Peter,
thanks for taking a look and for pointing this out. I took another look
at the restore path and agree that deleting the virtqueues from that
error path can race with `viortc_cb_alarmq()` once the device has been
readied.
This v2 follows the direction you suggested. It factors out
`__viortc_remove()`, reuses that path for restore failures before
`virtio_device_ready()`, routes `viortc_init_vqs()` failures in probe
through `err_reset_vdev` for consistent teardown, drops the
`viortc_del_vqs()` helper and the `viortc_msg_xfer()` NULL-vq guard,
and avoids deleting the virtqueues again after `virtio_device_ready()`.
Thanks,
JiaJia
v2:
- reuse the remove path for restore failures before virtio_device_ready()
- route viortc_init_vqs() failures in probe through err_reset_vdev
- drop the viortc_del_vqs() helper and the viortc_msg_xfer() NULL-vq guard
- avoid deleting virtqueues again after virtio_device_ready()
drivers/virtio/virtio_rtc_driver.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/virtio/virtio_rtc_driver.c b/drivers/virtio/virtio_rtc_driver.c
index a57d5e06e..4419735b0 100644
--- a/drivers/virtio/virtio_rtc_driver.c
+++ b/drivers/virtio/virtio_rtc_driver.c
@@ -1257,6 +1257,15 @@ static int viortc_init_vqs(struct viortc_dev *viortc)
return 0;
}
+static void __viortc_remove(struct viortc_dev *viortc)
+{
+ struct virtio_device *vdev = viortc->vdev;
+
+ viortc_clocks_deinit(viortc);
+ virtio_reset_device(vdev);
+ vdev->config->del_vqs(vdev);
+}
+
/**
* viortc_probe() - probe a virtio_rtc virtio device
* @vdev: virtio device
@@ -1282,7 +1291,7 @@ static int viortc_probe(struct virtio_device *vdev)
ret = viortc_init_vqs(viortc);
if (ret)
- return ret;
+ goto err_reset_vdev;
virtio_device_ready(vdev);
@@ -1329,10 +1338,7 @@ static void viortc_remove(struct virtio_device *vdev)
{
struct viortc_dev *viortc = vdev->priv;
- viortc_clocks_deinit(viortc);
-
- virtio_reset_device(vdev);
- vdev->config->del_vqs(vdev);
+ __viortc_remove(viortc);
}
static int viortc_freeze(struct virtio_device *dev)
@@ -1353,9 +1359,11 @@ static int viortc_restore(struct virtio_device *dev)
bool notify = false;
int ret;
+ dev->config->del_vqs(dev);
+
ret = viortc_init_vqs(viortc);
if (ret)
- return ret;
+ goto err_remove;
alarm_viortc_vq = &viortc->vqs[VIORTC_ALARMQ];
alarm_vq = alarm_viortc_vq->vq;
@@ -1364,7 +1372,7 @@ 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_remove;
notify = virtqueue_kick_prepare(alarm_vq);
}
@@ -1372,8 +1380,12 @@ static int viortc_restore(struct virtio_device *dev)
virtio_device_ready(dev);
if (notify && !virtqueue_notify(alarm_vq))
- ret = -EIO;
+ return -EIO;
+
+ return 0;
+err_remove:
+ __viortc_remove(viortc);
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] virtio: rtc: tear down old virtqueues before restore
2026-04-24 3:14 ` [PATCH v2] " JiaJia
@ 2026-04-24 13:54 ` Peter Hilber
2026-04-25 7:08 ` [PATCH v3] " physicalmtea
0 siblings, 1 reply; 8+ messages in thread
From: Peter Hilber @ 2026-04-24 13:54 UTC (permalink / raw)
To: JiaJia; +Cc: mst, jasowang, xuanzhuo, eperezma, virtualization, linux-kernel
On Fri, Apr 24, 2026 at 11:14:53AM +0800, JiaJia wrote:
> 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 can run vp_del_vqs() against a newly allocated vp_dev->vqs array
> while vdev->vqs still contains the old virtqueues. vp_del_vqs() then
> looks up queue state through the new array and can dereference a NULL
> info pointer in vp_del_vq(), crashing the guest kernel during restore.
>
> Delete the stale virtqueues before rebuilding them. If restore fails
> before virtio_device_ready(), reuse the remove path to stop the device.
> Once the device is ready, return errors directly instead of deleting the
> virtqueues again.
>
> Signed-off-by: JiaJia <physicalmtea@gmail.com>
Reviewed-by: Peter Hilber <peter.hilber@oss.qualcomm.com>
What should still be added is the Fixes tag:
Fixes: 0623c7592768 ("virtio_rtc: Add module and driver core")
Maybe the commit message could also mention that this can happen during
a non-faulty reinitialization, when one of the vp_find_vqs_msix()
attempts is unsuccessful before a later attempt would succeed (as far as
I understand).
Thanks!
Peter
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v3] virtio: rtc: tear down old virtqueues before restore
2026-04-24 13:54 ` Peter Hilber
@ 2026-04-25 7:08 ` physicalmtea
2026-04-25 15:10 ` Michael S. Tsirkin
0 siblings, 1 reply; 8+ messages in thread
From: physicalmtea @ 2026-04-25 7:08 UTC (permalink / raw)
To: Peter Hilber
Cc: mst, jasowang, xuanzhuo, eperezma, virtualization, linux-kernel,
JiaJia
From: JiaJia <physicalmtea@gmail.com>
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 can run vp_del_vqs() against a newly allocated vp_dev->vqs array
while vdev->vqs still contains the old virtqueues. vp_del_vqs() then
looks up queue state through the new array and can dereference a NULL
info pointer in vp_del_vq(), crashing the guest kernel during restore.
This can also happen during a non-faulty reinitialization, when one of
the vp_find_vqs_msix() attempts is unsuccessful before a later attempt
would succeed.
Delete the stale virtqueues before rebuilding them. If restore fails
before virtio_device_ready(), reuse the remove path to stop the device.
Once the device is ready, return errors directly instead of deleting the
virtqueues again.
Fixes: 0623c7592768 ("virtio_rtc: Add module and driver core")
Signed-off-by: JiaJia <physicalmtea@gmail.com>
Reviewed-by: Peter Hilber <peter.hilber@oss.qualcomm.com>
---
v3:
- Add Fixes tag.
- Add Peter's Reviewed-by tag.
- Clarify commit message regarding reinitialization scenarios.
Apologies for missing the Fixes tag, and thanks for catching that!
I've also updated the commit message to include the scenario you
mentioned.
drivers/virtio/virtio_rtc_driver.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/virtio/virtio_rtc_driver.c b/drivers/virtio/virtio_rtc_driver.c
index a57d5e06e..4419735b0 100644
--- a/drivers/virtio/virtio_rtc_driver.c
+++ b/drivers/virtio/virtio_rtc_driver.c
@@ -1257,6 +1257,15 @@ static int viortc_init_vqs(struct viortc_dev *viortc)
return 0;
}
+static void __viortc_remove(struct viortc_dev *viortc)
+{
+ struct virtio_device *vdev = viortc->vdev;
+
+ viortc_clocks_deinit(viortc);
+ virtio_reset_device(vdev);
+ vdev->config->del_vqs(vdev);
+}
+
/**
* viortc_probe() - probe a virtio_rtc virtio device
* @vdev: virtio device
@@ -1282,7 +1291,7 @@ static int viortc_probe(struct virtio_device *vdev)
ret = viortc_init_vqs(viortc);
if (ret)
- return ret;
+ goto err_reset_vdev;
virtio_device_ready(vdev);
@@ -1329,10 +1338,7 @@ static void viortc_remove(struct virtio_device *vdev)
{
struct viortc_dev *viortc = vdev->priv;
- viortc_clocks_deinit(viortc);
-
- virtio_reset_device(vdev);
- vdev->config->del_vqs(vdev);
+ __viortc_remove(viortc);
}
static int viortc_freeze(struct virtio_device *dev)
@@ -1353,9 +1359,11 @@ static int viortc_restore(struct virtio_device *dev)
bool notify = false;
int ret;
+ dev->config->del_vqs(dev);
+
ret = viortc_init_vqs(viortc);
if (ret)
- return ret;
+ goto err_remove;
alarm_viortc_vq = &viortc->vqs[VIORTC_ALARMQ];
alarm_vq = alarm_viortc_vq->vq;
@@ -1364,7 +1372,7 @@ 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_remove;
notify = virtqueue_kick_prepare(alarm_vq);
}
@@ -1372,8 +1380,12 @@ static int viortc_restore(struct virtio_device *dev)
virtio_device_ready(dev);
if (notify && !virtqueue_notify(alarm_vq))
- ret = -EIO;
+ return -EIO;
+
+ return 0;
+err_remove:
+ __viortc_remove(viortc);
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] virtio: rtc: tear down old virtqueues before restore
2026-04-25 7:08 ` [PATCH v3] " physicalmtea
@ 2026-04-25 15:10 ` Michael S. Tsirkin
2026-04-25 16:09 ` Jia Jia
0 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2026-04-25 15:10 UTC (permalink / raw)
To: physicalmtea
Cc: Peter Hilber, jasowang, xuanzhuo, eperezma, virtualization,
linux-kernel
On Sat, Apr 25, 2026 at 03:08:40PM +0800, physicalmtea wrote:
> From: JiaJia <physicalmtea@gmail.com>
>
> 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 can run vp_del_vqs() against a newly allocated vp_dev->vqs array
> while vdev->vqs still contains the old virtqueues. vp_del_vqs() then
> looks up queue state through the new array and can dereference a NULL
> info pointer in vp_del_vq(), crashing the guest kernel during restore.
>
> This can also happen during a non-faulty reinitialization, when one of
> the vp_find_vqs_msix() attempts is unsuccessful before a later attempt
> would succeed.
>
> Delete the stale virtqueues before rebuilding them. If restore fails
> before virtio_device_ready(), reuse the remove path to stop the device.
> Once the device is ready, return errors directly instead of deleting the
> virtqueues again.
>
> Fixes: 0623c7592768 ("virtio_rtc: Add module and driver core")
> Signed-off-by: JiaJia <physicalmtea@gmail.com>
Sorry is this just the 1st name or the 1st and the last name?
I think it's preferred to have the full name here.
> Reviewed-by: Peter Hilber <peter.hilber@oss.qualcomm.com>
> ---
> v3:
> - Add Fixes tag.
> - Add Peter's Reviewed-by tag.
> - Clarify commit message regarding reinitialization scenarios.
>
> Apologies for missing the Fixes tag, and thanks for catching that!
> I've also updated the commit message to include the scenario you
> mentioned.
>
> drivers/virtio/virtio_rtc_driver.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/virtio/virtio_rtc_driver.c b/drivers/virtio/virtio_rtc_driver.c
> index a57d5e06e..4419735b0 100644
> --- a/drivers/virtio/virtio_rtc_driver.c
> +++ b/drivers/virtio/virtio_rtc_driver.c
> @@ -1257,6 +1257,15 @@ static int viortc_init_vqs(struct viortc_dev *viortc)
> return 0;
> }
>
> +static void __viortc_remove(struct viortc_dev *viortc)
> +{
> + struct virtio_device *vdev = viortc->vdev;
> +
> + viortc_clocks_deinit(viortc);
> + virtio_reset_device(vdev);
> + vdev->config->del_vqs(vdev);
> +}
> +
> /**
> * viortc_probe() - probe a virtio_rtc virtio device
> * @vdev: virtio device
> @@ -1282,7 +1291,7 @@ static int viortc_probe(struct virtio_device *vdev)
>
> ret = viortc_init_vqs(viortc);
> if (ret)
> - return ret;
> + goto err_reset_vdev;
>
> virtio_device_ready(vdev);
>
> @@ -1329,10 +1338,7 @@ static void viortc_remove(struct virtio_device *vdev)
> {
> struct viortc_dev *viortc = vdev->priv;
>
> - viortc_clocks_deinit(viortc);
> -
> - virtio_reset_device(vdev);
> - vdev->config->del_vqs(vdev);
> + __viortc_remove(viortc);
> }
>
> static int viortc_freeze(struct virtio_device *dev)
> @@ -1353,9 +1359,11 @@ static int viortc_restore(struct virtio_device *dev)
> bool notify = false;
> int ret;
>
> + dev->config->del_vqs(dev);
> +
> ret = viortc_init_vqs(viortc);
> if (ret)
> - return ret;
> + goto err_remove;
>
> alarm_viortc_vq = &viortc->vqs[VIORTC_ALARMQ];
> alarm_vq = alarm_viortc_vq->vq;
> @@ -1364,7 +1372,7 @@ 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_remove;
>
> notify = virtqueue_kick_prepare(alarm_vq);
> }
> @@ -1372,8 +1380,12 @@ static int viortc_restore(struct virtio_device *dev)
> virtio_device_ready(dev);
>
> if (notify && !virtqueue_notify(alarm_vq))
> - ret = -EIO;
> + return -EIO;
> +
> + return 0;
>
> +err_remove:
> + __viortc_remove(viortc);
> return ret;
> }
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] virtio: rtc: tear down old virtqueues before restore
2026-04-25 15:10 ` Michael S. Tsirkin
@ 2026-04-25 16:09 ` Jia Jia
2026-04-25 17:03 ` Michael S. Tsirkin
0 siblings, 1 reply; 8+ messages in thread
From: Jia Jia @ 2026-04-25 16:09 UTC (permalink / raw)
To: Michael S . Tsirkin
Cc: Peter Hilber, jasowang, xuanzhuo, eperezma, virtualization,
linux-kernel
Hi Michael,
JiaJia is my full name. My family name is Jia and my given name is also Jia.
I wrote it as "JiaJia", but "Jia Jia" is probably clearer. I will use "Jia Jia"
in future postings.
Thanks,
Jia Jia
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] virtio: rtc: tear down old virtqueues before restore
2026-04-25 16:09 ` Jia Jia
@ 2026-04-25 17:03 ` Michael S. Tsirkin
0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2026-04-25 17:03 UTC (permalink / raw)
To: Jia Jia
Cc: Peter Hilber, jasowang, xuanzhuo, eperezma, virtualization,
linux-kernel
On Sun, Apr 26, 2026 at 12:09:06AM +0800, Jia Jia wrote:
> Hi Michael,
>
> JiaJia is my full name. My family name is Jia and my given name is also Jia.
>
> I wrote it as "JiaJia", but "Jia Jia" is probably clearer. I will use "Jia Jia"
> in future postings.
>
> Thanks,
> Jia Jia
Beautiful. Yes, clearer, thank you so much.
--
MST
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-04-25 17:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 9:08 [PATCH] virtio: rtc: tear down old virtqueues before restore JiaJia
2026-04-23 18:02 ` Peter Hilber
2026-04-24 3:14 ` [PATCH v2] " JiaJia
2026-04-24 13:54 ` Peter Hilber
2026-04-25 7:08 ` [PATCH v3] " physicalmtea
2026-04-25 15:10 ` Michael S. Tsirkin
2026-04-25 16:09 ` Jia Jia
2026-04-25 17:03 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox