Re: [Qemu-devel] [PATCH] virtio-balloon: fix buffer overflow in memory stats feature

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 15 Sep 2014 13:16:01 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 09/15/2014 12:09 PM, Luiz Capitulino wrote:
> > When a QMP client changes the polling interval time by setting
> > the guest-stats-polling-interval property, the interval value
> > is stored and manipuled as an int64_t variable.
> > 
> 
> s/manipuled/manipulated/
> 
> > However, the balloon_stats_change_timer() function, which is
> > used to set the actual timer with the interval value, takes
> > an int instead, causing an overflow for big interval values.
> > 
> > Fix it.
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hw/virtio/virtio-balloon.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> > index 2c30b3d..9629264 100644
> > --- a/hw/virtio/virtio-balloon.c
> > +++ b/hw/virtio/virtio-balloon.c
> > @@ -87,7 +87,7 @@ static void balloon_stats_destroy_timer(VirtIOBalloon *s)
> >      }
> >  }
> >  
> > -static void balloon_stats_change_timer(VirtIOBalloon *s, int secs)
> > +static void balloon_stats_change_timer(VirtIOBalloon *s, int64_t secs)
> >  {
> >      timer_mod(s->stats_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + secs * 1000);
> 
> secs * 1000 can still overflow for (really large) values, do we care
> about that?

Hmm, good point. I think I could keep the s/int/int64_t change but limit secs
to UINT_MAX for simplicity. I guess we don't expect anyone to set this to
billions of seconds in the future :)