linux-kernel - Re: PROBLEM: Resume form hibernate broken by setting NX on gap

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: <19310985.zBnH00loSf@vostro.rjw.lan>
Date:	Sat, 11 Jun 2016 02:13:45 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Logan Gunthorpe <logang@...tatee.com>,
	Kees Cook <keescook@...omium.org>
Cc:	"Rafael J. Wysocki" <rafael@...nel.org>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	Ingo Molnar <mingo@...nel.org>, Ingo Molnar <mingo@...hat.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andy Lutomirski <luto@...nel.org>,
	Borislav Petkov <bp@...en8.de>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Brian Gerst <brgerst@...il.com>
Subject: Re: PROBLEM: Resume form hibernate broken by setting NX on gap

On Saturday, June 11, 2016 12:33:31 AM Rafael J. Wysocki wrote:
> On Friday, June 10, 2016 04:28:01 PM Logan Gunthorpe wrote:
> > 
> > On 10/06/16 04:29 PM, Rafael J. Wysocki wrote:
> > > OK, I have a theory, but I need a bit of help.
> > > 
> > > This may be a dumb question, but I don't quite remember the answer readily.
> > > 
> > > Given a physical address, how do I get the corresponding virtual one under
> > > the kernel identity mapping?
> > 
> > Is that not just 'phys_to_virt(x)'??
> 
> Ah that.  Thanks!

So first, appended is a cleanup of the 64-bit hibernate/restore code that hopefully
will make it a bit clearer what happens there.

In particular, it follows from it quite clearly that the restore will only work
if the virtual address of the trampoline (&restore_registers) in the image
kernel happens to match the virtual address of the same chunk of memory in the
boot kernel (and after it has switched to the temporary page tables).

That appears to be the case most of the time, or hibernation would randomly
break for people all over, but I'm not actually sure if that's guaranteed to
happen.  If not, that very well may explain unexpected failures in there.

If it is not guaranteed to happen, the solution would be to pass the physical
address of that memory from the image kernel to the boot kernel instead of its
virtual address, but simply applying virt_to_phys() to &restore_registers in
arch_hibernation_header_save(), storing that in rdr->jump_address and then applying
phys_to_virt() to rdr->jump_address in arch_hibernation_header_restore() doesn't
work.

Any hints on how to do it correctly?

Thanks,
Rafael

---
From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Subject: [PATCH] x86 / hibernate: Simplify passing of trampoline address

During resume from hibernation, the boot kernel has to jump to
the image kernel's trampoline code to pass control to it.  On
x86_64 the address of that code is passed from the image kernel
to the boot kernel in the image header.  Currently, the way in
which that address is saved by the image kernel is not entirely
straightforward (assembly code is involved in that unnecessarily
etc), so simplify it to make it easier to follow.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
 arch/x86/power/hibernate_64.c     |    6 +++---
 arch/x86/power/hibernate_asm_64.S |    3 ---
 2 files changed, 3 insertions(+), 6 deletions(-)

Index: linux-pm/arch/x86/power/hibernate_64.c
===================================================================
--- linux-pm.orig/arch/x86/power/hibernate_64.c
+++ linux-pm/arch/x86/power/hibernate_64.c
@@ -27,7 +27,7 @@ extern asmlinkage __visible int restore_
  * Address to jump to in the last phase of restore in order to get to the image
  * kernel's text (this value is passed in the image header).
  */
-unsigned long restore_jump_address __visible;
+void *restore_jump_address __visible;

 /*
  * Value of the cr3 register from before the hibernation (this value is passed
@@ -108,7 +108,7 @@ int pfn_is_nosave(unsigned long pfn)
 }

 struct restore_data_record {
-	unsigned long jump_address;
+	void *jump_address;
 	unsigned long cr3;
 	unsigned long magic;
 };
@@ -126,7 +126,7 @@ int arch_hibernation_header_save(void *a

 	if (max_size < sizeof(struct restore_data_record))
 		return -EOVERFLOW;
-	rdr->jump_address = restore_jump_address;
+	rdr->jump_address = &restore_registers;
 	rdr->cr3 = restore_cr3;
 	rdr->magic = RESTORE_MAGIC;
 	return 0;
Index: linux-pm/arch/x86/power/hibernate_asm_64.S
===================================================================
--- linux-pm.orig/arch/x86/power/hibernate_asm_64.S
+++ linux-pm/arch/x86/power/hibernate_asm_64.S
@@ -44,9 +44,6 @@ ENTRY(swsusp_arch_suspend)
 	pushfq
 	popq	pt_regs_flags(%rax)

-	/* save the address of restore_registers */
-	movq	$restore_registers, %rax
-	movq	%rax, restore_jump_address(%rip)
 	/* save cr3 */
 	movq	%cr3, %rax
 	movq	%rax, restore_cr3(%rip)