Re: [PATCH] hw/timer/hpet: Fix expiration time overflow

[Akihiko Odaki]

On 2023/02/28 21:57, Michael S. Tsirkin wrote:
> On Mon, Jan 30, 2023 at 11:55:18PM +0100, Philippe Mathieu-Daudé wrote:
> > On 30/1/23 14:50, Akihiko Odaki wrote:
> > > The expiration time provided for timer_mod() can overflow if a
> > > ridiculously large value is set to the comparator register. The
> > > resulting value can represent a past time after rounded, forcing the
> > > timer to fire immediately. If the timer is configured as periodic, it
> > > will rearm the timer again, and form an endless loop.
> > >
> > > Check if the expiration value will overflow, and if it will, stop the
> > > timer instead of rearming the timer with the overflowed time.
> > >
> > > This bug was found by Alexander Bulekov when fuzzing igb, a new
> > > network device emulation:
> > > https://patchew.org/QEMU/20230129053316.1071513-1-alxndr@bu.edu/
> > >
> > > The fixed test case is:
> > > fuzz/crash_2d7036941dcda1ad4380bb8a9174ed0c949bcefd
> > >
> > > Fixes: 16b29ae180 ("Add HPET emulation to qemu (Beth Kon)")
> > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > ---
> > >    hw/timer/hpet.c | 19 +++++++++++++------
> > >    1 file changed, 13 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> > > index 9520471be2..3657d5f463 100644
> > > --- a/hw/timer/hpet.c
> > > +++ b/hw/timer/hpet.c
> > > @@ -352,6 +352,16 @@ static const VMStateDescription vmstate_hpet = {
> > >        }
> > >    };
> > > +static void arm(HPETTimer *t, uint64_t ticks)
> >
> > Could we rename as hpet_[re]arm() similarly to this file's other helpers?
>
> Akihiko Odaki, I expect there will be a new version of this?

There is v2:
https://patchew.org/QEMU/20230131030037.18856-1-akihiko.odaki@daynix.com/

Regards,
Akihiko Odaki

> > > +{
> > > +    if (ticks < ns_to_ticks(INT64_MAX / 2)) {
> > > +        timer_mod(t->qemu_timer,
> > > +                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ticks_to_ns(ticks));
> > > +    } else {
> > > +        timer_del(t->qemu_timer);
> > > +    }
> > > +}