Re: [PATCH] system/rtc: Fix a possible year-2038 integer overflow problem

On 19/05/2026 11.18, Thomas Huth wrote:
> On 19/05/2026 10.24, Philippe Mathieu-Daudé wrote:
>> On 18/5/26 18:18, Daniel P. Berrangé wrote:
>>> On Mon, May 18, 2026 at 05:10:36PM +0100, Daniel P. Berrangé wrote:
>>>> On Mon, May 18, 2026 at 06:02:55PM +0200, Thomas Huth wrote:
>>>>> From: Thomas Huth <[email protected]>
>>>>>
>>>>> rtc_realtime_clock_offset is initialized with:
>>>>>
>>>>>    rtc_realtime_clock_offset = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 
>>>>> 1000;
>>>>>
>>>>> And QEMU_CLOCK_REALTIME might be based on gettimeofday() in certain
>>>>> cases (see get_clock_realtime() in include/qemu/timer.h). So this
>>>>> counter will exceed 32 bits in the year 2038, thus we should not
>>>>> store this value in a normal integer variable. Change it to an int64_t
>>>>> to fix the problem.
>>>>> And while we're at it, also adjust the nearby rtc_host_datetime_offset
>>>>> variable to be on the safe side in the related code.
>>>>>
>>>>> Signed-off-by: Thomas Huth <[email protected]>
>>>>> ---
>>>>>   system/rtc.c | 4 ++--
>>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> Reviewed-by: Daniel P. Berrangé <[email protected]>
>>>
>>> Actually on second reading I think this patch is not quite right.
>>> The code which uses the value is:
>>>
>>>      time_t value = qemu_clock_get_ms(clock) / 1000;
>>>      switch (clock) {
>>>      case QEMU_CLOCK_REALTIME:
>>>          value -= rtc_realtime_clock_offset;
>>>
>>>
>>> On 64-bit platforms 'int' was 64-bit already and so was time_t so we
>>> have no bug to fix.
>>>
>>> On 32-bit platforms the patch fixes 'int' to 'int64' which is fine
>>> but that int64 value is then subtracted from time_t which is usually
>>> going to be 32-bit again, unless the OS was built with 64-bit time_t
>>> on 32-bit which almost no one does.
>>>
>>> IOW we only platform this fixes is 32-bit OS with 64-bit time_t.
>>
>> "32-bit *host* OS".
>>
>> Per ./configure script, we still allow:
>> - x86_64 x32 ABI      (CPU_CFLAGS="-mx32")
> 
> We should maybe deprecate that x32 ABI ...
> 
>> - sparc32             (CPU_CFLAGS="-m32 -mv8plus -mcpu=ultrasparc")
> 
> ... and that one as well?
> 
>> - s390                (CPU_CFLAGS="-m31")
> 
> Uh, we really still allow 31-bit s390 hosts here? I think this could be 
> removed nowadays, also upstream kernel recently ditched the 31-bit userspace:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ 
> commit/?id=8e0b986c59c67e08ada646249f834655a9e6da16
> 
> Question is whether we can remove it immediately, or whether it needs to be 
> deprecated first?

Ah, wait, we completely disallow disallow 32-bit hosts nowadays:

  https://gitlab.com/qemu-project/qemu/-/commit/372ec46b9f1215f48a4717f2b7ed969f65bfadc6

... so these checks in the configure script are just dead code nowadays that 
could be removed now? Or is there still some hidden magic here that depends 
on these lines?

  Thomas