Since c578123e63da ("virtio_balloon: Treat stats requests as wakeup
events") the stats_request() virtqueue callback takes vb->wakeup_lock
via start_wakeup_event(). init_vqs() registers that callback and even
primes and kicks the stats queue, so the lock is reachable from irq
context as soon as init_vqs() returns, long before the driver sets
DRIVER_OK: vring_interrupt() dispatches to the callback without any
DRIVER_OK check unless CONFIG_VIRTIO_HARDEN_NOTIFICATION is set, and
that option depends on BROKEN. syzbot hit it from a threaded irq:
INFO: trying to register non-static key in stats_request
...
assign_lock_key
register_lock_class
lock_acquire
rt_spin_lock
start_wakeup_event drivers/virtio/virtio_balloon.c:150 [inline]
stats_request drivers/virtio/virtio_balloon.c:435
The lock is only initialized near the end of virtballoon_probe(),
after init_vqs() and several steps that can still fail and unwind
through del_vqs(), where a racing interrupt still finds the callback
registered. Until then it is whatever kzalloc() left in vb: lockdep
complains about the unregistered lock class, and wakeup_signal_mask
and processing_wakeup_event get updated from irq context while probe
is still running.
The rt_spin_lock() in the report is just spin_lock() on PREEMPT_RT;
any PROVE_LOCKING build takes the same path.
stop_update_lock(), which the same callback also acquires, is already
initialized before init_vqs(); do the same for wakeup_lock.
Fixes: c578123e63da ("virtio_balloon: Treat stats requests as wakeup events")
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=13df72e0dc4be5c62215
Signed-off-by: Mingbing Zhai <[email protected]>
---
Hi David,
On 24.08.26 14:57, David Hildenbrand (Arm) wrote:
> I don't really understand what's happening here, Is the
> register_lock_class() trying to reguster a non-static key?
It is: the wakeup_lock that stats_request() ends up taking at that
point hasn't been run through spin_lock_init() yet.
> I don't immediately see how virtio-balloon is doing something odd
> here, all it does is a spin_lock_irqsave(&vb->wakeup_lock, flags),
> that was earlier initialized with spin_lock_init(&vb->wakeup_lock),
> before the device was ready.
Right, the lock is initialized before virtio_device_ready(). But if
I follow correctly, stats_request() can already fire once init_vqs()
registers it as the stats virtqueue callback, some 100 lines before
the spin_lock_init(). I also don't see anything on the guest side
gating the IRQ: vring_interrupt() calls the callback without any
DRIVER_OK check unless CONFIG_VIRTIO_HARDEN_NOTIFICATION (which
depends on BROKEN). So a device raising the interrupt while probe is
still running would take the zero-initialized lock.
> virtio-balloon in an RT kernel/environment, unlikely in pactice ... :)
I don't think it is RT-specific, though: rt_spin_lock() is just what
spin_lock() compiles to on PREEMPT_RT, so any PROVE_LOCKING build
should hit the same register_lock_class() path on that lock.
stop_update_lock(), which the same callback takes first, is already
initialized before init_vqs(); the patch below does the same for
wakeup_lock. Does that look reasonable?
drivers/virtio/virtio_balloon.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index ab3e3d887e..8c1b47ec17 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -942,6 +942,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
spin_lock_init(&vb->stop_update_lock);
+ spin_lock_init(&vb->wakeup_lock);
mutex_init(&vb->balloon_lock);
init_waitqueue_head(&vb->acked);
vb->vdev = vdev;
@@ -1049,8 +1050,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
goto out_unregister_oom;
}
- spin_lock_init(&vb->wakeup_lock);
-
/*
* The virtio balloon itself can't wake up the device, but it is
* responsible for processing wakeup events passed up from the transport
--
2.53.0.windows.1
