Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Low] virtio_balloon: inaccurate Fixes trailer commit in commit message --
commit 86ecd6e43dcbe0f92a723582119232b2820f4329 Author: Mingbing Zhai <[email protected]> > virtio_balloon: drain update works on probe error paths > > 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. > > Also reset the device before del_vqs() on these paths, like > remove_common() and the virtio_net/virtio_scsi error unwinds do: > without the reset the queues stay enabled on the device and the next > probe fails in setup_vq() with -ENOENT. > > 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: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=0f659760bac1ecb01812 > Signed-off-by: Mingbing Zhai <[email protected]> [Severity: Low] In 1f34c71afe51 ("virtio: Fix scheduling while atomic in virtio_balloon stats"), stats_request() only set vb->need_stats_update and woke the vballoon kthread, which was created as the final fallible step in virtballoon_probe(), and init_vqs() called BUG() if stats_vq priming failed. The undrained work on the virtballoon_probe() error path was introduced in fad7b7b27b6a ("virtio_balloon: Use a workqueue instead of "vballoon" kthread"), and the early error return in init_vqs() without cleaning up the virtqueues was introduced in 74cf5b169c4c ("virtio-balloon: remove BUG() in init_vqs"). Should the Fixes: tags reference those two commits instead? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
