linux-kernel - Re: [RFC][PATCH] misc: Introduce reboot

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, 8 Dec 2015 14:15:10 -0800
From:	Bjorn Andersson <bjorn.andersson@...ymobile.com>
To:	Arnd Bergmann <arnd@...db.de>
CC:	John Stultz <john.stultz@...aro.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Vinay Simha BN <simhavcs@...il.com>,
	Haojian Zhuang <haojian.zhuang@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [RFC][PATCH] misc: Introduce reboot_reason driver

On Tue 08 Dec 13:52 PST 2015, Arnd Bergmann wrote:

> On Tuesday 08 December 2015 13:29:22 John Stultz wrote:
> 
[..]
> > +static int reboot_reason(struct notifier_block *nb, unsigned long action,
> > +								void *data)
> > +{
> > +	char *cmd = (char *)data;
> > +	long reason = reasons[NONE];
> > +
> > +	if (!reboot_reason_addr)
> > +		return NOTIFY_DONE;
> > +
> > +	if (cmd != NULL) {
> > +		if (!strncmp(cmd, "bootloader", 10))
> > +			reason = reasons[BOOTLOADER];
> > +		else if (!strncmp(cmd, "recovery", 8))
> > +			reason = reasons[RECOVERY];
> > +		else if (!strncmp(cmd, "oem-", 4)) {
> > +			unsigned long code;
> > +
> > +			if (!kstrtoul(cmd+4, 0, &code))
> > +				reason = reasons[OEM] | (code & 0xff);
> > +		}
> > +	}
> > +
> > +	if (reason != -1)
> > +		writel(reason, reboot_reason_addr);
> > +	return NOTIFY_DONE;
> > +}
> 
> Will this reboot the machine?
> 

It will store the magic in IMEM, that is then read by the boot loader
after the reboot to make it trigger alternative boot flows - e.g. the
recovery/update path or enter fastboot mode.

> > +	/* Install the notifier */
> > +	restart_nb.notifier_call = reboot_reason;
> > +	restart_nb.priority = 256;
> > +	if (register_restart_handler(&restart_nb)) {
> 
> If not, you should use register_reboot_notifier() rather than
> register_restart_handler(). The former gets called to do something
> just before rebooting, while the latter attempts to actually reboot
> the machine.
> 

It should be a reboot_notifier, sorry for missing this in my answer.

> > +static int __init reboot_reason_init(void)
> > +{
> > +	return platform_driver_register(&reboot_reason_driver);
> > +}
> > +arch_initcall(reboot_reason_init);
> 
> Why this early? If it can be a normal device_initcall, you can use
> module_platform_driver().
> 

Not represented in this patch is that several Android vendors uses this
mechanism to communicate a panic() to the boot loader; to let the system
enter some sort of memory dump state.

Adding support for that would be convenient and mandate the early,
otherwise it's just userspace that will trigger this so module would be
fine.

Regards,
Bjorn
--
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/