linux-kernel - Re: [PATCH 12/51] rtc: coh901331: stop using rtc deprecated functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Jun 2017 22:21:53 +0100
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Benjamin Gaignard <benjamin.gaignard@...aro.org>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        linaro-kernel@...ts.linaro.org, rtc-linux@...glegroups.com,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-kernel@...r.kernel.org,
        Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 12/51] rtc: coh901331: stop using rtc deprecated functions

On Tue, Jun 20, 2017 at 11:35:20AM +0200, Benjamin Gaignard wrote:
> rtc_time_to_tm() and rtc_tm_to_time() are deprecated because they
> rely on 32bits variables and that will make rtc break in y2038/2016.
> Stop using those two functions to safer 64bits ones.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...aro.org>
> CC: Linus Walleij <linus.walleij@...aro.org>
> CC: Alessandro Zummo <a.zummo@...ertech.it>
> CC: Alexandre Belloni <alexandre.belloni@...e-electrons.com>
> CC: rtc-linux@...glegroups.com
> CC: linux-kernel@...r.kernel.org
> CC: linux-arm-kernel@...ts.infradead.org
> ---
>  drivers/rtc/rtc-coh901331.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-coh901331.c b/drivers/rtc/rtc-coh901331.c
> index cfc4141..5645011 100644
> --- a/drivers/rtc/rtc-coh901331.c
> +++ b/drivers/rtc/rtc-coh901331.c
> @@ -80,7 +80,8 @@ static int coh901331_read_time(struct device *dev, struct rtc_time *tm)
>  	clk_enable(rtap->clk);
>  	/* Check if the time is valid */
>  	if (readl(rtap->virtbase + COH901331_VALID)) {
> -		rtc_time_to_tm(readl(rtap->virtbase + COH901331_CUR_TIME), tm);
> +		rtc_time64_to_tm(
> +			(u64)readl(rtap->virtbase + COH901331_CUR_TIME), tm);
>  		clk_disable(rtap->clk);
>  		return rtc_valid_tm(tm);
>  	}
> @@ -88,7 +89,7 @@ static int coh901331_read_time(struct device *dev, struct rtc_time *tm)
>  	return -EINVAL;
>  }
>  
> -static int coh901331_set_mmss(struct device *dev, unsigned long secs)
> +static int coh901331_set_mmss64(struct device *dev, time64_t secs)

Do you realise how stupid this is?  Here, you're implicitly truncating
the 64-bit time value to 32-bit when you write it into the register.
So, when your clock wraps past 7 February 2106 (*not* 2038), when you
next read it, you read a date in 1970.

Exactly the same happens with the existing implementation, so this
fixes nothing at all.  As I've said in my other mail, these changes
make it harder to fix the problem, because you're stuck with that
truncation - you can never do anything but truncate it.

Keeping the 32-bit conversion functions allows us to wind the date
progressively forward if we so choose.

Sure, if we don't want to maintain two conversion functions, then we
can define the 32-bit conversion functions in terms of the 64-bit
versions, but do _not_ get rid of them.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.