2021-06-14 11:42:12

by Zeh, Werner

[permalink] [raw]
Subject: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

The timekeeper is synchronized with the CMOS RTC when it is initialized.
If the RTC buffering is bad (not buffered at all, empty battery) the RTC
registers can contain random data. In order to avoid date and time
being completely rubbish check the sanity of the registers before
calling mktime64. If the values are not valid, set tv_sec to 0 so that
at least the starting time is valid.

Signed-off-by: Werner Zeh <[email protected]>
---
[resent due to wrong lkml address]
arch/x86/kernel/rtc.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 586f718b8e95..f4af7b18c6c0 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -9,6 +9,7 @@
#include <linux/export.h>
#include <linux/pnp.h>
#include <linux/of.h>
+#include <linux/rtc.h>

#include <asm/vsyscall.h>
#include <asm/x86_init.h>
@@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64 *now)
{
unsigned int status, year, mon, day, hour, min, sec, century = 0;
unsigned long flags;
+ struct rtc_time tm = {0};

/*
* If pm_trace abused the RTC as storage, set the timespec to 0,
@@ -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64 *now)
} else
year += CMOS_YEARS_OFFS;

- now->tv_sec = mktime64(year, mon, day, hour, min, sec);
+ tm.tm_sec = sec;
+ tm.tm_min = min;
+ tm.tm_hour = hour;
+ tm.tm_mday = day;
+ tm.tm_mon = mon;
+ tm.tm_year = year;
+ now->tv_sec = 0;
+ if (rtc_valid_tm(&tm) == 0)
+ now->tv_sec = mktime64(year, mon, day, hour, min, sec);
now->tv_nsec = 0;
}

--
2.21.3


2021-06-24 08:19:09

by Zeh, Werner

[permalink] [raw]
Subject: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

The timekeeper is synchronized with the CMOS RTC when it is initialized.
If the RTC buffering is bad (not buffered at all, empty battery) the RTC
registers can contain random data. In order to avoid date and time
being completely rubbish check the sanity of the registers before
calling mktime64. If the values are not valid, set tv_sec to 0 so that
at least the starting time is valid.

Signed-off-by: Werner Zeh <[email protected]>
---
[resent due to wrong lkml address]
[added RTC maintainers to the recipients]
This change introduces the same validity check that is already done in
drivers/rtc/interface.c. If it is not done here, the timekeeper can be
set up wrongly in the first run and won't be corrected once the RTC driver
is started because the validity check in the RTC driver drops the time and
date due to invalid entries.

arch/x86/kernel/rtc.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 586f718b8e95..f4af7b18c6c0 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -9,6 +9,7 @@
#include <linux/export.h>
#include <linux/pnp.h>
#include <linux/of.h>
+#include <linux/rtc.h>

#include <asm/vsyscall.h>
#include <asm/x86_init.h>
@@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64 *now)
{
unsigned int status, year, mon, day, hour, min, sec, century = 0;
unsigned long flags;
+ struct rtc_time tm = {0};

/*
* If pm_trace abused the RTC as storage, set the timespec to 0,
@@ -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64 *now)
} else
year += CMOS_YEARS_OFFS;

- now->tv_sec = mktime64(year, mon, day, hour, min, sec);
+ tm.tm_sec = sec;
+ tm.tm_min = min;
+ tm.tm_hour = hour;
+ tm.tm_mday = day;
+ tm.tm_mon = mon;
+ tm.tm_year = year;
+ now->tv_sec = 0;
+ if (rtc_valid_tm(&tm) == 0)
+ now->tv_sec = mktime64(year, mon, day, hour, min, sec);
now->tv_nsec = 0;
}

--
2.21.3

2021-06-28 23:38:58

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

Hello,

On 24/06/2021 10:15:07+0200, Werner Zeh wrote:
> The timekeeper is synchronized with the CMOS RTC when it is initialized.
> If the RTC buffering is bad (not buffered at all, empty battery) the RTC
> registers can contain random data. In order to avoid date and time
> being completely rubbish check the sanity of the registers before
> calling mktime64. If the values are not valid, set tv_sec to 0 so that
> at least the starting time is valid.
>
> Signed-off-by: Werner Zeh <[email protected]>
> ---
> [resent due to wrong lkml address]
> [added RTC maintainers to the recipients]
> This change introduces the same validity check that is already done in
> drivers/rtc/interface.c. If it is not done here, the timekeeper can be
> set up wrongly in the first run and won't be corrected once the RTC driver
> is started because the validity check in the RTC driver drops the time and
> date due to invalid entries.
>
> arch/x86/kernel/rtc.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
> index 586f718b8e95..f4af7b18c6c0 100644
> --- a/arch/x86/kernel/rtc.c
> +++ b/arch/x86/kernel/rtc.c
> @@ -9,6 +9,7 @@
> #include <linux/export.h>
> #include <linux/pnp.h>
> #include <linux/of.h>
> +#include <linux/rtc.h>
>
> #include <asm/vsyscall.h>
> #include <asm/x86_init.h>
> @@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64 *now)
> {
> unsigned int status, year, mon, day, hour, min, sec, century = 0;
> unsigned long flags;
> + struct rtc_time tm = {0};
>
> /*
> * If pm_trace abused the RTC as storage, set the timespec to 0,
> @@ -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64 *now)
> } else
> year += CMOS_YEARS_OFFS;
>
> - now->tv_sec = mktime64(year, mon, day, hour, min, sec);
> + tm.tm_sec = sec;
> + tm.tm_min = min;
> + tm.tm_hour = hour;
> + tm.tm_mday = day;
> + tm.tm_mon = mon;
> + tm.tm_year = year;
> + now->tv_sec = 0;
> + if (rtc_valid_tm(&tm) == 0)

Doesn't that make the x86 architecture depend on CONFIG_RTC_LIB?


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

2021-06-30 06:27:20

by Zeh, Werner

[permalink] [raw]
Subject: AW: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

Hi Alexandre

> Hello,
>
> On 24/06/2021 10:15:07+0200, Werner Zeh wrote:
> > The timekeeper is synchronized with the CMOS RTC when it is initialized.
> > If the RTC buffering is bad (not buffered at all, empty battery) the
> > RTC registers can contain random data. In order to avoid date and time
> > being completely rubbish check the sanity of the registers before
> > calling mktime64. If the values are not valid, set tv_sec to 0 so that
> > at least the starting time is valid.
> >
> > Signed-off-by: Werner Zeh <[email protected]>
> > ---
> > [resent due to wrong lkml address]
> > [added RTC maintainers to the recipients] This change introduces the
> > same validity check that is already done in drivers/rtc/interface.c.
> > If it is not done here, the timekeeper can be set up wrongly in the
> > first run and won't be corrected once the RTC driver is started
> > because the validity check in the RTC driver drops the time and date
> > due to invalid entries.
> >
> > arch/x86/kernel/rtc.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index
> > 586f718b8e95..f4af7b18c6c0 100644
> > --- a/arch/x86/kernel/rtc.c
> > +++ b/arch/x86/kernel/rtc.c
> > @@ -9,6 +9,7 @@
> > #include <linux/export.h>
> > #include <linux/pnp.h>
> > #include <linux/of.h>
> > +#include <linux/rtc.h>
> >
> > #include <asm/vsyscall.h>
> > #include <asm/x86_init.h>
> > @@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64 *now)
> {
> > unsigned int status, year, mon, day, hour, min, sec, century = 0;
> > unsigned long flags;
> > + struct rtc_time tm = {0};
> >
> > /*
> > * If pm_trace abused the RTC as storage, set the timespec to 0, @@
> > -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64 *now)
> > } else
> > year += CMOS_YEARS_OFFS;
> >
> > - now->tv_sec = mktime64(year, mon, day, hour, min, sec);
> > + tm.tm_sec = sec;
> > + tm.tm_min = min;
> > + tm.tm_hour = hour;
> > + tm.tm_mday = day;
> > + tm.tm_mon = mon;
> > + tm.tm_year = year;
> > + now->tv_sec = 0;
> > + if (rtc_valid_tm(&tm) == 0)
>
> Doesn't that make the x86 architecture depend on CONFIG_RTC_LIB?
>
CONFIG_RTC_LIB is already default enabled for x86, see arch/x86/Kconfig.
Do you have any other dependencies in mind I have overseen?

Werner

2021-07-02 14:13:28

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

On 30/06/2021 06:25:44+0000, Zeh, Werner wrote:
> Hi Alexandre
>
> > Hello,
> >
> > On 24/06/2021 10:15:07+0200, Werner Zeh wrote:
> > > The timekeeper is synchronized with the CMOS RTC when it is initialized.
> > > If the RTC buffering is bad (not buffered at all, empty battery) the
> > > RTC registers can contain random data. In order to avoid date and time
> > > being completely rubbish check the sanity of the registers before
> > > calling mktime64. If the values are not valid, set tv_sec to 0 so that
> > > at least the starting time is valid.
> > >
> > > Signed-off-by: Werner Zeh <[email protected]>
> > > ---
> > > [resent due to wrong lkml address]
> > > [added RTC maintainers to the recipients] This change introduces the
> > > same validity check that is already done in drivers/rtc/interface.c.
> > > If it is not done here, the timekeeper can be set up wrongly in the
> > > first run and won't be corrected once the RTC driver is started
> > > because the validity check in the RTC driver drops the time and date
> > > due to invalid entries.
> > >
> > > arch/x86/kernel/rtc.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index
> > > 586f718b8e95..f4af7b18c6c0 100644
> > > --- a/arch/x86/kernel/rtc.c
> > > +++ b/arch/x86/kernel/rtc.c
> > > @@ -9,6 +9,7 @@
> > > #include <linux/export.h>
> > > #include <linux/pnp.h>
> > > #include <linux/of.h>
> > > +#include <linux/rtc.h>
> > >
> > > #include <asm/vsyscall.h>
> > > #include <asm/x86_init.h>
> > > @@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64 *now)
> > {
> > > unsigned int status, year, mon, day, hour, min, sec, century = 0;
> > > unsigned long flags;
> > > + struct rtc_time tm = {0};
> > >
> > > /*
> > > * If pm_trace abused the RTC as storage, set the timespec to 0, @@
> > > -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64 *now)
> > > } else
> > > year += CMOS_YEARS_OFFS;
> > >
> > > - now->tv_sec = mktime64(year, mon, day, hour, min, sec);
> > > + tm.tm_sec = sec;
> > > + tm.tm_min = min;
> > > + tm.tm_hour = hour;
> > > + tm.tm_mday = day;
> > > + tm.tm_mon = mon;
> > > + tm.tm_year = year;
> > > + now->tv_sec = 0;
> > > + if (rtc_valid_tm(&tm) == 0)
> >
> > Doesn't that make the x86 architecture depend on CONFIG_RTC_LIB?
> >
> CONFIG_RTC_LIB is already default enabled for x86, see arch/x86/Kconfig.
> Do you have any other dependencies in mind I have overseen?
>

Nope, everything is fine, it would be better if we could get rid of
mach_get_cmos_time but I don't have any clue as to why this is
necessary.


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

2021-12-09 08:05:17

by Zeh, Werner

[permalink] [raw]
Subject: RE: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

Hi Alexandre.

Is there anything more I can do for that patch in order to get some process on it?
Or why is this patch stuck for a long time?

Thanks
Werner

> -----Original Message-----
> From: Alexandre Belloni <[email protected]>
> Sent: Friday, July 2, 2021 4:11 PM
> To: Zeh, Werner (DI MC MTS SP HW 1) <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and
> time
>
> On 30/06/2021 06:25:44+0000, Zeh, Werner wrote:
> > Hi Alexandre
> >
> > > Hello,
> > >
> > > On 24/06/2021 10:15:07+0200, Werner Zeh wrote:
> > > > The timekeeper is synchronized with the CMOS RTC when it is
> initialized.
> > > > If the RTC buffering is bad (not buffered at all, empty battery)
> > > > the RTC registers can contain random data. In order to avoid date
> > > > and time being completely rubbish check the sanity of the
> > > > registers before calling mktime64. If the values are not valid,
> > > > set tv_sec to 0 so that at least the starting time is valid.
> > > >
> > > > Signed-off-by: Werner Zeh <[email protected]>
> > > > ---
> > > > [resent due to wrong lkml address] [added RTC maintainers to the
> > > > recipients] This change introduces the same validity check that is
> > > > already done in drivers/rtc/interface.c.
> > > > If it is not done here, the timekeeper can be set up wrongly in
> > > > the first run and won't be corrected once the RTC driver is
> > > > started because the validity check in the RTC driver drops the
> > > > time and date due to invalid entries.
> > > >
> > > > arch/x86/kernel/rtc.c | 12 +++++++++++-
> > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index
> > > > 586f718b8e95..f4af7b18c6c0 100644
> > > > --- a/arch/x86/kernel/rtc.c
> > > > +++ b/arch/x86/kernel/rtc.c
> > > > @@ -9,6 +9,7 @@
> > > > #include <linux/export.h>
> > > > #include <linux/pnp.h>
> > > > #include <linux/of.h>
> > > > +#include <linux/rtc.h>
> > > >
> > > > #include <asm/vsyscall.h>
> > > > #include <asm/x86_init.h>
> > > > @@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64
> *now)
> > > {
> > > > unsigned int status, year, mon, day, hour, min, sec, century = 0;
> > > > unsigned long flags;
> > > > + struct rtc_time tm = {0};
> > > >
> > > > /*
> > > > * If pm_trace abused the RTC as storage, set the timespec to 0,
> > > > @@
> > > > -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64
> *now)
> > > > } else
> > > > year += CMOS_YEARS_OFFS;
> > > >
> > > > - now->tv_sec = mktime64(year, mon, day, hour, min, sec);
> > > > + tm.tm_sec = sec;
> > > > + tm.tm_min = min;
> > > > + tm.tm_hour = hour;
> > > > + tm.tm_mday = day;
> > > > + tm.tm_mon = mon;
> > > > + tm.tm_year = year;
> > > > + now->tv_sec = 0;
> > > > + if (rtc_valid_tm(&tm) == 0)
> > >
> > > Doesn't that make the x86 architecture depend on CONFIG_RTC_LIB?
> > >
> > CONFIG_RTC_LIB is already default enabled for x86, see arch/x86/Kconfig.
> > Do you have any other dependencies in mind I have overseen?
> >
>
> Nope, everything is fine, it would be better if we could get rid of
> mach_get_cmos_time but I don't have any clue as to why this is necessary.
>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> engineering
> https://bootlin.com/


2021-12-09 08:50:25

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

Hello,

On 09/12/2021 08:05:10+0000, Zeh, Werner wrote:
> Hi Alexandre.
>
> Is there anything more I can do for that patch in order to get some process on it?
> Or why is this patch stuck for a long time?
>

I'm not the maintainer for that part of the kernel, I expect this to go
through the x86 tree.

> Thanks
> Werner
>
> > -----Original Message-----
> > From: Alexandre Belloni <[email protected]>
> > Sent: Friday, July 2, 2021 4:11 PM
> > To: Zeh, Werner (DI MC MTS SP HW 1) <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and
> > time
> >
> > On 30/06/2021 06:25:44+0000, Zeh, Werner wrote:
> > > Hi Alexandre
> > >
> > > > Hello,
> > > >
> > > > On 24/06/2021 10:15:07+0200, Werner Zeh wrote:
> > > > > The timekeeper is synchronized with the CMOS RTC when it is
> > initialized.
> > > > > If the RTC buffering is bad (not buffered at all, empty battery)
> > > > > the RTC registers can contain random data. In order to avoid date
> > > > > and time being completely rubbish check the sanity of the
> > > > > registers before calling mktime64. If the values are not valid,
> > > > > set tv_sec to 0 so that at least the starting time is valid.
> > > > >
> > > > > Signed-off-by: Werner Zeh <[email protected]>
> > > > > ---
> > > > > [resent due to wrong lkml address] [added RTC maintainers to the
> > > > > recipients] This change introduces the same validity check that is
> > > > > already done in drivers/rtc/interface.c.
> > > > > If it is not done here, the timekeeper can be set up wrongly in
> > > > > the first run and won't be corrected once the RTC driver is
> > > > > started because the validity check in the RTC driver drops the
> > > > > time and date due to invalid entries.
> > > > >
> > > > > arch/x86/kernel/rtc.c | 12 +++++++++++-
> > > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index
> > > > > 586f718b8e95..f4af7b18c6c0 100644
> > > > > --- a/arch/x86/kernel/rtc.c
> > > > > +++ b/arch/x86/kernel/rtc.c
> > > > > @@ -9,6 +9,7 @@
> > > > > #include <linux/export.h>
> > > > > #include <linux/pnp.h>
> > > > > #include <linux/of.h>
> > > > > +#include <linux/rtc.h>
> > > > >
> > > > > #include <asm/vsyscall.h>
> > > > > #include <asm/x86_init.h>
> > > > > @@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64
> > *now)
> > > > {
> > > > > unsigned int status, year, mon, day, hour, min, sec, century = 0;
> > > > > unsigned long flags;
> > > > > + struct rtc_time tm = {0};
> > > > >
> > > > > /*
> > > > > * If pm_trace abused the RTC as storage, set the timespec to 0,
> > > > > @@
> > > > > -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64
> > *now)
> > > > > } else
> > > > > year += CMOS_YEARS_OFFS;
> > > > >
> > > > > - now->tv_sec = mktime64(year, mon, day, hour, min, sec);
> > > > > + tm.tm_sec = sec;
> > > > > + tm.tm_min = min;
> > > > > + tm.tm_hour = hour;
> > > > > + tm.tm_mday = day;
> > > > > + tm.tm_mon = mon;
> > > > > + tm.tm_year = year;
> > > > > + now->tv_sec = 0;
> > > > > + if (rtc_valid_tm(&tm) == 0)
> > > >
> > > > Doesn't that make the x86 architecture depend on CONFIG_RTC_LIB?
> > > >
> > > CONFIG_RTC_LIB is already default enabled for x86, see arch/x86/Kconfig.
> > > Do you have any other dependencies in mind I have overseen?
> > >
> >
> > Nope, everything is fine, it would be better if we could get rid of
> > mach_get_cmos_time but I don't have any clue as to why this is necessary.
> >
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> > engineering
> > https://bootlin.com/
>

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

2021-12-09 09:07:51

by Zeh, Werner

[permalink] [raw]
Subject: RE: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

> I'm not the maintainer for that part of the kernel, I expect this to go through
> the x86 tree.

OK, understood. Thank you.
Any hint whom I can contact directly in this regard?
I had a hard time to debug this issue and it would be a pity if it will not make it in possibly causing issues for other users.

Werner

> -----Original Message-----
> From: Alexandre Belloni <[email protected]>
> Sent: Thursday, December 9, 2021 9:50 AM
> To: Zeh, Werner (DI MC MTS SP HW 1) <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and
> time
>
> Hello,
>
> On 09/12/2021 08:05:10+0000, Zeh, Werner wrote:
> > Hi Alexandre.
> >
> > Is there anything more I can do for that patch in order to get some process
> on it?
> > Or why is this patch stuck for a long time?
> >
>
> I'm not the maintainer for that part of the kernel, I expect this to go through
> the x86 tree.
>
> > Thanks
> > Werner
> >
> > > -----Original Message-----
> > > From: Alexandre Belloni <[email protected]>
> > > Sent: Friday, July 2, 2021 4:11 PM
> > > To: Zeh, Werner (DI MC MTS SP HW 1) <[email protected]>
> > > Cc: [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected]
> > > Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC
> > > date and time
> > >
> > > On 30/06/2021 06:25:44+0000, Zeh, Werner wrote:
> > > > Hi Alexandre
> > > >
> > > > > Hello,
> > > > >
> > > > > On 24/06/2021 10:15:07+0200, Werner Zeh wrote:
> > > > > > The timekeeper is synchronized with the CMOS RTC when it is
> > > initialized.
> > > > > > If the RTC buffering is bad (not buffered at all, empty
> > > > > > battery) the RTC registers can contain random data. In order
> > > > > > to avoid date and time being completely rubbish check the
> > > > > > sanity of the registers before calling mktime64. If the values
> > > > > > are not valid, set tv_sec to 0 so that at least the starting time is valid.
> > > > > >
> > > > > > Signed-off-by: Werner Zeh <[email protected]>
> > > > > > ---
> > > > > > [resent due to wrong lkml address] [added RTC maintainers to
> > > > > > the recipients] This change introduces the same validity check
> > > > > > that is already done in drivers/rtc/interface.c.
> > > > > > If it is not done here, the timekeeper can be set up wrongly
> > > > > > in the first run and won't be corrected once the RTC driver is
> > > > > > started because the validity check in the RTC driver drops the
> > > > > > time and date due to invalid entries.
> > > > > >
> > > > > > arch/x86/kernel/rtc.c | 12 +++++++++++-
> > > > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
> > > > > > index
> > > > > > 586f718b8e95..f4af7b18c6c0 100644
> > > > > > --- a/arch/x86/kernel/rtc.c
> > > > > > +++ b/arch/x86/kernel/rtc.c
> > > > > > @@ -9,6 +9,7 @@
> > > > > > #include <linux/export.h>
> > > > > > #include <linux/pnp.h>
> > > > > > #include <linux/of.h>
> > > > > > +#include <linux/rtc.h>
> > > > > >
> > > > > > #include <asm/vsyscall.h>
> > > > > > #include <asm/x86_init.h>
> > > > > > @@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64
> > > *now)
> > > > > {
> > > > > > unsigned int status, year, mon, day, hour, min, sec, century = 0;
> > > > > > unsigned long flags;
> > > > > > + struct rtc_time tm = {0};
> > > > > >
> > > > > > /*
> > > > > > * If pm_trace abused the RTC as storage, set the
> > > > > > timespec to 0, @@
> > > > > > -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64
> > > *now)
> > > > > > } else
> > > > > > year += CMOS_YEARS_OFFS;
> > > > > >
> > > > > > - now->tv_sec = mktime64(year, mon, day, hour, min, sec);
> > > > > > + tm.tm_sec = sec;
> > > > > > + tm.tm_min = min;
> > > > > > + tm.tm_hour = hour;
> > > > > > + tm.tm_mday = day;
> > > > > > + tm.tm_mon = mon;
> > > > > > + tm.tm_year = year;
> > > > > > + now->tv_sec = 0;
> > > > > > + if (rtc_valid_tm(&tm) == 0)
> > > > >
> > > > > Doesn't that make the x86 architecture depend on CONFIG_RTC_LIB?
> > > > >
> > > > CONFIG_RTC_LIB is already default enabled for x86, see
> arch/x86/Kconfig.
> > > > Do you have any other dependencies in mind I have overseen?
> > > >
> > >
> > > Nope, everything is fine, it would be better if we could get rid of
> > > mach_get_cmos_time but I don't have any clue as to why this is
> necessary.
> > >
> > >
> > > --
> > > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and
> > > Kernel engineering
> > >
> https://bootlin.com
> >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> engineering
> https://bootlin.com

2021-12-17 21:53:25

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and time

On 09/12/2021 09:07:46+0000, Zeh, Werner wrote:
> > I'm not the maintainer for that part of the kernel, I expect this to go through
> > the x86 tree.
>
> OK, understood. Thank you.
> Any hint whom I can contact directly in this regard?
> I had a hard time to debug this issue and it would be a pity if it will not make it in possibly causing issues for other users.
>

Well, tglx and mingo are in copy of the thread. You can probably resend
with my:

Acked-by: Alexandre Belloni <[email protected]>

to get their attention.

> Werner
>
> > -----Original Message-----
> > From: Alexandre Belloni <[email protected]>
> > Sent: Thursday, December 9, 2021 9:50 AM
> > To: Zeh, Werner (DI MC MTS SP HW 1) <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC date and
> > time
> >
> > Hello,
> >
> > On 09/12/2021 08:05:10+0000, Zeh, Werner wrote:
> > > Hi Alexandre.
> > >
> > > Is there anything more I can do for that patch in order to get some process
> > on it?
> > > Or why is this patch stuck for a long time?
> > >
> >
> > I'm not the maintainer for that part of the kernel, I expect this to go through
> > the x86 tree.
> >
> > > Thanks
> > > Werner
> > >
> > > > -----Original Message-----
> > > > From: Alexandre Belloni <[email protected]>
> > > > Sent: Friday, July 2, 2021 4:11 PM
> > > > To: Zeh, Werner (DI MC MTS SP HW 1) <[email protected]>
> > > > Cc: [email protected]; [email protected]; [email protected];
> > > > [email protected]; [email protected]; [email protected]
> > > > Subject: Re: [PATCH 1/1] x86/kernel/rtc: add sanity check for RTC
> > > > date and time
> > > >
> > > > On 30/06/2021 06:25:44+0000, Zeh, Werner wrote:
> > > > > Hi Alexandre
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > On 24/06/2021 10:15:07+0200, Werner Zeh wrote:
> > > > > > > The timekeeper is synchronized with the CMOS RTC when it is
> > > > initialized.
> > > > > > > If the RTC buffering is bad (not buffered at all, empty
> > > > > > > battery) the RTC registers can contain random data. In order
> > > > > > > to avoid date and time being completely rubbish check the
> > > > > > > sanity of the registers before calling mktime64. If the values
> > > > > > > are not valid, set tv_sec to 0 so that at least the starting time is valid.
> > > > > > >
> > > > > > > Signed-off-by: Werner Zeh <[email protected]>
> > > > > > > ---
> > > > > > > [resent due to wrong lkml address] [added RTC maintainers to
> > > > > > > the recipients] This change introduces the same validity check
> > > > > > > that is already done in drivers/rtc/interface.c.
> > > > > > > If it is not done here, the timekeeper can be set up wrongly
> > > > > > > in the first run and won't be corrected once the RTC driver is
> > > > > > > started because the validity check in the RTC driver drops the
> > > > > > > time and date due to invalid entries.
> > > > > > >
> > > > > > > arch/x86/kernel/rtc.c | 12 +++++++++++-
> > > > > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
> > > > > > > index
> > > > > > > 586f718b8e95..f4af7b18c6c0 100644
> > > > > > > --- a/arch/x86/kernel/rtc.c
> > > > > > > +++ b/arch/x86/kernel/rtc.c
> > > > > > > @@ -9,6 +9,7 @@
> > > > > > > #include <linux/export.h>
> > > > > > > #include <linux/pnp.h>
> > > > > > > #include <linux/of.h>
> > > > > > > +#include <linux/rtc.h>
> > > > > > >
> > > > > > > #include <asm/vsyscall.h>
> > > > > > > #include <asm/x86_init.h>
> > > > > > > @@ -64,6 +65,7 @@ void mach_get_cmos_time(struct timespec64
> > > > *now)
> > > > > > {
> > > > > > > unsigned int status, year, mon, day, hour, min, sec, century = 0;
> > > > > > > unsigned long flags;
> > > > > > > + struct rtc_time tm = {0};
> > > > > > >
> > > > > > > /*
> > > > > > > * If pm_trace abused the RTC as storage, set the
> > > > > > > timespec to 0, @@
> > > > > > > -118,7 +120,15 @@ void mach_get_cmos_time(struct timespec64
> > > > *now)
> > > > > > > } else
> > > > > > > year += CMOS_YEARS_OFFS;
> > > > > > >
> > > > > > > - now->tv_sec = mktime64(year, mon, day, hour, min, sec);
> > > > > > > + tm.tm_sec = sec;
> > > > > > > + tm.tm_min = min;
> > > > > > > + tm.tm_hour = hour;
> > > > > > > + tm.tm_mday = day;
> > > > > > > + tm.tm_mon = mon;
> > > > > > > + tm.tm_year = year;
> > > > > > > + now->tv_sec = 0;
> > > > > > > + if (rtc_valid_tm(&tm) == 0)
> > > > > >
> > > > > > Doesn't that make the x86 architecture depend on CONFIG_RTC_LIB?
> > > > > >
> > > > > CONFIG_RTC_LIB is already default enabled for x86, see
> > arch/x86/Kconfig.
> > > > > Do you have any other dependencies in mind I have overseen?
> > > > >
> > > >
> > > > Nope, everything is fine, it would be better if we could get rid of
> > > > mach_get_cmos_time but I don't have any clue as to why this is
> > necessary.
> > > >
> > > >
> > > > --
> > > > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and
> > > > Kernel engineering
> > > >
> > https://bootlin.com
> > >
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> > engineering
> > https://bootlin.com

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