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

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?

  Thomas


> otherwise falling back to TCI.
> 
> For sure, none of them are tested.
> 
> So, only TCI left?
> 
>>
>> We can't do any better than that as the caller code paths need a
>> time_t to pass into gmtime/localtime.
>>
>> So IMHO we should just declare it as time_t.
>>
>> With regards,
>> Daniel
>