2022-09-08 12:09:52

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] [RFC] rtc: y2038: remove broken RTC_HCTOSYS workaround

From: Arnd Bergmann <[email protected]>

The RTC_HCTOSYS is fundamentally flawed for legacy user space, and
unfortunately there are no good solutions for it.

As Reinier Kuipers reports, the current code is broken for any 32-bit
platform that relies on CONFIG_RTC_HCTOSYS when they want to support
64-bit time_t. Running with the RTC set to 2038 or later, the system
clock silently reverts back to 1970, which makes it impossible to actually
deploy such a system today and expect it to keep working.

The code that caused this issue was in turn a workaround for a recurring
problem that happens when RTCs are set to the wrong time accidentally
or as the result of a depleted battery. If the time is accidentally set
past INT_MAX, this causes a systemd with 32-bit time_t to hang on boot,
with no easy recovery.

As the kernel has no idea about whether the time from the RTC is real
or fake, or if the user space it will run has 32-bit or 64-bit time_t,
it is impossible to make CONFIG_RTC_HCTOSYS work in a way that reliably
avoids both of the above problems.

Additionally, the current behavior is inconsistent because it works
differently based on whether the kernel itself is 32-bit or 64-bit,
regardless of what userspace is actually running. Configurations in
full compat mode are getting more popular (again) as Arm based SoCs are
increasingly built using 64-bit CPU cores like Cortex-A35, but memory
constrained systems still benefit from running 32-bit user space.

Change the current behavior in two ways:

- In an attempt to pick the lesser of two evils, do not actually set
the time differently based on the boot-time RTC contents, but
instead warn about affected configurations. This effectively reverts
b3a5ac42ab18 ("rtc: hctosys: Ensure system time doesn't overflow
time_t") and will make systemd hang again on broken hardware, but
at least makes it possible to deploy y2038 ready 32-bit systems.

- Change the #ifdef to check for CONFIG_COMPAT_32BIT_TIME instead of
BITS_PER_LONG, so we do not catch kernels that only support time64_t
userspace, but do catch 64-bit kernels that may run a 32-bit rootfs.

Other ideas that have been suggested but discarded include:

- Using a blacklist or whitelist of RTC drivers to only set
the time on reliable hardware, which would effectively break current
users of that hardware immediately, rather than in 16 years.

- Changing affected RTC drivers to interpret the register contents
as belonging within the 32-bit time_t range but still allow changing
that window using the "start-year" DT property to make it work
past y2038. This work avoid most of the problems but still requires
identifying exactly which drivers are affected.

- Dropping CONFIG_RTC_HCTOSYS from the kernel. This is still
Alexandre's long-term goal, but we cannot wait for that as users
are trying to deploy y2038 compatible systems today.

- Making CONFIG_RTC_HCTOSYS depend on !CONFIG_COMPAT_32BIT_TIME,
which is a trivial way of avoiding both of the bugs, but breaks
all general purpose 32-bit userland until they upgrade to
a full time64 build or away from RTC_HCTOSYS.

- Changing systemd to no longer crash on broken hardware, or
ideally just set the system time itself to a value that is within
its time_t range. This was previously rejected by the systemd
developers, but is probably needed again with the b3a5ac42ab18
patch reverted.

Cc: Reinier Kuipers <[email protected]>
Cc: Alexandre Belloni <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
Cc: Russell King <[email protected]>
Link: https://lore.kernel.org/all/CAKYb531CyL8XRVRcRN30cC3xRgsd-1FzXUeS7o2LiZqALJ42qw@mail.gmail.com/
Link: https://github.com/systemd/systemd/issues/1143
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/rtc/class.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index e48223c00c67..572e356821b5 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -73,11 +73,31 @@ 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;
- }
+#ifdef CONFIG_COMPAT_32BIT_TIME
+ /*
+ * User space with 32-bit time_t behaves in unexpected ways when
+ * the time is set past January 19 2038, often leading to
+ * unbootable systems. With CONFIG_RTC_HCTOSYS, this can happen
+ * either because of broken RTC hardware that sets a random time
+ * with a depleted battery, or because it is actually the future.
+ *
+ * This can affect 32-bit kernels as well as 64-bit kernels when
+ * running a 32-bit rootfs or container.
+ *
+ * Since the kernel has no way of knowing what user space it runs,
+ * warn here whenever the kernel is able to run it.
+ * When CONFIG_COMPAT_32BIT_TIME is disabled, we know that the
+ * system is safe, but unfortunately this this is currently not
+ * supported by musl-1.2.x or most glibc based user space.
+ *
+ * Disabling CONFIG_RTC_HCTOSYS defers the problem to user space,
+ * which will work correctly when all of it supports 64-bit
+ * time_t, or will alternatively be unable to set a time past
+ * y2038 if it still uses 32-bit time_t.
+ */
+ dev_warn(rtc->dev.parent, "Warning: CONFIG_RTC_HCTOSYS is broken for 32-bit time_t userspace\n");
+ if (tv64.tv_sec > INT_MAX)
+ dev_warn(rtc->dev.parent, "Time set to past y2038 date\n");
#endif

err = do_settimeofday64(&tv64);
--
2.29.2


2022-11-15 22:34:35

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH] [RFC] rtc: y2038: remove broken RTC_HCTOSYS workaround

Hello,

I'm fine with the patch and I'll probably take it, I a an observation
though:

On 08/09/2022 13:53:20+0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The RTC_HCTOSYS is fundamentally flawed for legacy user space, and
> unfortunately there are no good solutions for it.
>
> As Reinier Kuipers reports, the current code is broken for any 32-bit
> platform that relies on CONFIG_RTC_HCTOSYS when they want to support
> 64-bit time_t. Running with the RTC set to 2038 or later, the system
> clock silently reverts back to 1970, which makes it impossible to actually
> deploy such a system today and expect it to keep working.
>
> The code that caused this issue was in turn a workaround for a recurring
> problem that happens when RTCs are set to the wrong time accidentally
> or as the result of a depleted battery. If the time is accidentally set
> past INT_MAX, this causes a systemd with 32-bit time_t to hang on boot,
> with no easy recovery.
>
> As the kernel has no idea about whether the time from the RTC is real
> or fake, or if the user space it will run has 32-bit or 64-bit time_t,
> it is impossible to make CONFIG_RTC_HCTOSYS work in a way that reliably
> avoids both of the above problems.
>
> Additionally, the current behavior is inconsistent because it works
> differently based on whether the kernel itself is 32-bit or 64-bit,
> regardless of what userspace is actually running. Configurations in
> full compat mode are getting more popular (again) as Arm based SoCs are
> increasingly built using 64-bit CPU cores like Cortex-A35, but memory
> constrained systems still benefit from running 32-bit user space.
>
> Change the current behavior in two ways:
>
> - In an attempt to pick the lesser of two evils, do not actually set
> the time differently based on the boot-time RTC contents, but
> instead warn about affected configurations. This effectively reverts
> b3a5ac42ab18 ("rtc: hctosys: Ensure system time doesn't overflow
> time_t") and will make systemd hang again on broken hardware, but
> at least makes it possible to deploy y2038 ready 32-bit systems.
>
> - Change the #ifdef to check for CONFIG_COMPAT_32BIT_TIME instead of
> BITS_PER_LONG, so we do not catch kernels that only support time64_t
> userspace, but do catch 64-bit kernels that may run a 32-bit rootfs.
>
> Other ideas that have been suggested but discarded include:
>
> - Using a blacklist or whitelist of RTC drivers to only set
> the time on reliable hardware, which would effectively break current
> users of that hardware immediately, rather than in 16 years.
>
> - Changing affected RTC drivers to interpret the register contents
> as belonging within the 32-bit time_t range but still allow changing
> that window using the "start-year" DT property to make it work
> past y2038. This work avoid most of the problems but still requires
> identifying exactly which drivers are affected.
>
> - Dropping CONFIG_RTC_HCTOSYS from the kernel. This is still
> Alexandre's long-term goal, but we cannot wait for that as users
> are trying to deploy y2038 compatible systems today.
>
> - Making CONFIG_RTC_HCTOSYS depend on !CONFIG_COMPAT_32BIT_TIME,
> which is a trivial way of avoiding both of the bugs, but breaks
> all general purpose 32-bit userland until they upgrade to
> a full time64 build or away from RTC_HCTOSYS.
>
> - Changing systemd to no longer crash on broken hardware, or
> ideally just set the system time itself to a value that is within
> its time_t range. This was previously rejected by the systemd
> developers, but is probably needed again with the b3a5ac42ab18
> patch reverted.
>
> Cc: Reinier Kuipers <[email protected]>
> Cc: Alexandre Belloni <[email protected]>
> Cc: Alessandro Zummo <[email protected]>
> Cc: [email protected]
> Cc: Russell King <[email protected]>
> Link: https://lore.kernel.org/all/CAKYb531CyL8XRVRcRN30cC3xRgsd-1FzXUeS7o2LiZqALJ42qw@mail.gmail.com/
> Link: https://github.com/systemd/systemd/issues/1143
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/rtc/class.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index e48223c00c67..572e356821b5 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -73,11 +73,31 @@ 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;
> - }
> +#ifdef CONFIG_COMPAT_32BIT_TIME
> + /*
> + * User space with 32-bit time_t behaves in unexpected ways when
> + * the time is set past January 19 2038, often leading to
> + * unbootable systems. With CONFIG_RTC_HCTOSYS, this can happen
> + * either because of broken RTC hardware that sets a random time
> + * with a depleted battery, or because it is actually the future.
> + *
> + * This can affect 32-bit kernels as well as 64-bit kernels when
> + * running a 32-bit rootfs or container.
> + *
> + * Since the kernel has no way of knowing what user space it runs,
> + * warn here whenever the kernel is able to run it.
> + * When CONFIG_COMPAT_32BIT_TIME is disabled, we know that the
> + * system is safe, but unfortunately this this is currently not
> + * supported by musl-1.2.x or most glibc based user space.

I was under the impression that musl never had a 32bit time_t nor used
the 32bit time APIs so it would not be affected by the bug.
So I guess the only affected userspace is glibc without _TIME_BITS=64

It doesn't change the fact that the kernel is not aware of any of that.

> + *
> + * Disabling CONFIG_RTC_HCTOSYS defers the problem to user space,
> + * which will work correctly when all of it supports 64-bit
> + * time_t, or will alternatively be unable to set a time past
> + * y2038 if it still uses 32-bit time_t.
> + */
> + dev_warn(rtc->dev.parent, "Warning: CONFIG_RTC_HCTOSYS is broken for 32-bit time_t userspace\n");
> + if (tv64.tv_sec > INT_MAX)
> + dev_warn(rtc->dev.parent, "Time set to past y2038 date\n");
> #endif
>
> err = do_settimeofday64(&tv64);
> --
> 2.29.2
>

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

2022-11-16 07:07:01

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] [RFC] rtc: y2038: remove broken RTC_HCTOSYS workaround

On Tue, Nov 15, 2022, at 23:08, Alexandre Belloni wrote:
> Hello,
>
> I'm fine with the patch and I'll probably take it,

Ok, thanks!

> I a an observation though:
>
> On 08/09/2022 13:53:20+0200, Arnd Bergmann wrote:
>> + *
>> + * Since the kernel has no way of knowing what user space it runs,
>> + * warn here whenever the kernel is able to run it.
>> + * When CONFIG_COMPAT_32BIT_TIME is disabled, we know that the
>> + * system is safe, but unfortunately this this is currently not
>> + * supported by musl-1.2.x or most glibc based user space.
>
> I was under the impression that musl never had a 32bit time_t nor used
> the 32bit time APIs so it would not be affected by the bug.
> So I guess the only affected userspace is glibc without _TIME_BITS=64

It's actually the opposite: while new versions of musl only allow
building applications against the time64 interfaces, musl itself
uses a mix of the time32 and time64 system calls, and the musl
maintainer considers turning CONFIG_COMPAT_32BIT_TIME off a
misfeature of the kernel that he does not want to support.

Arnd