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 <arnd@xxxxxxxx>
>
> 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 <kuipers.reinier@xxxxxxxxx>
> Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>
> Cc: linux-rtc@xxxxxxxxxxxxxxx
> Cc: Russell King <linux@xxxxxxxxxxxxxxx>
> Link: https://lore.kernel.org/all/CAKYb531CyL8XRVRcRN30cC3xRgsd-1FzXUeS7o2LiZqALJ42qw@xxxxxxxxxxxxxx/
> Link: https://github.com/systemd/systemd/issues/1143
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> 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
