* [PATCH] hw/timer/hpet: Fix expiration time overflow
@ 2023-01-30 13:50 Akihiko Odaki
2023-01-30 15:11 ` Michael S. Tsirkin
2023-01-30 22:55 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 5+ messages in thread
From: Akihiko Odaki @ 2023-01-30 13:50 UTC (permalink / raw)
Cc: qemu-devel, Paolo Bonzini, Michael S. Tsirkin, Alexander Bulekov,
Akihiko Odaki
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)
+{
+ 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);
+ }
+}
+
/*
* timer expiration callback
*/
@@ -374,13 +384,11 @@ static void hpet_timer(void *opaque)
}
}
diff = hpet_calculate_diff(t, cur_tick);
- timer_mod(t->qemu_timer,
- qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (int64_t)ticks_to_ns(diff));
+ arm(t, diff);
} else if (t->config & HPET_TN_32BIT && !timer_is_periodic(t)) {
if (t->wrap_flag) {
diff = hpet_calculate_diff(t, cur_tick);
- timer_mod(t->qemu_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
- (int64_t)ticks_to_ns(diff));
+ arm(t, diff);
t->wrap_flag = 0;
}
}
@@ -407,8 +415,7 @@ static void hpet_set_timer(HPETTimer *t)
t->wrap_flag = 1;
}
}
- timer_mod(t->qemu_timer,
- qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (int64_t)ticks_to_ns(diff));
+ arm(t, diff);
}
static void hpet_del_timer(HPETTimer *t)
--
2.39.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] hw/timer/hpet: Fix expiration time overflow
2023-01-30 13:50 [PATCH] hw/timer/hpet: Fix expiration time overflow Akihiko Odaki
@ 2023-01-30 15:11 ` Michael S. Tsirkin
2023-01-30 22:55 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2023-01-30 15:11 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: qemu-devel, Paolo Bonzini, Alexander Bulekov
On Mon, Jan 30, 2023 at 10:50:01PM +0900, 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>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
I'll tag this for merge.
> ---
> 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)
> +{
> + 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);
> + }
> +}
> +
> /*
> * timer expiration callback
> */
> @@ -374,13 +384,11 @@ static void hpet_timer(void *opaque)
> }
> }
> diff = hpet_calculate_diff(t, cur_tick);
> - timer_mod(t->qemu_timer,
> - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (int64_t)ticks_to_ns(diff));
> + arm(t, diff);
> } else if (t->config & HPET_TN_32BIT && !timer_is_periodic(t)) {
> if (t->wrap_flag) {
> diff = hpet_calculate_diff(t, cur_tick);
> - timer_mod(t->qemu_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> - (int64_t)ticks_to_ns(diff));
> + arm(t, diff);
> t->wrap_flag = 0;
> }
> }
> @@ -407,8 +415,7 @@ static void hpet_set_timer(HPETTimer *t)
> t->wrap_flag = 1;
> }
> }
> - timer_mod(t->qemu_timer,
> - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (int64_t)ticks_to_ns(diff));
> + arm(t, diff);
> }
>
> static void hpet_del_timer(HPETTimer *t)
> --
> 2.39.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hw/timer/hpet: Fix expiration time overflow
2023-01-30 13:50 [PATCH] hw/timer/hpet: Fix expiration time overflow Akihiko Odaki
2023-01-30 15:11 ` Michael S. Tsirkin
@ 2023-01-30 22:55 ` Philippe Mathieu-Daudé
2023-02-28 12:57 ` Michael S. Tsirkin
1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-30 22:55 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Paolo Bonzini, Michael S. Tsirkin, Alexander Bulekov
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?
> +{
> + 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);
> + }
> +}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hw/timer/hpet: Fix expiration time overflow
2023-01-30 22:55 ` Philippe Mathieu-Daudé
@ 2023-02-28 12:57 ` Michael S. Tsirkin
2023-03-01 3:47 ` Akihiko Odaki
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2023-02-28 12:57 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Akihiko Odaki, qemu-devel, Paolo Bonzini, Alexander Bulekov
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?
> > +{
> > + 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);
> > + }
> > +}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hw/timer/hpet: Fix expiration time overflow
2023-02-28 12:57 ` Michael S. Tsirkin
@ 2023-03-01 3:47 ` Akihiko Odaki
0 siblings, 0 replies; 5+ messages in thread
From: Akihiko Odaki @ 2023-03-01 3:47 UTC (permalink / raw)
To: Michael S. Tsirkin, Philippe Mathieu-Daudé
Cc: qemu-devel, Paolo Bonzini, Alexander Bulekov
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);
>>> + }
>>> +}
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-01 3:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-30 13:50 [PATCH] hw/timer/hpet: Fix expiration time overflow Akihiko Odaki
2023-01-30 15:11 ` Michael S. Tsirkin
2023-01-30 22:55 ` Philippe Mathieu-Daudé
2023-02-28 12:57 ` Michael S. Tsirkin
2023-03-01 3:47 ` Akihiko Odaki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).