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. The same hole exists when init_vqs() itself fails after virtio_find_vqs() succeeded: the stats priming can fail with the queues up and the callback armed, yet probe jumps straight to out_free_vb and leaks the virtqueues while the queued work still references vb. Unwind inside init_vqs() in that case, stopping the works before the queues are deleted. The drained work takes vb->wakeup_lock in process_wakeup_event() and finish_wakeup_event(), so this depends on "virtio_balloon: initialize wakeup_lock before virtqueues are set up", which moves that spin_lock_init() to the top of probe. Fixes: 1f34c71afe51 ("virtio: Fix scheduling while atomic in virtio_balloon stats") Reported-by: syzbot+0f659760bac1ecb01812@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=0f659760bac1ecb01812 Signed-off-by: Mingbing Zhai --- Changes in v2 (from sashiko review): - unwind inside init_vqs() when the stats priming fails after virtio_find_vqs() succeeded, instead of leaking the virtqueues on the out_free_vb path with the callback still armed. - spell out the dependency on the wakeup_lock initialization patch: the drained work takes that lock. drivers/virtio/virtio_balloon.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 8c1b47ec1..dd2056882 100644 --- 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; @@ -633,6 +644,12 @@ static int init_vqs(struct virtio_balloon *vb) if (err) { dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n", __func__); + /* + * The virtqueues are up and the stats callback is + * armed: stop the works and unwind the queues. + */ + virtballoon_stop_updates(vb); + vb->vdev->config->del_vqs(vb->vdev); return err; } virtqueue_kick(vb->stats_vq); @@ -1075,6 +1092,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); @@ -1115,11 +1138,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.43.0