* [PATCH 1/2] include/linux/bcd.h: provide bcd_is_valid() helper
2022-11-23 9:55 [PATCH RESEND 0/2] rtc: rv8803 patches Sascha Hauer
@ 2022-11-23 9:55 ` Sascha Hauer
2022-11-23 9:55 ` [PATCH 2/2] rtc: rv8803: invalidate date/time if alarm time is invalid Sascha Hauer
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2022-11-23 9:55 UTC (permalink / raw)
To: linux-rtc
Cc: Alexandre Belloni, kernel, Alessandro Zummo, Ahmad Fatoum, Sascha Hauer
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
bcd2bin(0x0A) happily returns 10, despite this being an invalid BCD
value. RTC drivers converting possibly corrupted BCD timestamps might
want to validate their input before calling bcd2bin().
Provide a macro to do so. Unlike bcd2bin and bin2bcd, out-of-line
versions are not implemented. Should the macro experience enough use,
this can be retrofitted.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
include/linux/bcd.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/bcd.h b/include/linux/bcd.h
index 118bea36d7d49..abbc8149178e6 100644
--- a/include/linux/bcd.h
+++ b/include/linux/bcd.h
@@ -14,8 +14,12 @@
const_bin2bcd(x) : \
_bin2bcd(x))
+#define bcd_is_valid(x) \
+ const_bcd_is_valid(x)
+
#define const_bcd2bin(x) (((x) & 0x0f) + ((x) >> 4) * 10)
#define const_bin2bcd(x) ((((x) / 10) << 4) + (x) % 10)
+#define const_bcd_is_valid(x) (((x) & 0x0f) < 10 && ((x) >> 4) < 10)
unsigned _bcd2bin(unsigned char val) __attribute_const__;
unsigned char _bin2bcd(unsigned val) __attribute_const__;
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] rtc: rv8803: invalidate date/time if alarm time is invalid
2022-11-23 9:55 [PATCH RESEND 0/2] rtc: rv8803 patches Sascha Hauer
2022-11-23 9:55 ` [PATCH 1/2] include/linux/bcd.h: provide bcd_is_valid() helper Sascha Hauer
@ 2022-11-23 9:55 ` Sascha Hauer
2022-12-13 10:08 ` [PATCH RESEND 0/2] rtc: rv8803 patches Marc Kleine-Budde
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2022-11-23 9:55 UTC (permalink / raw)
To: linux-rtc
Cc: Alexandre Belloni, kernel, Alessandro Zummo, Sascha Hauer, Ahmad Fatoum
RTC core never calls rv8803_set_alarm with an invalid alarm time,
so if an invalid alarm time > 0 is set, external factors must have
corrupted the RTC's alarm time and possibly other registers.
Play it safe by marking the date/time invalid, so all registers are
reinitialized on a ->set_time.
This may cause existing setups to lose time if they so far set only
date/time, but ignored that the alarm registers had an invalid date
value, e.g.:
rtc rtc0: invalid alarm value: 2020-3-27 7:82:0
These systems will have their ->get_time return -EINVAL till
->set_time initializes the alarm value (and sets a new time).
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/rtc/rtc-rv8803.c | 45 +++++++++++++++++++++++++++++++++-------
1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 3527a0521e9b2..4875728014bed 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -70,6 +70,7 @@ struct rv8803_data {
struct mutex flags_lock;
u8 ctrl;
u8 backup;
+ u8 alarm_invalid:1;
enum rv8803_type type;
};
@@ -165,13 +166,13 @@ static int rv8803_regs_init(struct rv8803_data *rv8803)
static int rv8803_regs_configure(struct rv8803_data *rv8803);
-static int rv8803_regs_reset(struct rv8803_data *rv8803)
+static int rv8803_regs_reset(struct rv8803_data *rv8803, bool full)
{
/*
* The RV-8803 resets all registers to POR defaults after voltage-loss,
* the Epson RTCs don't, so we manually reset the remainder here.
*/
- if (rv8803->type == rx_8803 || rv8803->type == rx_8900) {
+ if (full || rv8803->type == rx_8803 || rv8803->type == rx_8900) {
int ret = rv8803_regs_init(rv8803);
if (ret)
return ret;
@@ -238,6 +239,11 @@ static int rv8803_get_time(struct device *dev, struct rtc_time *tm)
u8 *date = date1;
int ret, flags;
+ if (rv8803->alarm_invalid) {
+ dev_warn(dev, "Corruption detected, data may be invalid.\n");
+ return -EINVAL;
+ }
+
flags = rv8803_read_reg(rv8803->client, RV8803_FLAG);
if (flags < 0)
return flags;
@@ -313,12 +319,19 @@ static int rv8803_set_time(struct device *dev, struct rtc_time *tm)
return flags;
}
- if (flags & RV8803_FLAG_V2F) {
- ret = rv8803_regs_reset(rv8803);
+ if ((flags & RV8803_FLAG_V2F) || rv8803->alarm_invalid) {
+ /*
+ * If we sense corruption in the alarm registers, but see no
+ * voltage loss flag, we can't rely on other registers having
+ * sensible values. Reset them fully.
+ */
+ ret = rv8803_regs_reset(rv8803, rv8803->alarm_invalid);
if (ret) {
mutex_unlock(&rv8803->flags_lock);
return ret;
}
+
+ rv8803->alarm_invalid = false;
}
ret = rv8803_write_reg(rv8803->client, RV8803_FLAG,
@@ -344,15 +357,33 @@ static int rv8803_get_alarm(struct device *dev, struct rtc_wkalrm *alrm)
if (flags < 0)
return flags;
+ alarmvals[0] &= 0x7f;
+ alarmvals[1] &= 0x3f;
+ alarmvals[2] &= 0x3f;
+
+ if (!bcd_is_valid(alarmvals[0]) ||
+ !bcd_is_valid(alarmvals[1]) ||
+ !bcd_is_valid(alarmvals[2]))
+ goto err_invalid;
+
alrm->time.tm_sec = 0;
- alrm->time.tm_min = bcd2bin(alarmvals[0] & 0x7f);
- alrm->time.tm_hour = bcd2bin(alarmvals[1] & 0x3f);
- alrm->time.tm_mday = bcd2bin(alarmvals[2] & 0x3f);
+ alrm->time.tm_min = bcd2bin(alarmvals[0]);
+ alrm->time.tm_hour = bcd2bin(alarmvals[1]);
+ alrm->time.tm_mday = bcd2bin(alarmvals[2]);
alrm->enabled = !!(rv8803->ctrl & RV8803_CTRL_AIE);
alrm->pending = (flags & RV8803_FLAG_AF) && alrm->enabled;
+ if ((unsigned int)alrm->time.tm_mday > 31 ||
+ (unsigned int)alrm->time.tm_hour >= 24 ||
+ (unsigned int)alrm->time.tm_min >= 60)
+ goto err_invalid;
+
return 0;
+
+err_invalid:
+ rv8803->alarm_invalid = true;
+ return -EINVAL;
}
static int rv8803_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 0/2] rtc: rv8803 patches
2022-11-23 9:55 [PATCH RESEND 0/2] rtc: rv8803 patches Sascha Hauer
2022-11-23 9:55 ` [PATCH 1/2] include/linux/bcd.h: provide bcd_is_valid() helper Sascha Hauer
2022-11-23 9:55 ` [PATCH 2/2] rtc: rv8803: invalidate date/time if alarm time is invalid Sascha Hauer
@ 2022-12-13 10:08 ` Marc Kleine-Budde
2023-01-09 14:36 ` Marc Kleine-Budde
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2022-12-13 10:08 UTC (permalink / raw)
To: Sascha Hauer, Alexandre Belloni; +Cc: linux-rtc, kernel, Alessandro Zummo
[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]
Hey Alexandre,
On 23.11.2022 10:55:25, Sascha Hauer wrote:
> This series has the remainder of
> https://lore.kernel.org/all/20220426071056.1187235-1-s.hauer@pengutronix.de/
> which was partly applied.
>
> Alexandre,
>
> Last time this series was send you asked if this series fixes a problem
> we've really seen to which Ahmad answered:
>
> > The kernel message
> >
> > rtc rtc0: invalid alarm value: 2020-3-27 7:82:0
> >
> > listed in the commit message is something I actually ran into. There
> > was no v2f set then. The customer has also variously observed bit flips
> > independently of v2f: During EMC testing, electrostatic discharge at developer
> > desks and even in the field: Suspected causes were lightning strikes in the
> > vicinity and the switching of larger inductive loads.
> > They're very paranoid of logging invalid timestamps, so we'll keep the patch
> > anyhow at our side, but I think it is generally useful as well: If we can't
> > set an invalid alarm time by normal means, but read back an invalid time,
> > something may have corrupted other memory, so treating it as a v2f is sensible.
>
> There was no answer to this. I would be glad if you could take this
> series. I would understand though if you say that this problem is too
> esoteric to fix it upstream, we would keep the patches locally then.
> Please just say so, it would help me to get the problem from my desk ;)
Any news on this, what's the status?
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 0/2] rtc: rv8803 patches
2022-11-23 9:55 [PATCH RESEND 0/2] rtc: rv8803 patches Sascha Hauer
` (2 preceding siblings ...)
2022-12-13 10:08 ` [PATCH RESEND 0/2] rtc: rv8803 patches Marc Kleine-Budde
@ 2023-01-09 14:36 ` Marc Kleine-Budde
2023-01-31 8:19 ` Marc Kleine-Budde
2023-02-09 22:02 ` Alexandre Belloni
5 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-01-09 14:36 UTC (permalink / raw)
To: Sascha Hauer; +Cc: linux-rtc, Alexandre Belloni, kernel, Alessandro Zummo
[-- Attachment #1: Type: text/plain, Size: 698 bytes --]
On 23.11.2022 10:55:25, Sascha Hauer wrote:
> This series has the remainder of
> https://lore.kernel.org/all/20220426071056.1187235-1-s.hauer@pengutronix.de/
> which was partly applied.
>
> Alexandre,
>
> Last time this series was send you asked if this series fixes a problem
> we've really seen to which Ahmad answered:
Happy new Year Alexandre!
What can we do to bring this patch froward?
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 0/2] rtc: rv8803 patches
2022-11-23 9:55 [PATCH RESEND 0/2] rtc: rv8803 patches Sascha Hauer
` (3 preceding siblings ...)
2023-01-09 14:36 ` Marc Kleine-Budde
@ 2023-01-31 8:19 ` Marc Kleine-Budde
2023-01-31 12:00 ` Alexandre Belloni
2023-02-09 22:02 ` Alexandre Belloni
5 siblings, 1 reply; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-01-31 8:19 UTC (permalink / raw)
To: Sascha Hauer, linux-rtc, Alexandre Belloni, kernel, Alessandro Zummo
[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]
Hello Alexandre,
On 23.11.2022 10:55:25, Sascha Hauer wrote:
> This series has the remainder of
> https://lore.kernel.org/all/20220426071056.1187235-1-s.hauer@pengutronix.de/
> which was partly applied.
>
> Alexandre,
>
> Last time this series was send you asked if this series fixes a problem
> we've really seen to which Ahmad answered:
>
> > The kernel message
> >
> > rtc rtc0: invalid alarm value: 2020-3-27 7:82:0
> >
> > listed in the commit message is something I actually ran into. There
> > was no v2f set then. The customer has also variously observed bit flips
> > independently of v2f: During EMC testing, electrostatic discharge at developer
> > desks and even in the field: Suspected causes were lightning strikes in the
> > vicinity and the switching of larger inductive loads.
> > They're very paranoid of logging invalid timestamps, so we'll keep the patch
> > anyhow at our side, but I think it is generally useful as well: If we can't
> > set an invalid alarm time by normal means, but read back an invalid time,
> > something may have corrupted other memory, so treating it as a v2f is sensible.
>
> There was no answer to this. I would be glad if you could take this
> series. I would understand though if you say that this problem is too
> esoteric to fix it upstream, we would keep the patches locally then.
> Please just say so, it would help me to get the problem from my desk
> ;)
Can someone take this patch series? If not, what can we do to get these
changes upstream?
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 0/2] rtc: rv8803 patches
2023-01-31 8:19 ` Marc Kleine-Budde
@ 2023-01-31 12:00 ` Alexandre Belloni
2023-02-06 15:18 ` Marc Kleine-Budde
0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Belloni @ 2023-01-31 12:00 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Sascha Hauer, linux-rtc, kernel, Alessandro Zummo
On 31/01/2023 09:19:55+0100, Marc Kleine-Budde wrote:
> Hello Alexandre,
>
> On 23.11.2022 10:55:25, Sascha Hauer wrote:
> > This series has the remainder of
> > https://lore.kernel.org/all/20220426071056.1187235-1-s.hauer@pengutronix.de/
> > which was partly applied.
> >
> > Alexandre,
> >
> > Last time this series was send you asked if this series fixes a problem
> > we've really seen to which Ahmad answered:
> >
> > > The kernel message
> > >
> > > rtc rtc0: invalid alarm value: 2020-3-27 7:82:0
> > >
> > > listed in the commit message is something I actually ran into. There
> > > was no v2f set then. The customer has also variously observed bit flips
> > > independently of v2f: During EMC testing, electrostatic discharge at developer
> > > desks and even in the field: Suspected causes were lightning strikes in the
> > > vicinity and the switching of larger inductive loads.
> > > They're very paranoid of logging invalid timestamps, so we'll keep the patch
> > > anyhow at our side, but I think it is generally useful as well: If we can't
> > > set an invalid alarm time by normal means, but read back an invalid time,
> > > something may have corrupted other memory, so treating it as a v2f is sensible.
> >
> > There was no answer to this. I would be glad if you could take this
> > series. I would understand though if you say that this problem is too
> > esoteric to fix it upstream, we would keep the patches locally then.
> > Please just say so, it would help me to get the problem from my desk
> > ;)
>
> Can someone take this patch series? If not, what can we do to get these
> changes upstream?
I'm going to take it but this may silently break existing users with a
niche use case.
Also, this check will only happen at boot time so I'm not sure there is
a huge benefit, unless your customer reboots the platform often.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 0/2] rtc: rv8803 patches
2023-01-31 12:00 ` Alexandre Belloni
@ 2023-02-06 15:18 ` Marc Kleine-Budde
2023-02-09 22:03 ` Alexandre Belloni
0 siblings, 1 reply; 10+ messages in thread
From: Marc Kleine-Budde @ 2023-02-06 15:18 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: Sascha Hauer, linux-rtc, kernel, Alessandro Zummo
[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]
On 31.01.2023 13:00:12, Alexandre Belloni wrote:
> On 31/01/2023 09:19:55+0100, Marc Kleine-Budde wrote:
> > Hello Alexandre,
> >
> > On 23.11.2022 10:55:25, Sascha Hauer wrote:
> > > This series has the remainder of
> > > https://lore.kernel.org/all/20220426071056.1187235-1-s.hauer@pengutronix.de/
> > > which was partly applied.
> > >
> > > Alexandre,
> > >
> > > Last time this series was send you asked if this series fixes a problem
> > > we've really seen to which Ahmad answered:
> > >
> > > > The kernel message
> > > >
> > > > rtc rtc0: invalid alarm value: 2020-3-27 7:82:0
> > > >
> > > > listed in the commit message is something I actually ran into. There
> > > > was no v2f set then. The customer has also variously observed bit flips
> > > > independently of v2f: During EMC testing, electrostatic discharge at developer
> > > > desks and even in the field: Suspected causes were lightning strikes in the
> > > > vicinity and the switching of larger inductive loads.
> > > > They're very paranoid of logging invalid timestamps, so we'll keep the patch
> > > > anyhow at our side, but I think it is generally useful as well: If we can't
> > > > set an invalid alarm time by normal means, but read back an invalid time,
> > > > something may have corrupted other memory, so treating it as a v2f is sensible.
> > >
> > > There was no answer to this. I would be glad if you could take this
> > > series. I would understand though if you say that this problem is too
> > > esoteric to fix it upstream, we would keep the patches locally then.
> > > Please just say so, it would help me to get the problem from my desk
> > > ;)
> >
> > Can someone take this patch series? If not, what can we do to get these
> > changes upstream?
>
> I'm going to take it but this may silently break existing users with a
> niche use case.
> Also, this check will only happen at boot time so I'm not sure there is
> a huge benefit, unless your customer reboots the platform often.
Thanks Alexandre, is this patch already on an immutable branch?
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 0/2] rtc: rv8803 patches
2023-02-06 15:18 ` Marc Kleine-Budde
@ 2023-02-09 22:03 ` Alexandre Belloni
0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2023-02-09 22:03 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Sascha Hauer, linux-rtc, kernel, Alessandro Zummo
On 06/02/2023 16:18:24+0100, Marc Kleine-Budde wrote:
> On 31.01.2023 13:00:12, Alexandre Belloni wrote:
> > On 31/01/2023 09:19:55+0100, Marc Kleine-Budde wrote:
> > > Hello Alexandre,
> > >
> > > On 23.11.2022 10:55:25, Sascha Hauer wrote:
> > > > This series has the remainder of
> > > > https://lore.kernel.org/all/20220426071056.1187235-1-s.hauer@pengutronix.de/
> > > > which was partly applied.
> > > >
> > > > Alexandre,
> > > >
> > > > Last time this series was send you asked if this series fixes a problem
> > > > we've really seen to which Ahmad answered:
> > > >
> > > > > The kernel message
> > > > >
> > > > > rtc rtc0: invalid alarm value: 2020-3-27 7:82:0
> > > > >
> > > > > listed in the commit message is something I actually ran into. There
> > > > > was no v2f set then. The customer has also variously observed bit flips
> > > > > independently of v2f: During EMC testing, electrostatic discharge at developer
> > > > > desks and even in the field: Suspected causes were lightning strikes in the
> > > > > vicinity and the switching of larger inductive loads.
> > > > > They're very paranoid of logging invalid timestamps, so we'll keep the patch
> > > > > anyhow at our side, but I think it is generally useful as well: If we can't
> > > > > set an invalid alarm time by normal means, but read back an invalid time,
> > > > > something may have corrupted other memory, so treating it as a v2f is sensible.
> > > >
> > > > There was no answer to this. I would be glad if you could take this
> > > > series. I would understand though if you say that this problem is too
> > > > esoteric to fix it upstream, we would keep the patches locally then.
> > > > Please just say so, it would help me to get the problem from my desk
> > > > ;)
> > >
> > > Can someone take this patch series? If not, what can we do to get these
> > > changes upstream?
> >
> > I'm going to take it but this may silently break existing users with a
> > niche use case.
> > Also, this check will only happen at boot time so I'm not sure there is
> > a huge benefit, unless your customer reboots the platform often.
>
> Thanks Alexandre, is this patch already on an immutable branch?
It is not, I just pushed it to rtc-next now.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 0/2] rtc: rv8803 patches
2022-11-23 9:55 [PATCH RESEND 0/2] rtc: rv8803 patches Sascha Hauer
` (4 preceding siblings ...)
2023-01-31 8:19 ` Marc Kleine-Budde
@ 2023-02-09 22:02 ` Alexandre Belloni
5 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2023-02-09 22:02 UTC (permalink / raw)
To: linux-rtc, Sascha Hauer; +Cc: kernel, Alessandro Zummo
On Wed, 23 Nov 2022 10:55:25 +0100, Sascha Hauer wrote:
> This series has the remainder of
> https://lore.kernel.org/all/20220426071056.1187235-1-s.hauer@pengutronix.de/
> which was partly applied.
>
> Alexandre,
>
> Last time this series was send you asked if this series fixes a problem
> we've really seen to which Ahmad answered:
>
> [...]
Applied, thanks!
[1/2] include/linux/bcd.h: provide bcd_is_valid() helper
commit: 19409796578c879a41e88ddbdbce50c19457658d
[2/2] rtc: rv8803: invalidate date/time if alarm time is invalid
commit: e5c594233fcf1a55a439dec103aa815cdbf392a7
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread