Message-ID: <1518490.AMlP2tjrQi@vostro.rjw.lan>
Date:	Tue, 14 Jun 2016 00:15 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Kees Cook <keescook@...omium.org>
Cc:	Linux PM list <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	Ingo Molnar <mingo@...nel.org>,
	Logan Gunthorpe <logang@...tatee.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	Andy Lutomirski <luto@...nel.org>,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH] x86 / hibernate: Fix 64-bit code passing control to image kernel

On Monday, June 13, 2016 02:58:57 PM Kees Cook wrote:
> On Mon, Jun 13, 2016 at 6:42 AM, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > Logan Gunthorpe reports that hibernation stopped working reliably for
> > him after commit ab76f7b4ab23 (x86/mm: Set NX on gap between __ex_table
> > and rodata).  Most likely, what happens is that the page containing
> > the image kernel's entry point is sometimes marked as non-executable
> > in the page tables used at the time of the final jump to the image
> > kernel.  That at least is why commit ab76f7b4ab23 may matter.
> >
> > However, there is one more long-standing issue with the code in
> > question, which is that the temporary page tables set up by it
> > to avoid page tables corruption when the last bits of the image
> > kernel's memory contents are copied into their original page frames
> > re-use the boot kernel's text mapping, but that mapping may very
> > well get corrupted just like any other part of the page tables.
> > Of course, if that happens, the final jump to the image kernel's
> > entry point will go to nowhere.
> >
> > As it turns out, those two issues may be addressed simultaneously.
> >
> > To that end, note that the code copying the last bits of the image
> > kernel's memory contents to the page frames occupied by them
> > previoulsy doesn't use the kernel text mapping, because it runs from
> > a special page covered by the identity mapping set up for that code
> > from scratch.  Hence, the kernel text mapping is only needed before
> > that code starts to run and then it will only be used just for the
> > final jump to the image kernel's entry point.
> >
> > Accordingly, the temporary page tables set up in swsusp_arch_resume()
> > on x86-64 can re-use the boot kernel's text mapping to start with,
> > but after all of the image kernel's memory contents are in place,
> > that mapping has to be replaced with a new one that will allow the
> > final jump to the image kernel's entry point to succeed.  Of course,
> > since the first thing the image kernel does after getting control back
> > is to switch over to its own original page tables, the new kernel text
> > mapping only has to cover the image kernel's entry point (along with
> > some following bytes).  Moreover, it has to do that so the virtual
> > address of the image kernel's entry point before the jump is the same
> > as the one mapped by the image kernel's page tables.
> >
> > With that in mind, modify the x86-64's arch_hibernation_header_save()
> > and arch_hibernation_header_restore() routines to pass the physical
> > address of the image kernel's entry point (in addition to its virtual
> > address) to the boot kernel (a small piece of assembly code involved
> > in passing the entry point's virtual address to the image kernel is
> > not necessary any more after that, so drop it).  Update RESTORE_MAGIC
> > too to reflect the image header format change.
> >
> > Next, in set_up_temporary_mappings(), use the physical and virtual
> > addresses of the image kernel's entry point passed in the image
> > header to set up a minimum kernel text mapping (using memory pages
> > that won't be overwritten by the image kernel's memory contents) that
> > will map those addresses to each other as appropriate.  Do not use
> > that mapping immediately, though.  Instead, use the original boot
> > kernel text mapping to start with and switch over to the new one
> > after all of the image kernel's memory has been restored, right
> > before the final jump to the image kernel's entry point.
> >
> > This makes the concern about the possible corruption of the original
> > boot kernel text mapping go away and if the the minimum kernel text
> > mapping used for the final jump marks the image kernel's entry point
> > memory as executable, the jump to it is guaraneed to succeed.
> >
> > Fixes: ab76f7b4ab23 (x86/mm: Set NX on gap between __ex_table and rodata)
> > Link: http://marc.info/?l=linux-pm&m=146372852823760&w=2
> > Reported-by: Logan Gunthorpe <logang@...tatee.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> Acked-by: Kees Cook <keescook@...omium.org>
> 
> And as an awesome added benefit: this fixes KASLR hibernation for me,
> too!

Interesting. :-)

Is there documentation I can read about how the KASLR thing works exactly,
wrt page tables in particular?

> I will send a follow-up patch that removes all the KASLR vs
> hibernation hacks.

But that on x86-64 only?  32-bit still doesn't work I suppose?

> Yay!

I'm glad you like it. ;-)

Cheers,
Rafael

Powered by blists - more mailing lists