[PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Einar Jon Gunnarsson]

The check for BITS_PER_LONG == 32 makes no sense after calling
tv64.tv_sec = rtc_tm_to_time64(&tm);

With this check, any 32-bit system will silently return an -ERANGE error
instead of setting the correct time if the hardware clock is storing a
date after Y2K38 (2038-01-19).
Without this check they should all work as intended, since the rest of
the function is perfectly 64-bit safe.

Fixes: f9b2a4d6a5f1 ("rtc: class: support hctosys from modular RTC drivers")

Signed-off-by: Einar Jon Gunnarsson <tolvupostur@gmail.com>
---
 drivers/rtc/class.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index e31fa0ad127e..df58edf99ed3 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -72,13 +72,6 @@ static void rtc_hctosys(struct rtc_device *rtc)
 
 	tv64.tv_sec = rtc_tm_to_time64(&tm);
 
-#if BITS_PER_LONG == 32
-	if (tv64.tv_sec > INT_MAX) {
-		err = -ERANGE;
-		goto err_read;
-	}
-#endif
-
 	err = do_settimeofday64(&tv64);
 
 	dev_info(rtc->dev.parent, "setting system clock to %ptR UTC (%lld)\n",
-- 
2.34.1

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Einar Jón]

Hello again

On second thought, removing is too general.
But it's still very much broken. Is there any reason why this was not merged?
https://lore.kernel.org/all/c5e8ab50-aacb-4651-8893-a6dd9edcd155@app.fastmail.com/T/

Any thoughts on how this should be handled?

Best regards
Einar Jón

On Tue, 18 Feb 2025 at 13:45, Einar Jón <tolvupostur@gmail.com> wrote:
>
> Hello again
>
> On second thought, removing is too general.
> But it's still very much broken. Is there any reason why this was not merged?
> https://lore.kernel.org/all/c5e8ab50-aacb-4651-8893-a6dd9edcd155@app.fastmail.com/T/
>
> Any thoughts on how this should be handled?
>
> Best regards
> Einar Jón
>
> On Tue, 18 Feb 2025 at 12:12, Einar Jon Gunnarsson <tolvupostur@gmail.com> wrote:
>>
>> The check for BITS_PER_LONG == 32 makes no sense after calling
>> tv64.tv_sec = rtc_tm_to_time64(&tm);
>>
>> With this check, any 32-bit system will silently return an -ERANGE error
>> instead of setting the correct time if the hardware clock is storing a
>> date after Y2K38 (2038-01-19).
>> Without this check they should all work as intended, since the rest of
>> the function is perfectly 64-bit safe.
>>
>> Fixes: f9b2a4d6a5f1 ("rtc: class: support hctosys from modular RTC drivers")
>>
>> Signed-off-by: Einar Jon Gunnarsson <tolvupostur@gmail.com>
>> ---
>>  drivers/rtc/class.c | 7 -------
>>  1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
>> index e31fa0ad127e..df58edf99ed3 100644
>> --- a/drivers/rtc/class.c
>> +++ b/drivers/rtc/class.c
>> @@ -72,13 +72,6 @@ static void rtc_hctosys(struct rtc_device *rtc)
>>
>>         tv64.tv_sec = rtc_tm_to_time64(&tm);
>>
>> -#if BITS_PER_LONG == 32
>> -       if (tv64.tv_sec > INT_MAX) {
>> -               err = -ERANGE;
>> -               goto err_read;
>> -       }
>> -#endif
>> -
>>         err = do_settimeofday64(&tv64);
>>
>>         dev_info(rtc->dev.parent, "setting system clock to %ptR UTC (%lld)\n",
>> --
>> 2.34.1
>>
>
>
> --
> Regards
> Einar Jón



-- 
Regards
Einar Jón

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Alexandre Belloni]

On 18/02/2025 13:45:56+0100, Einar Jón wrote:
> Hello again
> 
> On second thought, removing is too general.
> But it's still very much broken. Is there any reason why this was not
> merged?
> https://lore.kernel.org/all/c5e8ab50-aacb-4651-8893-a6dd9edcd155@app.fastmail.com/T/
> 
> Any thoughts on how this should be handled?

The first step is to convince Lennart that mandating RTC_HCTOSYS xwas a
bad idea, the second step is to let userspace set the time instead. The
kernel can't take the proper decision because it simply doesn't know
whether userspace is TIME64 ready or not.

> 
> Best regards
> Einar Jón
> 
> On Tue, 18 Feb 2025 at 12:12, Einar Jon Gunnarsson <tolvupostur@gmail.com>
> wrote:
> 
> > The check for BITS_PER_LONG == 32 makes no sense after calling
> > tv64.tv_sec = rtc_tm_to_time64(&tm);
> >
> > With this check, any 32-bit system will silently return an -ERANGE error
> > instead of setting the correct time if the hardware clock is storing a
> > date after Y2K38 (2038-01-19).
> > Without this check they should all work as intended, since the rest of
> > the function is perfectly 64-bit safe.
> >
> > Fixes: f9b2a4d6a5f1 ("rtc: class: support hctosys from modular RTC
> > drivers")
> >
> > Signed-off-by: Einar Jon Gunnarsson <tolvupostur@gmail.com>
> > ---
> >  drivers/rtc/class.c | 7 -------
> >  1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> > index e31fa0ad127e..df58edf99ed3 100644
> > --- a/drivers/rtc/class.c
> > +++ b/drivers/rtc/class.c
> > @@ -72,13 +72,6 @@ static void rtc_hctosys(struct rtc_device *rtc)
> >
> >         tv64.tv_sec = rtc_tm_to_time64(&tm);
> >
> > -#if BITS_PER_LONG == 32
> > -       if (tv64.tv_sec > INT_MAX) {
> > -               err = -ERANGE;
> > -               goto err_read;
> > -       }
> > -#endif
> > -
> >         err = do_settimeofday64(&tv64);
> >
> >         dev_info(rtc->dev.parent, "setting system clock to %ptR UTC
> > (%lld)\n",
> > --
> > 2.34.1
> >
> >
> 
> -- 
> Regards
> Einar Jón

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Arnd Bergmann]

On Tue, Feb 18, 2025, at 14:30, Alexandre Belloni wrote:
> On 18/02/2025 13:45:56+0100, Einar Jón wrote:
>> Hello again
>> 
>> On second thought, removing is too general.
>> But it's still very much broken. Is there any reason why this was not
>> merged?
>> https://lore.kernel.org/all/c5e8ab50-aacb-4651-8893-a6dd9edcd155@app.fastmail.com/T/
>> 
>> Any thoughts on how this should be handled?
>
> The first step is to convince Lennart that mandating RTC_HCTOSYS xwas a
> bad idea, the second step is to let userspace set the time instead. The
> kernel can't take the proper decision because it simply doesn't know
> whether userspace is TIME64 ready or not.

It's been seven years since the you added the workaround, and
there are a few things that have changed in the meantime:

- most distros that use systemd have stopped supporting
  32-bit targets
- most distros that still support 32-bit have moved to a
  64-bit time_t
- 2038 is only 13 years away instead of 20, adding to the
  urgency of having future-proof default behavior.

I don't know how many 32-bit machines are affected by the bug
where they return a random time, or if they are more or less
common than in the past.

Probably some variant of my old patch is still appropriate
now.

     Arnd

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Alexandre Belloni]

On 18/02/2025 15:51:24+0100, Arnd Bergmann wrote:
> On Tue, Feb 18, 2025, at 14:30, Alexandre Belloni wrote:
> > On 18/02/2025 13:45:56+0100, Einar Jón wrote:
> >> Hello again
> >> 
> >> On second thought, removing is too general.
> >> But it's still very much broken. Is there any reason why this was not
> >> merged?
> >> https://lore.kernel.org/all/c5e8ab50-aacb-4651-8893-a6dd9edcd155@app.fastmail.com/T/
> >> 
> >> Any thoughts on how this should be handled?
> >
> > The first step is to convince Lennart that mandating RTC_HCTOSYS xwas a
> > bad idea, the second step is to let userspace set the time instead. The
> > kernel can't take the proper decision because it simply doesn't know
> > whether userspace is TIME64 ready or not.
> 
> It's been seven years since the you added the workaround, and
> there are a few things that have changed in the meantime:
> 
> - most distros that use systemd have stopped supporting
>   32-bit targets
> - most distros that still support 32-bit have moved to a
>   64-bit time_t
> - 2038 is only 13 years away instead of 20, adding to the
>   urgency of having future-proof default behavior.
> 
> I don't know how many 32-bit machines are affected by the bug
> where they return a random time, or if they are more or less
> common than in the past.

This is going to break some of the Marvell board that RMK uses because I
guess he is not updating his userspace.

Also, I'd note that OpenEmbedded switched to 64-bit time_t by default
just last year.

I'm not against removing the workaround but keeping it doesn't actually
break anyone, it is still possible to set the time properly from
userspace as it should be done anyway so I'm not sure about the urgency.
The impact is mostly about messages timestamp in the boot log, before
being able to run hwclock or any similar tool.

Or am I missing anything?


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Einar Jón]

On Tue, 18 Feb 2025 at 16:40, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 18/02/2025 15:51:24+0100, Arnd Bergmann wrote:
> > On Tue, Feb 18, 2025, at 14:30, Alexandre Belloni wrote:
> > > On 18/02/2025 13:45:56+0100, Einar Jón wrote:
> > >> Hello again
> > >>
> > >> On second thought, removing is too general.
> > >> But it's still very much broken. Is there any reason why this was not
> > >> merged?
> > >> https://lore.kernel.org/all/c5e8ab50-aacb-4651-8893-a6dd9edcd155@app.fastmail.com/T/
> > >>
> > >> Any thoughts on how this should be handled?
> > >
> > > The first step is to convince Lennart that mandating RTC_HCTOSYS xwas a
> > > bad idea, the second step is to let userspace set the time instead. The
> > > kernel can't take the proper decision because it simply doesn't know
> > > whether userspace is TIME64 ready or not.
> >
> > It's been seven years since the you added the workaround, and
> > there are a few things that have changed in the meantime:
> >
> > - most distros that use systemd have stopped supporting
> >   32-bit targets
> > - most distros that still support 32-bit have moved to a
> >   64-bit time_t
> > - 2038 is only 13 years away instead of 20, adding to the
> >   urgency of having future-proof default behavior.
> >
> > I don't know how many 32-bit machines are affected by the bug
> > where they return a random time, or if they are more or less
> > common than in the past.
>
> This is going to break some of the Marvell board that RMK uses because I
> guess he is not updating his userspace.
>
> Also, I'd note that OpenEmbedded switched to 64-bit time_t by default
> just last year.

This triggered me on a yocto build (armhf) that is indeed using 64-bit time_t
and systemd. I just assumed everyone is on 64-bit time_t already, since glibc
made the change like 3 years ago. My bad.

> I'm not against removing the workaround but keeping it doesn't actually
> break anyone, it is still possible to set the time properly from
> userspace as it should be done anyway so I'm not sure about the urgency.
> The impact is mostly about messages timestamp in the boot log, before
> being able to run hwclock or any similar tool.

I have over a decade until this becomes urgent.
I'll be OK with it being unchanged for a little longer.

> Or am I missing anything?

Without a doubt. So let's not rock the boat and keep it as is.
Although printing a warning before err = -ERANGE; would be nice.

>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

-- 
Regards
Einar Jón

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Arnd Bergmann]

On Tue, Feb 18, 2025, at 16:40, Alexandre Belloni wrote:
> On 18/02/2025 15:51:24+0100, Arnd Bergmann wrote:
>> On Tue, Feb 18, 2025, at 14:30, Alexandre Belloni wrote:
>> 
>> I don't know how many 32-bit machines are affected by the bug
>> where they return a random time, or if they are more or less
>> common than in the past.
>
> This is going to break some of the Marvell board that RMK uses because I
> guess he is not updating his userspace.
>
> Also, I'd note that OpenEmbedded switched to 64-bit time_t by default
> just last year.
>
> I'm not against removing the workaround but keeping it doesn't actually
> break anyone, it is still possible to set the time properly from
> userspace as it should be done anyway so I'm not sure about the urgency.
> The impact is mostly about messages timestamp in the boot log, before
> being able to run hwclock or any similar tool.
>
> Or am I missing anything?

The main problem with the current approach is that it suddenly
breaks systems in the future (at the time_t overflow), which is
the opposite of how the rest of the time_t conversion worked.

We now have distros with systemd that advertize support [1],
and we know this breaks every single machine they run on that
uses an RTC to set the time.

     Arnd

[1] https://ubuntu.com/blog/lts-cra-arm

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Alexandre Belloni]

On 18/02/2025 17:05:23+0100, Arnd Bergmann wrote:
> On Tue, Feb 18, 2025, at 16:40, Alexandre Belloni wrote:
> > On 18/02/2025 15:51:24+0100, Arnd Bergmann wrote:
> >> On Tue, Feb 18, 2025, at 14:30, Alexandre Belloni wrote:
> >> 
> >> I don't know how many 32-bit machines are affected by the bug
> >> where they return a random time, or if they are more or less
> >> common than in the past.
> >
> > This is going to break some of the Marvell board that RMK uses because I
> > guess he is not updating his userspace.
> >
> > Also, I'd note that OpenEmbedded switched to 64-bit time_t by default
> > just last year.
> >
> > I'm not against removing the workaround but keeping it doesn't actually
> > break anyone, it is still possible to set the time properly from
> > userspace as it should be done anyway so I'm not sure about the urgency.
> > The impact is mostly about messages timestamp in the boot log, before
> > being able to run hwclock or any similar tool.
> >
> > Or am I missing anything?
> 
> The main problem with the current approach is that it suddenly
> breaks systems in the future (at the time_t overflow), which is
> the opposite of how the rest of the time_t conversion worked.

Yes, it will suddenly stop setting the time on boot which the platform
can (and will) survive because it will probably then start NTP. However,
without the workaround, systemd will just crash on boot, bricking the
device. So as I see it, on one hand, I have mostly recoverable breakage
and on the other breakage that mean going on the field and reflashing
the device because the system will probably not be able to do any OTA
anymore.

> 
> We now have distros with systemd that advertize support [1],
> and we know this breaks every single machine they run on that
> uses an RTC to set the time.

No, this breaks systems that only rely on RTC_HCTOSYS without a
fallback.

RTC_HCTOSYS alone is anyway pretty bad idea and no one interested in
time should use it because it can be off up to a second which can take
4 to 7 days to resolve by selewing and not stepping. Anyone serious
should use hwclock or similar to set the system time precisely after
boot so the offset will be closer to a few ms.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] rtc: class: Fix max epoch in rtc_hctosys on 32-bit systems

[Einar Jón]

On Tue, 18 Feb 2025 at 17:39, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 18/02/2025 17:05:23+0100, Arnd Bergmann wrote:
> > On Tue, Feb 18, 2025, at 16:40, Alexandre Belloni wrote:
> > > On 18/02/2025 15:51:24+0100, Arnd Bergmann wrote:
> > >> On Tue, Feb 18, 2025, at 14:30, Alexandre Belloni wrote:
> > >>
> > >> I don't know how many 32-bit machines are affected by the bug
> > >> where they return a random time, or if they are more or less
> > >> common than in the past.
> > >
> > > This is going to break some of the Marvell board that RMK uses because I
> > > guess he is not updating his userspace.
> > >
> > > Also, I'd note that OpenEmbedded switched to 64-bit time_t by default
> > > just last year.
> > >
> > > I'm not against removing the workaround but keeping it doesn't actually
> > > break anyone, it is still possible to set the time properly from
> > > userspace as it should be done anyway so I'm not sure about the urgency.
> > > The impact is mostly about messages timestamp in the boot log, before
> > > being able to run hwclock or any similar tool.
> > >
> > > Or am I missing anything?
> >
> > The main problem with the current approach is that it suddenly
> > breaks systems in the future (at the time_t overflow), which is
> > the opposite of how the rest of the time_t conversion worked.
>
> Yes, it will suddenly stop setting the time on boot which the platform
> can (and will) survive because it will probably then start NTP. However,
> without the workaround, systemd will just crash on boot, bricking the
> device. So as I see it, on one hand, I have mostly recoverable breakage
> and on the other breakage that mean going on the field and reflashing
> the device because the system will probably not be able to do any OTA
> anymore.

You guys think that all devices have a network connection? Even in 2038
I don't think that will be anywhere close to true, since there are a ton of
32-bit embedded devices out there running linux with no network.
Just my company has thousands.

I'm using rtc-pcf8563 with systemd on i.MX6 (majority has no internet), and
I just removed this check locally. That way my devices can keep time until
2070 (EPOCH+100Y), which is currently the upper limit of the rtc-pcf8563.

>
> >
> > We now have distros with systemd that advertize support [1],
> > and we know this breaks every single machine they run on that
> > uses an RTC to set the time.
>
> No, this breaks systems that only rely on RTC_HCTOSYS without a
> fallback.
>
> RTC_HCTOSYS alone is anyway pretty bad idea and no one interested in
> time should use it because it can be off up to a second which can take
> 4 to 7 days to resolve by selewing and not stepping. Anyone serious
> should use hwclock or similar to set the system time precisely after
> boot so the offset will be closer to a few ms.
>

Suits me fine. I do think that documenting this would be nice, since most of the
hardware seems to be able to handle dates after Y2038 - it's mostly a userspace
issue now.

-- 
Regards
Einar Jón