On Sat, Apr 25, 2026 at 03:08:40PM +0800, physicalmtea wrote: > From: JiaJia <[email protected]> > > 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 <[email protected]>
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 <[email protected]> > --- > 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
