On Wed, Jul 08, 2026 at 10:13:58AM +0200, David Hildenbrand (Arm) wrote:
> On 7/7/26 23:00, Michael S. Tsirkin wrote:
> > The virtio spec requires the driver not to kick the device before
> > DRIVER_OK is set. init_vqs() primes the stats virtqueue with a buffer
> > and kicks the device before virtio_device_ready() is called in
> > virtballoon_probe(), violating this requirement.
> >
> > Further, if the device responds to the early kick by processing the
> > buffer before DRIVER_OK, stats_request() fires and queues
> > update_balloon_stats_work. Should probe then fail and free vb, the work
> > runs against freed memory.
> >
> > To fix, move buffer setup to after DRIVER_OK. Be careful to
> > disable update_balloon_stats_work while this is going on,
> > to make sure it does not race with the setup.
> >
> > Testing: tested that stats still work after the change.
> >
> > Fixes: 9564e138b1f6 ("virtio: Add memory statistics reporting to the
> > balloon driver (V4)")
> > Reported-by: Sashiko:gemini-3.1-pro-preview
> > Cc: David Hildenbrand <[email protected]>
> > Assisted-by: Claude:claude-sonnet-4-6
> > Signed-off-by: Michael S. Tsirkin <[email protected]>
> >
> > ---
> > v4: extract stats_update_and_send() helper; call get_buf() before
> > updating stats buffer in stats_handle_request().
> > also ignore add bug failures (never trigger anyway) (reported by
> > sashiko)
> > v3: add disable_work comment and BUG_ON -> WARN_ON_ONCE (David Hildenbrand)
> > v2: check that work enable/disable is balanced; explain how add_outbuf
> > never fails in probe/restore
> > drivers/virtio/virtio_balloon.c | 67 +++++++++++++++++++--------------
> > 1 file changed, 39 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c
> > b/drivers/virtio/virtio_balloon.c
> > index 088b3a0e6ce6..4c4b9bea4356 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -417,6 +417,17 @@ static unsigned int update_balloon_stats(struct
> > virtio_balloon *vb)
> > return idx;
> > }
> >
> > +static void stats_update_and_send(struct virtio_balloon *vb)
> > +{
> > + struct scatterlist sg;
> > + unsigned int num_stats;
>
> Could do
>
> const unsigned int num_stats = update_balloon_stats(vb);
>
Indeed, a bit nicer. Thanks.
> LGTM.
>
> --
> Cheers,
>
> David
