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

["Alex Bennée" <alex.bennee@linaro.org>]

Thomas Huth <thuth@redhat.com> writes:

> 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 <thuth@redhat.com>
>>>>>>
>>>>>> 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 <thuth@redhat.com>
>>>>>> ---
>>>>>>   system/rtc.c | 4 ++--
>>>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>>
>>>> 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?

Didn't we re-enable 32 bit hosts for qemu-img and friends?
>
>  Thomas

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro