linux-kernel - Re: [GIT pull] rtc fixes for 2.6.39

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]

Message-ID: <1304648319.20980.100.camel@work-vm>
Date:	Thu, 05 May 2011 19:18:39 -0700
From:	john stultz <johnstul@...ibm.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Wolfram Sang <w.sang@...gutronix.de>
Subject: Re: [GIT pull] rtc fixes for 2.6.39

On Fri, 2011-05-06 at 02:23 +0200, Thomas Gleixner wrote:
> On Thu, 5 May 2011, john stultz wrote:
> 
> > Hey Thomas,
> > 	Here are some urgent 2.6.39 boot fixups from Wolfram for RTC devices
> > that register themselves before they are finished initializing.
> > 
> > They are available in the git repository at:
> > 
> >   git://git.linaro.org/people/jstultz/linux.git fortglx/39/tip/timers/rtc
> > 
> > Wolfram Sang (3):
> >       rtc: mxc: fix crash on boot
> >       rtc: davinci: fix crash on boot
> >       rtc: ep93xx: fix crash on boot
> 
> No. I'm not pulling that. The changelogs are completely ass backwards.
> 
>     rtc: $machine: fix crash on boot
> 
> "fix crash on boot" is equally informative as "Fix bug". It's not
> informative at all, it's just sloppy.
> 
>     rtc: $machine: Initialize driver data before registering device
> 
> would tell me: Yes, that's an obvious late -rc fix.
> 
>     Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") caused a
>     crash when initializing the driver.
> 
> That's totally misleading. The reality is that the stupid drivers
> registered their device _BEFORE_ completing the initialization of the
> setup and that commit unearthed that. So not the commit caused the
> problem, the problem was there forever and was not noticed because the
> register code did not touch any of the non initialized data up to that
> point.
> 
>     The reason is that rtc_device_register() calls rtc_read_alarm() after
>     that change, when the driver does not have all driver data set up yet.
> 
> And that's just the continuation of the distorted reality.
> 
> The reason is that the f*cking driver did register the device _BEFORE_
> initializing the relevant data structures and that commit unearthed
> the crap.
> 
> It does _NOT_ matter at all whether this worked before by chance. The
> ordering is and _WAS_ always completely wrong.

Yea. Sorry about this. I got caught up in the "A change I made keeps
systems from booting" view of it and rushed it a bit.

I'll work with Wolfram to get the subject/commit messages cleaned up.

> As a side note, this is not the first fallout of the RTC rework which
> unearthed these wrong order initialization problems. The obvious
> reaction on the first "fix ordering problems" patch should have been
> to add
> 
> 	if (driver data == NULL) {
> 		printk("Fix your crap\n");
> 		return -EMORON;}
> 	}
> 
> to rtc_device_register().

Well, unfortunately the various rtc drivers use the drvdata differently.
Some store indirection structures (which are the problematic case),
while others just store the rtc pointer returned from
rtc_device_register.

Regardless, I'll be going through doing an audit by hand to try to make
sure all the rtc drivers are looking correct.

thanks
-john

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/