linux-kernel - Re: [PATCH 000/100] rtc: remove cargo culted code

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:   Fri, 2 Mar 2018 09:56:54 +0100
From:   Alexandre Belloni <alexandre.belloni@...tlin.com>
To:     Steve Twiss <stwiss.opensource@...semi.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>
Subject: Re: [PATCH 000/100] rtc: remove cargo culted code

On 02/03/2018 at 08:46:48 +0000, Steve Twiss wrote:
> On Wed, Feb 21, 2018 at 8:54 PM, Alexandre Belloni wrote:
>  
> > subject:	[PATCH 000/100] rtc: remove cargo culted code
> > mailing list:	linux-kernel@...r.kernel.org Filter messages from this mailing list
> >
> > Hello,
> >
> > This series:
> >  - removes useless calls to rtc_valid_tm in .read_time, .set_time and
> >    .set_alarm
> >  - removes code setting default values for RTCs (and lets the core
> >    handle it)
> >  - removes useless "time is invalid" messages at probe time
> >  - removes useless indirect calls
> >
> > Those were mostly copy pasted from other drivers
> 
> Hi Alexandre,
> 
> Acked for:
>   rtc: da9063: stop validating rtc_time in .read_time
>   rtc: da9052: stop validating rtc_time in .read_time
>   rtc: da9055: stop validating rtc_time in .read_time
> 
> Acked-by: Steve Twiss <stwiss.opensource@...semi.com>
> 
> Agreed -- rtc_valid_tm() call is cargo cult for the above.
> 
> (By definition) for DA9063 I was trying to be rigorous.
> The .read_time function is slightly different here because I can make a copy the alarm time into the RTC time
> structure to solve an RTC synchronisation problem internally to the DA9063.
> https://elixir.bootlin.com/linux/v4.5.6/source/drivers/rtc/rtc-da9063.c#L253
> 
> But after some further looking, I have not got any explicit case of how the time read directly from the DA9063
> registers can be incorrectly represented. So there should be no need to check this.
> 

My point is that it is checked later in the core anyway so you end up
doing:

da9063_rtc_read_time()
	return rtc_valid_tm(tm);

__rtc_read_time()
	if (err < 0)
		return err;
	err = rtc_valid_tm(tm);

	return err;

So the check in da9063_rtc_read_time is always pointless.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com