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 for Android: free password hash cracker in your pocket

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

Message-ID: <alpine.LFD.2.02.1105060124440.3005@ionos>
Date:	Fri, 6 May 2011 02:23:27 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	john stultz <johnstul@...ibm.com>
cc:	lkml <linux-kernel@...r.kernel.org>,
	Wolfram Sang <w.sang@...gutronix.de>
Subject: Re: [GIT pull] rtc fixes for 2.6.39

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.

And you can deduce something important here:

    While the patch itself is correct, the changelog shows that the
    fix is just mechanical and not backed by the required semantical
    understanding of the problem.

It does not matter whether you fix it yourself or preferrbably push
back hard on folks who send you obvious crappy changelogs. That
depends on the situation and the person you're dealing with.

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().

Thanks,

	tglx
--
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/