linux-kernel - Re: [PATCH] reboot: Backup orderly

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] [day] [month] [year] [list]

Message-ID: <5695D1AE.3010802@ti.com>
Date:	Wed, 13 Jan 2016 09:55:18 +0530
From:	Keerthy <a0393675@...com>
To:	Josh Triplett <josh@...htriplett.org>, Keerthy <j-keerthy@...com>
CC:	<linux-kernel@...r.kernel.org>, <edubezval@...il.com>,
	<grygorii.strashko@...com>, <nm@...com>, <joel@....id.au>,
	<akpm@...ux-foundation.org>, <mingo@...nel.org>,
	<peterz@...radead.org>, <dyoung@...hat.com>, <mpe@...erman.id.au>
Subject: Re: [PATCH] reboot: Backup orderly_poweroff



On Monday 11 January 2016 08:25 PM, Josh Triplett wrote:
> On Mon, Jan 11, 2016 at 03:53:24PM +0530, Keerthy wrote:
>> orderly_poweroff is triggered when a graceful shutdown
>> of system is desired. This may be used in many critical states of the
>> kernel such as when subsystems detects conditions such as critical
>> temperature conditions. However, in certain conditions in system
>> boot up sequences like those in the middle of driver probes being
>> initiated, userspace will be unable to power off the system in a clean
>> manner and leaves the system in a critical state. In cases like these,
>> the /sbin/poweroff will return success (having forked off to attempt
>> powering off the system. However, the system overall will fail to
>> completely poweroff (since other modules will be probed) and the system
>> is still functional with no userspace (since that would have shut itself
>> off).
>>
>> However, there is no clean way of detecting such failure of userspace
>> powering off the system. In such scenarios, it is necessary for a backup
>> workqueue to be able to force a shutdown of the system when orderly
>> shutdown is not successful after a configurable time period.
>
> One issue below, inline.
>
>> Signed-off-by: Keerthy <j-keerthy@...com>
>> Suggested-by: Eduardo Valentin <edubezval@...il.com>
>> Reported-by: Nishanth Menon <nm@...com>
>> ---
>> Links to previous discussion can be found here:
>>
>> http://www.spinics.net/lists/linux-omap/msg124925.html
>>
>> Boot tested on DRA7.
>>
>>   arch/Kconfig    |  7 +++++++
>>   kernel/reboot.c | 23 ++++++++++++++++++-----
>>   2 files changed, 25 insertions(+), 5 deletions(-)
>>
>> Index: linux/arch/Kconfig
>> ===================================================================
>> --- linux.orig/arch/Kconfig	2016-01-11 15:26:07.732173131 +0530
>> +++ linux/arch/Kconfig	2016-01-11 15:26:07.728173205 +0530
>> @@ -37,6 +37,18 @@
>>   	def_bool y
>>   	depends on PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !PPC64
>>
>> +config SHUTDOWN_BACKUP_DELAY_MS
>> +	int "Backup shutdown delay in milli-seconds"
>> +	default 0
>
> With no dependencies, as far as I can tell this will always get defined
> with a value, defaulting to 0.  (I don't know of any special cases in
> Kconfig for an int value of 0.)  Thus, in the code below...

Yes. I will fix this and post a new version. Thanks for the review.

Regards,
Keerthy
>
>> --- linux.orig/kernel/reboot.c	2016-01-11 15:26:07.732173131 +0530
>> +++ linux/kernel/reboot.c	2016-01-11 15:38:33.502341511 +0530
>> @@ -424,6 +424,38 @@
>>   	return ret;
>>   }
>>
>> +#ifdef CONFIG_SHUTDOWN_BACKUP_DELAY_MS
>
> ...this should use #if, not #ifdef.  Otherwise this code will get
> compiled into every kernel.
>
> - Josh Triplett
>