linux-kernel - Re: [PATCH] x86 / hibernate: Use hlt_play_dead() when resuming from hibernation

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: <20160728193340.GC11657@amd>
Date:	Thu, 28 Jul 2016 21:33:40 +0200
From:	Pavel Machek <pavel@....cz>
To:	"Rafael J. Wysocki" <rafael@...nel.org>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Linux PM <linux-pm@...r.kernel.org>,
	the arch/x86 maintainers <x86@...nel.org>,
	Chen Yu <yu.c.chen@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...e.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Len Brown <lenb@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	James Morse <james.morse@....com>
Subject: Re: [PATCH] x86 / hibernate: Use hlt_play_dead() when resuming from
 hibernation

On Wed 2016-07-13 14:01:52, Rafael J. Wysocki wrote:
> On Wed, Jul 13, 2016 at 11:56 AM, Pavel Machek <pavel@....cz> wrote:
> > On Sun 2016-07-10 03:49:25, Rafael J. Wysocki wrote:
> >> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >>
> >> On Intel hardware, native_play_dead() uses mwait_play_dead() by
> >> default and only falls back to the other methods if that fails.
> >> That also happens during resume from hibernation, when the restore
> >> (boot) kernel runs disable_nonboot_cpus() to take all of the CPUs
> >> except for the boot one offline.
> >>
> >> However, that is problematic, because the address passed to
> >> __monitor() in mwait_play_dead() is likely to be written to in the
> >> last phase of hibernate image restoration and that causes the "dead"
> >> CPU to start executing instructions again.  Unfortunately, the page
> >> containing the address in that CPU's instruction pointer may not be
> >> valid any more at that point.
> >>
> >> First, that page may have been overwritten with image kernel memory
> >> contents already, so the instructions the CPU attempts to execute may
> >> simply be invalid.  Second, the page tables previously used by that
> >> CPU may have been overwritten by image kernel memory contents, so the
> >> address in its instruction pointer is impossible to resolve then.
> >>
> >> A report from Varun Koyyalagunta and investigation carried out by
> >> Chen Yu show that the latter sometimes happens in practice.
> >>
> >> To prevent it from happening, modify native_play_dead() to make
> >> it use hlt_play_dead() instead of mwait_play_dead() during resume
> >> from hibernation which avoids the inadvertent "revivals" of "dead"
> >> CPUs.
> >>
> >> A slightly unpleasant consequence of this change is that if the
> >> system is hibernated with one or more CPUs offline, it will generally
> >> draw more power after resume than it did before hibernation, because
> >> the physical state entered by CPUs via hlt_play_dead() is higher-power
> >> than the mwait_play_dead() one in the majority of cases.  It is
> >> possible to work around this, but it is unclear how much of a problem
> >> that's going to be in practice, so the workaround will be implemented
> >> later if it turns out to be necessary.
> >>
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=106371
> >> Reported-by: Varun Koyyalagunta <cpudebug@...ttech.com>
> >> Original-by: Chen Yu <yu.c.chen@...el.com>
> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > I notice that it changes even i386, where it should not be
> > neccessary. But we probably should switch i386 to support similar to
> > x86-64 one day (and I have patches) so no problem there.
> >
> > But I wonder if simpler solution is to place the mwait semaphore into
> > known address? (Nosave region comes to mind?)
> 
> It might work, but it wouldn't be simpler.
> 
> First off, we'd need to monitor a separate cache line for each CPU
> (see the message from Chen Yu) and it'd be a pain to guarantee that.
> Second, CPUs may be woken up from MWAIT for other reasons, so that
> needs to be taken into account too.
> 
> In principle, we might set up a MONITOR?MWAIT "play dead" loop in a
> safe page and make the "dead" CPUs jump to it during image restore,
> but then the image kernel (after getting control back) would need to
> migrate them away from there again, so doing the "halt" thing is *way*
> simpler than that.

Ok, it looks you have the best solution. Thanks...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html