init_vqs() arms the stats virtqueue callback, so an update work can
already be queued on the system freezable workqueue while
virtballoon_probe() is still running. If a later probe step fails, the
error unwind deletes the virtqueues and frees vb without stopping the
works, and the queued work runs after the balloon struct is gone:
BUG: KFENCE: use-after-free read in virtqueue_get_buf_ctx
update_balloon_stats_func+0x121/0x300
Workqueue: events_freezable update_balloon_stats_func
remove() already handles this via virtballoon_quiesce(). Factor the
stopping of the update works out of it and run it on the probe error
path as well, before the queues and vb go away.
Fixes: 1f34c71afe51 ("virtio: Fix scheduling while atomic in virtio_balloon
stats")
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=0f659760bac1ecb01812
Signed-off-by: Mingbing Zhai <[email protected]>
---
drivers/virtio/virtio_balloon.c | 17 ++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -554,6 +554,17 @@ static void update_balloon_stats_func(struct work_struct
*work)
finish_wakeup_event(vb);
}
+/* Stop queueing of and drain any pending balloon update works. */
+static void virtballoon_stop_updates(struct virtio_balloon *vb)
+{
+ spin_lock_irq(&vb->stop_update_lock);
+ vb->stop_update = true;
+ spin_unlock_irq(&vb->stop_update_lock);
+
+ cancel_work_sync(&vb->update_balloon_size_work);
+ cancel_work_sync(&vb->update_balloon_stats_work);
+}
+
static void update_balloon_size_func(struct work_struct *work)
{
struct virtio_balloon *vb;
@@ -1076,6 +1087,12 @@ static int virtballoon_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
destroy_workqueue(vb->balloon_wq);
out_del_vqs:
+ /*
+ * init_vqs() arms the stats virtqueue callback and primes the
+ * queue, so an update work may already be pending: stop queueing
+ * and drain it before the virtqueues and vb go away.
+ */
+ virtballoon_stop_updates(vb);
vdev->config->del_vqs(vdev);
out_free_vb:
mutex_destroy(&vb->balloon_lock);
@@ -1116,11 +1133,7 @@ static void virtballoon_quiesce(struct virtio_balloon
*vb)
if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
virtio_balloon_unregister_shrinker(vb);
- spin_lock_irq(&vb->stop_update_lock);
- vb->stop_update = true;
- spin_unlock_irq(&vb->stop_update_lock);
- cancel_work_sync(&vb->update_balloon_size_work);
- cancel_work_sync(&vb->update_balloon_stats_work);
+ virtballoon_stop_updates(vb);
if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
cancel_work_sync(&vb->report_free_page_work);
--
2.47.0
