[PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Jinjie Ruan]

In get_guest_rtc_ns(), "s->base_rtc" is uint64_t, which multiplied by
"NANOSECONDS_PER_SECOND" may overflow the uint64_t type, which will
cause the QEMU Linux Virtual Machine's RTC time to jump and in turn
triggers a kernel Soft Lockup and ultimately leads to a crash.

Fix it by avoiding adding s->base_rtc in get_guest_rtc_ns_offset(),
because get_guest_rtc_ns() is used either take the remainder of
NANOSECONDS_PER_SECOND or take the quotient of NANOSECONDS_PER_SECOND.

Fixes: 56038ef6234e ("RTC: Update the RTC clock only when reading it")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v2:
- Add comment for get_guest_rtc_ns().
- Update the commit message.
---
 hw/rtc/mc146818rtc.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 8631386b9f..36de30649c 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -77,12 +77,13 @@ static inline bool rtc_running(MC146818RtcState *s)
             (s->cmos_data[RTC_REG_A] & 0x70) <= 0x20);
 }
 
+/*
+ * Note: get_guest_rtc_ns() does not include the "base_rtc" seconds value,
+ * so the caller "must" handle it themselves!!!
+ */
 static uint64_t get_guest_rtc_ns(MC146818RtcState *s)
 {
-    uint64_t guest_clock = qemu_clock_get_ns(rtc_clock);
-
-    return s->base_rtc * NANOSECONDS_PER_SECOND +
-        guest_clock - s->last_update + s->offset;
+    return qemu_clock_get_ns(rtc_clock) - s->last_update + s->offset;
 }
 
 static void rtc_coalesced_timer_update(MC146818RtcState *s)
@@ -623,10 +624,8 @@ static void rtc_update_time(MC146818RtcState *s)
 {
     struct tm ret;
     time_t guest_sec;
-    int64_t guest_nsec;
 
-    guest_nsec = get_guest_rtc_ns(s);
-    guest_sec = guest_nsec / NANOSECONDS_PER_SECOND;
+    guest_sec = s->base_rtc + get_guest_rtc_ns(s) / NANOSECONDS_PER_SECOND;
     gmtime_r(&guest_sec, &ret);
 
     /* Is SET flag of Register B disabled? */
@@ -637,7 +636,7 @@ static void rtc_update_time(MC146818RtcState *s)
 
 static int update_in_progress(MC146818RtcState *s)
 {
-    int64_t guest_nsec;
+    uint64_t guest_nsec;
 
     if (!rtc_running(s)) {
         return 0;
-- 
2.34.1

Re: [PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Jinjie Ruan]

Gentle ping.

On 2026/1/14 9:32, Jinjie Ruan wrote:
> In get_guest_rtc_ns(), "s->base_rtc" is uint64_t, which multiplied by
> "NANOSECONDS_PER_SECOND" may overflow the uint64_t type, which will
> cause the QEMU Linux Virtual Machine's RTC time to jump and in turn
> triggers a kernel Soft Lockup and ultimately leads to a crash.
> 
> Fix it by avoiding adding s->base_rtc in get_guest_rtc_ns_offset(),
> because get_guest_rtc_ns() is used either take the remainder of
> NANOSECONDS_PER_SECOND or take the quotient of NANOSECONDS_PER_SECOND.
> 
> Fixes: 56038ef6234e ("RTC: Update the RTC clock only when reading it")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> v2:
> - Add comment for get_guest_rtc_ns().
> - Update the commit message.
> ---
>  hw/rtc/mc146818rtc.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index 8631386b9f..36de30649c 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -77,12 +77,13 @@ static inline bool rtc_running(MC146818RtcState *s)
>              (s->cmos_data[RTC_REG_A] & 0x70) <= 0x20);
>  }
>  
> +/*
> + * Note: get_guest_rtc_ns() does not include the "base_rtc" seconds value,
> + * so the caller "must" handle it themselves!!!
> + */
>  static uint64_t get_guest_rtc_ns(MC146818RtcState *s)
>  {
> -    uint64_t guest_clock = qemu_clock_get_ns(rtc_clock);
> -
> -    return s->base_rtc * NANOSECONDS_PER_SECOND +
> -        guest_clock - s->last_update + s->offset;
> +    return qemu_clock_get_ns(rtc_clock) - s->last_update + s->offset;
>  }
>  
>  static void rtc_coalesced_timer_update(MC146818RtcState *s)
> @@ -623,10 +624,8 @@ static void rtc_update_time(MC146818RtcState *s)
>  {
>      struct tm ret;
>      time_t guest_sec;
> -    int64_t guest_nsec;
>  
> -    guest_nsec = get_guest_rtc_ns(s);
> -    guest_sec = guest_nsec / NANOSECONDS_PER_SECOND;
> +    guest_sec = s->base_rtc + get_guest_rtc_ns(s) / NANOSECONDS_PER_SECOND;
>      gmtime_r(&guest_sec, &ret);
>  
>      /* Is SET flag of Register B disabled? */
> @@ -637,7 +636,7 @@ static void rtc_update_time(MC146818RtcState *s)
>  
>  static int update_in_progress(MC146818RtcState *s)
>  {
> -    int64_t guest_nsec;
> +    uint64_t guest_nsec;
>  
>      if (!rtc_running(s)) {
>          return 0;

Re: [PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Jinjie Ruan]

On 3/18/2026 3:03 PM, Jinjie Ruan wrote:
> Gentle ping.
> 
> On 2026/1/14 9:32, Jinjie Ruan wrote:
>> In get_guest_rtc_ns(), "s->base_rtc" is uint64_t, which multiplied by
>> "NANOSECONDS_PER_SECOND" may overflow the uint64_t type, which will
>> cause the QEMU Linux Virtual Machine's RTC time to jump and in turn
>> triggers a kernel Soft Lockup and ultimately leads to a crash.
>>
>> Fix it by avoiding adding s->base_rtc in get_guest_rtc_ns_offset(),
>> because get_guest_rtc_ns() is used either take the remainder of
>> NANOSECONDS_PER_SECOND or take the quotient of NANOSECONDS_PER_SECOND.
>>
>> Fixes: 56038ef6234e ("RTC: Update the RTC clock only when reading it")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> v2:
>> - Add comment for get_guest_rtc_ns().
>> - Update the commit message.

Hi, peter, do you have any further suggestions for this version, or is
it ready to be merged? Thanks!

>> ---
>>  hw/rtc/mc146818rtc.c | 15 +++++++--------
>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
>> index 8631386b9f..36de30649c 100644
>> --- a/hw/rtc/mc146818rtc.c
>> +++ b/hw/rtc/mc146818rtc.c
>> @@ -77,12 +77,13 @@ static inline bool rtc_running(MC146818RtcState *s)
>>              (s->cmos_data[RTC_REG_A] & 0x70) <= 0x20);
>>  }
>>  
>> +/*
>> + * Note: get_guest_rtc_ns() does not include the "base_rtc" seconds value,
>> + * so the caller "must" handle it themselves!!!
>> + */
>>  static uint64_t get_guest_rtc_ns(MC146818RtcState *s)
>>  {
>> -    uint64_t guest_clock = qemu_clock_get_ns(rtc_clock);
>> -
>> -    return s->base_rtc * NANOSECONDS_PER_SECOND +
>> -        guest_clock - s->last_update + s->offset;
>> +    return qemu_clock_get_ns(rtc_clock) - s->last_update + s->offset;
>>  }
>>  
>>  static void rtc_coalesced_timer_update(MC146818RtcState *s)
>> @@ -623,10 +624,8 @@ static void rtc_update_time(MC146818RtcState *s)
>>  {
>>      struct tm ret;
>>      time_t guest_sec;
>> -    int64_t guest_nsec;
>>  
>> -    guest_nsec = get_guest_rtc_ns(s);
>> -    guest_sec = guest_nsec / NANOSECONDS_PER_SECOND;
>> +    guest_sec = s->base_rtc + get_guest_rtc_ns(s) / NANOSECONDS_PER_SECOND;
>>      gmtime_r(&guest_sec, &ret);
>>  
>>      /* Is SET flag of Register B disabled? */
>> @@ -637,7 +636,7 @@ static void rtc_update_time(MC146818RtcState *s)
>>  
>>  static int update_in_progress(MC146818RtcState *s)
>>  {
>> -    int64_t guest_nsec;
>> +    uint64_t guest_nsec;
>>  
>>      if (!rtc_running(s)) {
>>          return 0;

Re: [PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Peter Maydell]

On Wed, 22 Apr 2026 at 04:08, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
>
>
> On 3/18/2026 3:03 PM, Jinjie Ruan wrote:
> > Gentle ping.
> >
> > On 2026/1/14 9:32, Jinjie Ruan wrote:
> >> In get_guest_rtc_ns(), "s->base_rtc" is uint64_t, which multiplied by
> >> "NANOSECONDS_PER_SECOND" may overflow the uint64_t type, which will
> >> cause the QEMU Linux Virtual Machine's RTC time to jump and in turn
> >> triggers a kernel Soft Lockup and ultimately leads to a crash.
> >>
> >> Fix it by avoiding adding s->base_rtc in get_guest_rtc_ns_offset(),
> >> because get_guest_rtc_ns() is used either take the remainder of
> >> NANOSECONDS_PER_SECOND or take the quotient of NANOSECONDS_PER_SECOND.
> >>
> >> Fixes: 56038ef6234e ("RTC: Update the RTC clock only when reading it")
> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> >> ---
> >> v2:
> >> - Add comment for get_guest_rtc_ns().
> >> - Update the commit message.
>
> Hi, peter, do you have any further suggestions for this version, or is
> it ready to be merged? Thanks!

As the mc146818's main user is the x86 PC models, this ought
to be reviewed by one of their maintainers. Paolo ?

thanks
-- PMM

Re: [PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Jinjie Ruan]

On 4/30/2026 8:53 PM, Peter Maydell wrote:
> On Wed, 22 Apr 2026 at 04:08, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>>
>>
>> On 3/18/2026 3:03 PM, Jinjie Ruan wrote:
>>> Gentle ping.
>>>
>>> On 2026/1/14 9:32, Jinjie Ruan wrote:
>>>> In get_guest_rtc_ns(), "s->base_rtc" is uint64_t, which multiplied by
>>>> "NANOSECONDS_PER_SECOND" may overflow the uint64_t type, which will
>>>> cause the QEMU Linux Virtual Machine's RTC time to jump and in turn
>>>> triggers a kernel Soft Lockup and ultimately leads to a crash.
>>>>
>>>> Fix it by avoiding adding s->base_rtc in get_guest_rtc_ns_offset(),
>>>> because get_guest_rtc_ns() is used either take the remainder of
>>>> NANOSECONDS_PER_SECOND or take the quotient of NANOSECONDS_PER_SECOND.
>>>>
>>>> Fixes: 56038ef6234e ("RTC: Update the RTC clock only when reading it")
>>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>>> ---
>>>> v2:
>>>> - Add comment for get_guest_rtc_ns().
>>>> - Update the commit message.
>>
>> Hi, peter, do you have any further suggestions for this version, or is
>> it ready to be merged? Thanks!
> 
> As the mc146818's main user is the x86 PC models, this ought
> to be reviewed by one of their maintainers. Paolo ?

That makes sense, as this change primarily impacts x86 platforms. I'll
wait for Paolo's feedback. @Paolo, please let me know if you have any
concerns.

> 
> thanks
> -- PMM
> 

Re: [PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Paolo Bonzini]

On 1/14/26 02:32, Jinjie Ruan wrote:
> In get_guest_rtc_ns(), "s->base_rtc" is uint64_t, which multiplied by
> "NANOSECONDS_PER_SECOND" may overflow the uint64_t type, which will
> cause the QEMU Linux Virtual Machine's RTC time to jump and in turn
> triggers a kernel Soft Lockup and ultimately leads to a crash.
> 
> Fix it by avoiding adding s->base_rtc in get_guest_rtc_ns_offset(),
> because get_guest_rtc_ns() is used either take the remainder of
> NANOSECONDS_PER_SECOND or take the quotient of NANOSECONDS_PER_SECOND.
> 
> Fixes: 56038ef6234e ("RTC: Update the RTC clock only when reading it")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

Thanks, queued - I also renamed the function to 
get_rtc_ns_since_last_update().  It's not entirely accurate due the 
presence of the sub-second offset s->offset, but it's good enough.

I have also considered changing the function to return a struct 
timespec.  I didn't like it too much because struct timespec uses a 
time_t which is possibly 32-bit.  Now, QEMU is probably broken for Y2038 
anyway, but having time_t dependencies in device emulation code is not 
great; at least mktimegm and gmtime_r could be rewritten easily.

Apologies for the delay!

Paolo

Re: [PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Philippe Mathieu-Daudé]

On 11/5/26 09:06, Paolo Bonzini wrote:

> I have also considered changing the function to return a struct 
> timespec.  I didn't like it too much because struct timespec uses a 
> time_t which is possibly 32-bit.  Now, QEMU is probably broken for Y2038 
> anyway, but having time_t dependencies in device emulation code is not 
> great; at least mktimegm and gmtime_r could be rewritten easily.

Worth tracking in GitLab issues?

Re: [PATCH v2] hw/rtc/mc146818rtc: Fix get_guest_rtc_ns() overflow bug

[Daniel P. Berrangé]

On Mon, May 11, 2026 at 09:06:54AM +0200, Paolo Bonzini wrote:
> On 1/14/26 02:32, Jinjie Ruan wrote:
> > In get_guest_rtc_ns(), "s->base_rtc" is uint64_t, which multiplied by
> > "NANOSECONDS_PER_SECOND" may overflow the uint64_t type, which will
> > cause the QEMU Linux Virtual Machine's RTC time to jump and in turn
> > triggers a kernel Soft Lockup and ultimately leads to a crash.
> > 
> > Fix it by avoiding adding s->base_rtc in get_guest_rtc_ns_offset(),
> > because get_guest_rtc_ns() is used either take the remainder of
> > NANOSECONDS_PER_SECOND or take the quotient of NANOSECONDS_PER_SECOND.
> > 
> > Fixes: 56038ef6234e ("RTC: Update the RTC clock only when reading it")
> > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> 
> Thanks, queued - I also renamed the function to
> get_rtc_ns_since_last_update().  It's not entirely accurate due the presence
> of the sub-second offset s->offset, but it's good enough.
> 
> I have also considered changing the function to return a struct timespec.  I
> didn't like it too much because struct timespec uses a time_t which is
> possibly 32-bit.  Now, QEMU is probably broken for Y2038 anyway, but having
> time_t dependencies in device emulation code is not great; at least mktimegm
> and gmtime_r could be rewritten easily.

For system mode at least, we can assume time_t will be 64-bit only
since we drop 32-bit host OS support.

For user mode, we can likewise assume 64-bit host OS, but still have
to deal with potential 32-bit time_t for the guest OS. One could
argue that in the user mode case though, the guest ABI is what is
broken for Y2038 - QEMU is just honouring the (broken) ABI that is
defined.

The tools could be impacted by 32-bit issues I guess, but we still
have 10+ years to kill off the 32-bit host OS build support for
tools

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|