NAK: [F][PATCH 0/1] rtc: class: support hctosys from modular RTC drivers

Thu Nov 25 15:48:15 UTC 2021

On Thu, Nov 25, 2021 at 2:49 PM Jesse Sung <jesse.sung at canonical.com> wrote:
>
> On Thu, Nov 25, 2021 at 9:14 PM Dimitri John Ledkov
> <dimitri.ledkov at canonical.com> wrote:
> >
> > Hi,
> >
> >
> > On Thu, Nov 25, 2021 at 10:09 AM Jesse Sung <jesse.sung at canonical.com> wrote:
> > >
> > > On Thu, Nov 25, 2021 at 12:00 AM Dimitri John Ledkov
> > > <dimitri.ledkov at canonical.com> wrote:
> > > >
> > > > But doesn't this break assumptions that systemd makes? I.e. it would
> > > > sync it's time from the timesync state file, and then loading rtc
> > > > module would trump this.
> > > >
> > > > This change in v5.7 kernel has caused breakage. Does introducing this
> > > > in v5.4 kernel going to break UC20 machines which sync a better/secure
> > > > time in the initrd; and then will get broken time from the rtc module
> > > > because it happens to not be battery backed. Leading to inability to
> > > > snap refresh?
> > >
> > > IMHO a proper solution for this is to have a way to add a list of
> > > modules when we use ubuntu-core-initramfs to generate the initrd.
> > > Something like /etc/initramfs-tools/modules in the classic image. For
> > > example:
> > > ubuntu-core-initram --extra-kernel-modules <mod1>,<mod2>,<mod3>
> > > So on one hand we don't need to make every RTC driver built-in, and on
> > > the other hand hctosys happens before systemd tries to access system
> > > time hence it acts like a built-in driver to systemd.
> > >
> >
> > That has not been true for a while now in recent updates of UC20.
>
> Humm... May I know which part has not been true?

"hence it acts like a built-in driver to systemd" => is not true.
That is because on UC20 systems, systemd is already running and has
changed clock to time-epoch, prior to any modular RTCs being available
from either initrd or rootfs.

On classic systems, with initramfs-tools there is a period of time
inside the initrd when systemd has not yet been executed, and modular
RTCs are available to be loaded.

>
> > ubuntu-core-initrd starts systemd inside the initrd, it looks up the
> > timestamp from /usr/lib/clock-epoch which is updated at every initrd
> > creation time and sets that as a system clock, if this timestamp is
> > older than the current system time.
> >
> > Then, snapd-bootstrap runs and reads & validates signed assertions. If
> > it finds any valid assertions with a newer timestamp than the current
> > time, it again updates the system clock.
> >
> > Thus all UC20 builds of recent enough kernels (Since around March
> > 2021) come up correctly with correct time of MAX ( initrd built time,
> > any assertions used by the image). Since implementing this feature we
> > have had no bug reports about UC20 machines failing to boot due to
> > wrong time, including from people using stock kernel, yet brand new
> > models/account-keys/etc.
>
> With all these checks and validations, how would reading the RTC at
> the very beginning of init scripts hurt UC20 systems even if RTC time
> is invalid? Shouldn't it be corrected by these later checks?
>
> > > >
> > > > Can you please explain what is prompting to backport this change into
> > > > v5.4 kernels?
> > >
> > > For systems which don't have Internet access but do have valid RTC
> > > time, if the RTC driver is a module, the RTC time will never be used
> > > and instead UC20 would start from a fixed time which causes problems
> > > in some projects.
> > >
> >
> > This sounds not true since Mar 2021. Given the above.
>
> Given the above, if a system doesn't have network access and has its
> RTC driver as a module, it would start its system time from "MAX (
> initrd built time, any assertions used by the image)" every time it
> boots until it gets a kernel snap update, even if there's valid RTC
> time, is that correct?
>
> > Strong NAK on applying this patch, given the core-initrd solutions
> > available since Mar 2021, which this patch will undo. Cause it will
> > reset time to random one, upon loading RTC module which may go back in
> > time to the past prior to initrd time-epoch & snapd assertions used to
> > boot.
>
> If a built-in RTC driver is not an issue, why would an RTC driver
> loaded as early as possible be one?

UC20 initrd does not use shell scripts as init in the initrd. It uses
systemd as init inside both initrd and the booted system.

built-in RTC drivers kick-in before executing intird's pid1 which is
systemd in UC20.

modular RTC drivers are loaded strictly after initrd's pid1 which is
systemd has started and moved time-epoch.

snap-bootstrap moving time based on assertions & loading of modular
RTC driver is racy with udevd starting and running cold plug.

HCTOSYS from modular RTC driver is racy because it overrides any time
that got sourced from time-epoch or snap-bootstrap without checking,
that it is updating the clock into the future.

when systemd sets time-eopoch or snap-bootstrap moves time it does
check that it is trying to push the clock into the future.

HCTOSYS from modular RTC can set the time into the past, breaking TLS
connectivity (if there is network) and breaking validating any
airgapped assertions (if there is no network).

It is literally an attack vector to brick the system.

If there are systems being enabled for UC20 with RTC modules, at the
moment they should be made built-in to be used safely & race free on
UC20.

>
> > The kernel RTC / hctosys must be fixed to not reset the clock into the
> > past; if it has already been adjusted by the userspace. At the moment
> > hctosys & userspace are racing each other, with userspace having
> > access to more time information than a battery-less RTC module.
>
> Kernel hctosys only happens on CONFIG_RTC_HCTOSYS_DEVICE, so for UC20
> systems as long as the specific driver gets loaded early it should be
> fine. Also it's not feasible to make every RTC driver built-in just
> for using the valid time in RTC.
> And the change doesn't seem to hurt Ubuntu desktop and server systems either.
>

There is no very early period of time for modular RTC on UC20, because
UC20 does not use shell scripts for init, it is using systemd inside
the initrd.

A race should be fixed in the kernel to prevent HCTOSYS from moving
time into the past. I have started working on that patch, and have a
draft, but cannot test it because i do not have access to any systems
with a modular RTC.

If you could grant me access to a classic system of any architecture
with a modular RTC that would be great. Cause then we could fix this
bug in upstream & our kernel, and then backport and enable modular
RTCs.

> Thanks,
> Jesse
>
> >
> >
> > > Thanks,
> > > Jesse
> > >
> > > >
> > > >
> > > > --
> > > > Regards,
> > > >
> > > > Dimitri.
> > > >
> > > > On Wed, Nov 24, 2021 at 11:01 AM Wen-chien Jesse Sung
> > > > <jesse.sung at canonical.com> wrote:
> > > > >
> > > > > BugLink: https://launchpad.net/bugs/1952077
> > > > >
> > > > > == Impact ==
> > > > > Although we have CONFIG_RTC_HCTOSYS enabled and set CONFIG_RTC_HCTOSYS_DEVICE
> > > > > to "rtc0", system time would only be set if the RTC driver is built-in. This
> > > > > may not be an issue on most amd64 systems because CONFIG_RTC_DRV_CMOS is y
> > > > > for amd64, however it is an issue for other architectures since there may not
> > > > > be a proper RTC driver enabled as built-in. Also making every RTC driver that
> > > > > may be used as built-in is not a good option.
> > > > >
> > > > > == Fix ==
> > > > > The commit is in mainline since v5.7:
> > > > >
> > > > > == Risk of Regression ==
> > > > > It's a clean cherry-pick for v5.4 and looks straight forward so risk should be low.
> > > > >
> > > > >
> > > > > Steve Muckle (1):
> > > > >   rtc: class: support hctosys from modular RTC drivers
> > > > >
> > > > >  drivers/rtc/Kconfig   |  3 --
> > > > >  drivers/rtc/Makefile  |  1 -
> > > > >  drivers/rtc/class.c   | 61 ++++++++++++++++++++++++++++++++++++++
> > > > >  drivers/rtc/hctosys.c | 69 -------------------------------------------
> > > > >  4 files changed, 61 insertions(+), 73 deletions(-)
> > > > >  delete mode 100644 drivers/rtc/hctosys.c
> > > > >
> > > > > --
> > > > > 2.25.1
> > > > >
> > > > >
> > > > > --
> > > > > kernel-team mailing list
> > > > > kernel-team at lists.ubuntu.com
> > > > > https://lists.ubuntu.com/mailman/listinfo/kernel-team