Patchwork rtc: mv: reset date if after year 2038

login
register
mail settings
Submitter Thomas Petazzoni
Date Feb. 18, 2014, 1:26 p.m.
Message ID <1392729966-25394-1-git-send-email-thomas.petazzoni@free-electrons.com>
Download mbox | patch
Permalink /patch/321472/
State New
Headers show

Comments

Thomas Petazzoni - Feb. 18, 2014, 1:26 p.m.
Dates after January, 19th 2038 are badly handled by userspace due to
the time being stored on 32 bits. This causes issues on some Marvell
platform on which the RTC is initialized by default to a date that's
beyond 2038, causing a really weird behavior of the RTC.

In order to avoid that, reset the date to a sane value if the RTC is
beyond 2038.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/rtc/rtc-mv.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Baruch Siach - Feb. 18, 2014, 1:34 p.m.
Hi Thomas,

On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> Dates after January, 19th 2038 are badly handled by userspace due to
> the time being stored on 32 bits. This causes issues on some Marvell
> platform on which the RTC is initialized by default to a date that's
> beyond 2038, causing a really weird behavior of the RTC.
> 
> In order to avoid that, reset the date to a sane value if the RTC is
> beyond 2038.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/rtc/rtc-mv.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-mv.c b/drivers/rtc/rtc-mv.c
> index d536c59..f124dc6 100644
> --- a/drivers/rtc/rtc-mv.c
> +++ b/drivers/rtc/rtc-mv.c
> @@ -222,6 +222,7 @@ static int __init mv_rtc_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct rtc_plat_data *pdata;
>  	u32 rtc_time;
> +	u32 rtc_date;
>  	int ret = 0;
>  
>  	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> @@ -257,6 +258,17 @@ static int __init mv_rtc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/*
> +	 * A date after January 19th, 2038 does not fit on 32 bits and
> +	 * will confuse the kernel and userspace. Reset to a sane date
> +	 * (January 1st, 2013) if we're after 2038.
> +	 */
> +	rtc_date = readl(pdata->ioaddr + RTC_DATE_REG_OFFS);
> +	if (bcd2bin((rtc_date >> RTC_YEAR_OFFS) & 0xff) >= 38) {
> +		dev_info(&pdev->dev, "invalid RTC date, resetting to January, 1st 2013\n");

Misplaced comma?

> +		writel(0x130101, pdata->ioaddr + RTC_DATE_REG_OFFS);
> +	}
> +
>  	pdata->irq = platform_get_irq(pdev, 0);
>  
>  	platform_set_drvdata(pdev, pdata);
> -- 
> 1.8.3.2

baruch
Andrew Lunn - Feb. 18, 2014, 2:04 p.m.
On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> Dates after January, 19th 2038 are badly handled by userspace due to
> the time being stored on 32 bits. This causes issues on some Marvell
> platform on which the RTC is initialized by default to a date that's
> beyond 2038, causing a really weird behavior of the RTC.
> 
> In order to avoid that, reset the date to a sane value if the RTC is
> beyond 2038.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/rtc/rtc-mv.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-mv.c b/drivers/rtc/rtc-mv.c
> index d536c59..f124dc6 100644
> --- a/drivers/rtc/rtc-mv.c
> +++ b/drivers/rtc/rtc-mv.c
> @@ -222,6 +222,7 @@ static int __init mv_rtc_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct rtc_plat_data *pdata;
>  	u32 rtc_time;
> +	u32 rtc_date;
>  	int ret = 0;
>  
>  	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> @@ -257,6 +258,17 @@ static int __init mv_rtc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/*
> +	 * A date after January 19th, 2038 does not fit on 32 bits and
> +	 * will confuse the kernel and userspace. Reset to a sane date
> +	 * (January 1st, 2013) if we're after 2038.
> +	 */

Hi Thomas

Would it be better to reset back to 01/01/1970? When we do reach 32
bit rollover, and assuming the world continues to exist, it has a
better chance of being right than 01/01/2013.

       Andrew
Jason - Feb. 18, 2014, 2:15 p.m.
On Tue, Feb 18, 2014 at 03:04:29PM +0100, Andrew Lunn wrote:
> On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> > Dates after January, 19th 2038 are badly handled by userspace due to
> > the time being stored on 32 bits. This causes issues on some Marvell
> > platform on which the RTC is initialized by default to a date that's
> > beyond 2038, causing a really weird behavior of the RTC.
> > 
> > In order to avoid that, reset the date to a sane value if the RTC is
> > beyond 2038.
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > ---
> >  drivers/rtc/rtc-mv.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/rtc/rtc-mv.c b/drivers/rtc/rtc-mv.c
> > index d536c59..f124dc6 100644
> > --- a/drivers/rtc/rtc-mv.c
> > +++ b/drivers/rtc/rtc-mv.c
> > @@ -222,6 +222,7 @@ static int __init mv_rtc_probe(struct platform_device *pdev)
> >  	struct resource *res;
> >  	struct rtc_plat_data *pdata;
> >  	u32 rtc_time;
> > +	u32 rtc_date;
> >  	int ret = 0;
> >  
> >  	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> > @@ -257,6 +258,17 @@ static int __init mv_rtc_probe(struct platform_device *pdev)
> >  		}
> >  	}
> >  
> > +	/*
> > +	 * A date after January 19th, 2038 does not fit on 32 bits and
> > +	 * will confuse the kernel and userspace. Reset to a sane date
> > +	 * (January 1st, 2013) if we're after 2038.
> > +	 */
> 
> Hi Thomas
> 
> Would it be better to reset back to 01/01/1970? When we do reach 32
> bit rollover, and assuming the world continues to exist, it has a
> better chance of being right than 01/01/2013.

The real issue here is that you don't want the clock to go *backwards*
when ntpdate/ntpd starts up.  That causes all kinds of badness (for
dhcp and friends).

So, by definition, any date _before_ the correct time is better than
being in the future.  Since the code can't be executed before it's
written, any date between 1/1/1970 and now is acceptable.

Not all ntpd configurations allow a large initial offset (jump to $now)
before disciplining the clock.  So the closer we can be to the user's
$now, the better.  In this case, the date chosen in the patch is a lot
better than 1/1/1970.

thx,

Jason.
Thomas Petazzoni - Feb. 18, 2014, 2:16 p.m.
Dear Andrew Lunn,

On Tue, 18 Feb 2014 15:04:29 +0100, Andrew Lunn wrote:

> > +	/*
> > +	 * A date after January 19th, 2038 does not fit on 32 bits and
> > +	 * will confuse the kernel and userspace. Reset to a sane date
> > +	 * (January 1st, 2013) if we're after 2038.
> > +	 */
> 
> Hi Thomas
> 
> Would it be better to reset back to 01/01/1970? When we do reach 32
> bit rollover, and assuming the world continues to exist, it has a
> better chance of being right than 01/01/2013.

I must say I don't really care which "sane date" we use for the RTC,
but I don't understand your argument.

Why would 01/01/1970 be more "right" than any other random date? If
the RTC has a date past 2^32, then we have absolutely no way of
converting this date into something sane that fits under 2^32, so
setting either 01/01/1970, 01/01/2013, or 12/11/2025 is just about the
same. And actually 01/01/2013 has a much higher chance of being closer
to the right date, because the boards I've seen affected by this
problem are the new Armada 375/38x boards, which have been manufactured
starting in 2013.

That being said, I don't care about which date is chosen. Just let me
know if you want me to resend the patch with a reset to 01/01/1970.

Thanks!

Thomas
Andrew Lunn - Feb. 18, 2014, 2:53 p.m.
On Tue, Feb 18, 2014 at 03:16:56PM +0100, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Tue, 18 Feb 2014 15:04:29 +0100, Andrew Lunn wrote:
> 
> > > +	/*
> > > +	 * A date after January 19th, 2038 does not fit on 32 bits and
> > > +	 * will confuse the kernel and userspace. Reset to a sane date
> > > +	 * (January 1st, 2013) if we're after 2038.
> > > +	 */
> > 
> > Hi Thomas
> > 
> > Would it be better to reset back to 01/01/1970? When we do reach 32
> > bit rollover, and assuming the world continues to exist, it has a
> > better chance of being right than 01/01/2013.
> 
> I must say I don't really care which "sane date" we use for the RTC,
> but I don't understand your argument.

I've no idea what the plans are for January 19th, 2038, but one
possible solution is to move the epoch to January 19th, 2038. And with
any luck, an RTC returning 01/01/1970 will get mapped to January 19th,
2038, and hence give the correct date.

      Andrew
Jason - Feb. 18, 2014, 2:58 p.m.
On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> Dates after January, 19th 2038 are badly handled by userspace due to
> the time being stored on 32 bits. This causes issues on some Marvell
> platform on which the RTC is initialized by default to a date that's
> beyond 2038, causing a really weird behavior of the RTC.
> 
> In order to avoid that, reset the date to a sane value if the RTC is
> beyond 2038.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/rtc/rtc-mv.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

Jason.
Josh Cartwright - Feb. 18, 2014, 7:11 p.m.
On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> Dates after January, 19th 2038 are badly handled by userspace due to
> the time being stored on 32 bits. This causes issues on some Marvell
> platform on which the RTC is initialized by default to a date that's
> beyond 2038, causing a really weird behavior of the RTC.
> 
> In order to avoid that, reset the date to a sane value if the RTC is
> beyond 2038.

Just so I better understand: is this really a problem that is unique to
this particular RTC?  It smells a bit like we're papering over a problem
that may exist for other RTCs as well, and if so, is better solved in
the core.

Thanks,
  Josh
Thomas Petazzoni - Feb. 18, 2014, 11:14 p.m.
Dear Josh Cartwright,

On Tue, 18 Feb 2014 13:11:11 -0600, Josh Cartwright wrote:
> On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> > Dates after January, 19th 2038 are badly handled by userspace due to
> > the time being stored on 32 bits. This causes issues on some Marvell
> > platform on which the RTC is initialized by default to a date that's
> > beyond 2038, causing a really weird behavior of the RTC.
> > 
> > In order to avoid that, reset the date to a sane value if the RTC is
> > beyond 2038.
> 
> Just so I better understand: is this really a problem that is unique to
> this particular RTC?  It smells a bit like we're papering over a problem
> that may exist for other RTCs as well, and if so, is better solved in
> the core.

I'd say it depends on how the RTC internally encodes the date. Some RTC
may have an internal date representation that allows to represent dates
past 2^32 seconds after Epoch, while maybe some RTC do not.

However, it is true that a fairly large number of RTC drivers seem to
use the bcd2bin() function, which indicates the RTC internally uses a
BCD representation, which allows to represent dates past 2^32 seconds
after Epoch.

That being said, I don't think the RTC core has any knowledge of what
the internal RTC representation of time is, so I don't know how the
core could fix things up without this information.

Thomas
Josh Cartwright - Feb. 19, 2014, 4:50 p.m.
On Wed, Feb 19, 2014 at 12:14:16AM +0100, Thomas Petazzoni wrote:
> On Tue, 18 Feb 2014 13:11:11 -0600, Josh Cartwright wrote:
> > On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> > > Dates after January, 19th 2038 are badly handled by userspace due to
> > > the time being stored on 32 bits. This causes issues on some Marvell
> > > platform on which the RTC is initialized by default to a date that's
> > > beyond 2038, causing a really weird behavior of the RTC.
> > >
> > > In order to avoid that, reset the date to a sane value if the RTC is
> > > beyond 2038.
>
> > Just so I better understand: is this really a problem that is unique to
> > this particular RTC?  It smells a bit like we're papering over a problem
> > that may exist for other RTCs as well, and if so, is better solved in
> > the core.
>
> I'd say it depends on how the RTC internally encodes the date. Some RTC
> may have an internal date representation that allows to represent dates
> past 2^32 seconds after Epoch, while maybe some RTC do not.
>
> However, it is true that a fairly large number of RTC drivers seem to
> use the bcd2bin() function, which indicates the RTC internally uses a
> BCD representation, which allows to represent dates past 2^32 seconds
> after Epoch.
>
> That being said, I don't think the RTC core has any knowledge of what
> the internal RTC representation of time is, so I don't know how the
> core could fix things up without this information.

Yeah, I agree that with the current state of affairs, the RTC core
doesn't have enough information to be managing this in a central way.
I'm wondering if the RTC core should provide a richer interface for a
driver to provide bounds information, and let the core figure out these
problems.  But that's a question for Alessandro :).

Anyway, could you clarify where the 32-bit assumption is currently being
made?  From my cursory glance at the ioctl/sysfs interfaces, the use of
struct rtc_time looks safe.  Is the real problem in usermode?  What is
"weird behavior" in this case?

Thanks,
  Josh
Jason - Feb. 21, 2014, 11:41 p.m.
On Tue, Feb 18, 2014 at 02:26:06PM +0100, Thomas Petazzoni wrote:
> Dates after January, 19th 2038 are badly handled by userspace due to
> the time being stored on 32 bits. This causes issues on some Marvell
> platform on which the RTC is initialized by default to a date that's
> beyond 2038, causing a really weird behavior of the RTC.
> 
> In order to avoid that, reset the date to a sane value if the RTC is
> beyond 2038.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/rtc/rtc-mv.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Applied to mvebu/drivers with Baruch's grammar fix.

thx,

Jason.

Patch

diff --git a/drivers/rtc/rtc-mv.c b/drivers/rtc/rtc-mv.c
index d536c59..f124dc6 100644
--- a/drivers/rtc/rtc-mv.c
+++ b/drivers/rtc/rtc-mv.c
@@ -222,6 +222,7 @@  static int __init mv_rtc_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct rtc_plat_data *pdata;
 	u32 rtc_time;
+	u32 rtc_date;
 	int ret = 0;
 
 	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
@@ -257,6 +258,17 @@  static int __init mv_rtc_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * A date after January 19th, 2038 does not fit on 32 bits and
+	 * will confuse the kernel and userspace. Reset to a sane date
+	 * (January 1st, 2013) if we're after 2038.
+	 */
+	rtc_date = readl(pdata->ioaddr + RTC_DATE_REG_OFFS);
+	if (bcd2bin((rtc_date >> RTC_YEAR_OFFS) & 0xff) >= 38) {
+		dev_info(&pdev->dev, "invalid RTC date, resetting to January, 1st 2013\n");
+		writel(0x130101, pdata->ioaddr + RTC_DATE_REG_OFFS);
+	}
+
 	pdata->irq = platform_get_irq(pdev, 0);
 
 	platform_set_drvdata(pdev, pdata);