| Message ID | 20220908115337.1604277-1-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] rtc: y2038: remove broken RTC_HCTOSYS workaround | expand |
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@arndb.de> > > 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@gmail.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Alessandro Zummo <a.zummo@towertech.it> > Cc: linux-rtc@vger.kernel.org > Cc: Russell King <linux@armlinux.org.uk> > Link: https://lore.kernel.org/all/CAKYb531CyL8XRVRcRN30cC3xRgsd-1FzXUeS7o2LiZqALJ42qw@mail.gmail.com/ > Link: https://github.com/systemd/systemd/issues/1143 > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > 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 >
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
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);