[PATCH 0/2] make kASLR vs hibernation boot-time selectable

[PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

Distros want to be able to offer CONFIG_RANDOMIZE_BASE as well as
CONFIG_HIBERNATION in a single kernel. Instead of making kASLR depend on
!HIBERNATION at compile time, allow kaslr to be selectable at boot time
(via "kaslr" kernel command line), which will disable hibernation in the
kernel. In this way the end user can choose which feature they want more
with hibernation continuing to stay enabled by default (no surprises).

-Kees

[PATCH 1/2] hibernate: create one-way disable mode

[Kees Cook]

To support using kernel features that are not compatible with hibernation,
this creates a "disabled" hibernation state so that hibernation selection
can be a boot-time choice instead of only a compile-time choice.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/suspend.h  |    2 ++
 kernel/power/hibernate.c |   55 +++++++++++++++++++++++++++++++---------------
 kernel/power/main.c      |    6 ++---
 kernel/power/user.c      |    3 +++
 4 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index f76994b9396c..519064e0c943 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -327,6 +327,7 @@ extern unsigned long get_safe_page(gfp_t gfp_mask);
 extern void hibernation_set_ops(const struct platform_hibernation_ops *ops);
 extern int hibernate(void);
 extern bool system_entering_hibernation(void);
+extern bool hibernation_available(void);
 asmlinkage int swsusp_save(void);
 extern struct pbe *restore_pblist;
 #else /* CONFIG_HIBERNATION */
@@ -339,6 +340,7 @@ static inline void swsusp_unset_page_free(struct page *p) {}
 static inline void hibernation_set_ops(const struct platform_hibernation_ops *ops) {}
 static inline int hibernate(void) { return -ENOSYS; }
 static inline bool system_entering_hibernation(void) { return false; }
+static inline bool hibernation_available(void) { return false; }
 #endif /* CONFIG_HIBERNATION */
 
 /* Hibernation and suspend events */
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index df88d55dc436..32672da81114 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -43,6 +43,7 @@ __visible int in_suspend __nosavedata;
 
 enum {
 	HIBERNATION_INVALID,
+	HIBERNATION_DISABLED,
 	HIBERNATION_PLATFORM,
 	HIBERNATION_SHUTDOWN,
 	HIBERNATION_REBOOT,
@@ -61,6 +62,11 @@ bool freezer_test_done;
 
 static const struct platform_hibernation_ops *hibernation_ops;
 
+bool hibernation_available(void)
+{
+	return (hibernation_mode > HIBERNATION_DISABLED);
+}
+
 /**
  * hibernation_set_ops - Set the global hibernate operations.
  * @ops: Hibernation operations to use in subsequent hibernation transitions.
@@ -75,10 +81,12 @@ void hibernation_set_ops(const struct platform_hibernation_ops *ops)
 	}
 	lock_system_sleep();
 	hibernation_ops = ops;
-	if (ops)
-		hibernation_mode = HIBERNATION_PLATFORM;
-	else if (hibernation_mode == HIBERNATION_PLATFORM)
-		hibernation_mode = HIBERNATION_SHUTDOWN;
+	if (hibernation_available()) {
+		if (ops)
+			hibernation_mode = HIBERNATION_PLATFORM;
+		else if (hibernation_mode == HIBERNATION_PLATFORM)
+			hibernation_mode = HIBERNATION_SHUTDOWN;
+	}
 
 	unlock_system_sleep();
 }
@@ -639,6 +647,11 @@ int hibernate(void)
 {
 	int error;
 
+	if (!hibernation_available()) {
+		pr_debug("PM: Hibernation not available.\n");
+		return -EINVAL;
+	}
+
 	lock_system_sleep();
 	/* The snapshot device should not be opened while we're running */
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
@@ -731,7 +744,7 @@ static int software_resume(void)
 	/*
 	 * If the user said "noresume".. bail out early.
 	 */
-	if (noresume)
+	if (noresume || !hibernation_available())
 		return 0;
 
 	/*
@@ -857,6 +870,7 @@ late_initcall_sync(software_resume);
 
 
 static const char * const hibernation_modes[] = {
+	[HIBERNATION_DISABLED]	= "disabled",
 	[HIBERNATION_PLATFORM]	= "platform",
 	[HIBERNATION_SHUTDOWN]	= "shutdown",
 	[HIBERNATION_REBOOT]	= "reboot",
@@ -931,6 +945,9 @@ static ssize_t disk_store(struct kobject *kobj, struct kobj_attribute *attr,
 	char *p;
 	int mode = HIBERNATION_INVALID;
 
+	if (!hibernation_available())
+		return -EINVAL;
+
 	p = memchr(buf, '\n', n);
 	len = p ? p - buf : n;
 
@@ -942,23 +959,25 @@ static ssize_t disk_store(struct kobject *kobj, struct kobj_attribute *attr,
 			break;
 		}
 	}
-	if (mode != HIBERNATION_INVALID) {
-		switch (mode) {
-		case HIBERNATION_SHUTDOWN:
-		case HIBERNATION_REBOOT:
+	switch (mode) {
+	case HIBERNATION_DISABLED:
+	case HIBERNATION_SHUTDOWN:
+	case HIBERNATION_REBOOT:
 #ifdef CONFIG_SUSPEND
-		case HIBERNATION_SUSPEND:
+	case HIBERNATION_SUSPEND:
 #endif
+		hibernation_mode = mode;
+		break;
+	case HIBERNATION_PLATFORM:
+		if (hibernation_ops)
 			hibernation_mode = mode;
-			break;
-		case HIBERNATION_PLATFORM:
-			if (hibernation_ops)
-				hibernation_mode = mode;
-			else
-				error = -EINVAL;
-		}
-	} else
+		else
+			error = -EINVAL;
+		break;
+	default:
 		error = -EINVAL;
+		break;
+	}
 
 	if (!error)
 		pr_debug("PM: Hibernation mode set to '%s'\n",
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 573410d6647e..8e90f330f139 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -300,13 +300,11 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
 			s += sprintf(s,"%s ", pm_states[i].label);
 
 #endif
-#ifdef CONFIG_HIBERNATION
-	s += sprintf(s, "%s\n", "disk");
-#else
+	if (hibernation_available())
+		s += sprintf(s, "disk ");
 	if (s != buf)
 		/* convert the last space to a newline */
 		*(s-1) = '\n';
-#endif
 	return (s - buf);
 }
 
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 98d357584cd6..000b94419182 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -49,6 +49,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
 	struct snapshot_data *data;
 	int error;
 
+	if (!hibernation_available())
+		return -EINVAL;
+
 	lock_system_sleep();
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
-- 
1.7.9.5

[PATCH 2/2] x86, kaslr: boot-time selectable with hibernation

[Kees Cook]

Changes kASLR from being compile-time selectable (blocked by
CONFIG_HIBERNATION), to being boot-time selectable (with hibernation
available by default) via the "kaslr" kernel command line.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Documentation/kernel-parameters.txt |   11 +++++++----
 arch/x86/Kconfig                    |    1 -
 arch/x86/boot/compressed/aslr.c     |    9 ++++++++-
 kernel/power/hibernate.c            |    7 +++++++
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index b9f67781c577..ea0430de1e52 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1464,6 +1464,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	js=		[HW,JOY] Analog joystick
 			See Documentation/input/joystick.txt.
 
+	kaslr/nokaslr	[X86]
+			Enable/disable kernel and module base offset ASLR
+			(Address Space Layout Randomization) if built into
+			the kernel. When CONFIG_HIBERNATION is selected,
+			kASLR is disabled by default. When kASLR is enabled,
+			hibernation will be disabled.
+
 	keepinitrd	[HW,ARM]
 
 	kernelcore=nn[KMG]	[KNL,X86,IA-64,PPC] This parameter
@@ -2100,10 +2107,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	noapic		[SMP,APIC] Tells the kernel to not make use of any
 			IOAPICs that may be present in the system.
 
-	nokaslr		[X86]
-			Disable kernel and module base offset ASLR (Address
-			Space Layout Randomization) if built into the kernel.
-
 	noautogroup	Disable scheduler automatic task group creation.
 
 	nobats		[PPC] Do not use BATs for mapping kernel lowmem
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b660088c220d..7fdb639f1b63 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1671,7 +1671,6 @@ config RELOCATABLE
 config RANDOMIZE_BASE
 	bool "Randomize the address of the kernel image"
 	depends on RELOCATABLE
-	depends on !HIBERNATION
 	default n
 	---help---
 	   Randomizes the physical and virtual address at which the
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 4dbf967da50d..fc6091abedb7 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -289,10 +289,17 @@ unsigned char *choose_kernel_location(unsigned char *input,
 	unsigned long choice = (unsigned long)output;
 	unsigned long random;
 
+#ifdef CONFIG_HIBERNATION
+	if (!cmdline_find_option_bool("kaslr")) {
+		debug_putstr("KASLR disabled by default...\n");
+		goto out;
+	}
+#else
 	if (cmdline_find_option_bool("nokaslr")) {
-		debug_putstr("KASLR disabled...\n");
+		debug_putstr("KASLR disabled by cmdline...\n");
 		goto out;
 	}
+#endif
 
 	/* Record the various known unsafe memory ranges. */
 	mem_avoid_init((unsigned long)input, input_size,
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 32672da81114..4aba1621b59c 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -67,6 +67,13 @@ bool hibernation_available(void)
 	return (hibernation_mode > HIBERNATION_DISABLED);
 }
 
+static int __init parse_kaslr(char *p)
+{
+	hibernation_mode = HIBERNATION_DISABLED;
+	return 0;
+}
+early_param("kaslr", parse_kaslr);
+
 /**
  * hibernation_set_ops - Set the global hibernate operations.
  * @ops: Hibernation operations to use in subsequent hibernation transitions.
-- 
1.7.9.5

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[H. Peter Anvin]

On 06/12/2014 12:46 PM, Kees Cook wrote:
> Distros want to be able to offer CONFIG_RANDOMIZE_BASE as well as
> CONFIG_HIBERNATION in a single kernel. Instead of making kASLR depend on
> !HIBERNATION at compile time, allow kaslr to be selectable at boot time
> (via "kaslr" kernel command line), which will disable hibernation in the
> kernel. In this way the end user can choose which feature they want more
> with hibernation continuing to stay enabled by default (no surprises).

Any way we can make them work together instead?

	-hpa

Re: [PATCH 1/2] hibernate: create one-way disable mode

[Rafael J. Wysocki]

On Thursday, June 12, 2014 12:46:58 PM Kees Cook wrote:
> To support using kernel features that are not compatible with hibernation,
> this creates a "disabled" hibernation state so that hibernation selection
> can be a boot-time choice instead of only a compile-time choice.

Well, hibernation_mode is only to determine how to turn the system off
after saving the image.  It doesn't say how the whole hibernation is
supposed to work (or not).

Please just extend the hibernate= command line argument to cover this.

Rafael

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Rafael J. Wysocki]

On Thursday, June 12, 2014 12:48:50 PM H. Peter Anvin wrote:
> On 06/12/2014 12:46 PM, Kees Cook wrote:
> > Distros want to be able to offer CONFIG_RANDOMIZE_BASE as well as
> > CONFIG_HIBERNATION in a single kernel. Instead of making kASLR depend on
> > !HIBERNATION at compile time, allow kaslr to be selectable at boot time
> > (via "kaslr" kernel command line), which will disable hibernation in the
> > kernel. In this way the end user can choose which feature they want more
> > with hibernation continuing to stay enabled by default (no surprises).
> 
> Any way we can make them work together instead?

Yeah, and that too.

Rafael

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

On Thu, Jun 12, 2014 at 12:48 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 06/12/2014 12:46 PM, Kees Cook wrote:
>> Distros want to be able to offer CONFIG_RANDOMIZE_BASE as well as
>> CONFIG_HIBERNATION in a single kernel. Instead of making kASLR depend on
>> !HIBERNATION at compile time, allow kaslr to be selectable at boot time
>> (via "kaslr" kernel command line), which will disable hibernation in the
>> kernel. In this way the end user can choose which feature they want more
>> with hibernation continuing to stay enabled by default (no surprises).
>
> Any way we can make them work together instead?

I'm sure there is, but I don't know the solution. :)

At the very least this gets us one step closer (we can build them together).

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[H. Peter Anvin]

On 06/12/2014 01:27 PM, Kees Cook wrote:
>>
>> Any way we can make them work together instead?
> 
> I'm sure there is, but I don't know the solution. :)
> 
> At the very least this gets us one step closer (we can build them together).
> 

But it is really invasive.

I have to admit to being somewhat fuzzy on what the core problem with
hibernation and kASLR is... in both cases there is a set of pages that
need to be installed, some of which will overlap the loader kernel.
What am I missing?

	-hpa

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

On Thu, Jun 12, 2014 at 1:29 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 06/12/2014 01:27 PM, Kees Cook wrote:
>>>
>>> Any way we can make them work together instead?
>>
>> I'm sure there is, but I don't know the solution. :)
>>
>> At the very least this gets us one step closer (we can build them together).
>>
>
> But it is really invasive.

Well, I don't agree there. I actually would like to be able to turn
off hibernation support on distro kernels regardless of kASLR, so I
think this is really killing two birds with one stone.

> I have to admit to being somewhat fuzzy on what the core problem with
> hibernation and kASLR is... in both cases there is a set of pages that
> need to be installed, some of which will overlap the loader kernel.
> What am I missing?

I don't know how resume works, but I have assumed that the newly
loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
modules, and userspace memory maps from disk. Since these things can
easily contain references to kernel text, if the newly loaded kernel
has moved with regard to the hibernated image, everything breaks.
IIUC, this is similar why you can't rebuild your kernel and resume
from a different version.

Potential solutions might be to do some kind of kexec-ish second
kernel load that puts it in the "right" place, based on what's stored
in the on-disk image. It sounds extremely non-trivial, and isn't
something I will have time to work on in the foreseeable future.

Making them both build together, however, that I can do. :)

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Pavel Machek]

Hi!


> >>> Any way we can make them work together instead?
> >>
> >> I'm sure there is, but I don't know the solution. :)
> >>
> >> At the very least this gets us one step closer (we can build them together).
> >>
> >
> > But it is really invasive.
> 
> Well, I don't agree there. I actually would like to be able to turn
> off hibernation support on distro kernels regardless of kASLR, so I
> think this is really killing two birds with one stone.
> 
> > I have to admit to being somewhat fuzzy on what the core problem with
> > hibernation and kASLR is... in both cases there is a set of pages that
> > need to be installed, some of which will overlap the loader kernel.
> > What am I missing?
> 
> I don't know how resume works, but I have assumed that the newly
> loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
> modules, and userspace memory maps from disk. Since these things can
> easily contain references to kernel text, if the newly loaded kernel
> has moved with regard to the hibernated image, everything breaks.
> IIUC, this is similar why you can't rebuild your kernel and resume
> from a different version.

x86-64 can resume from different kernel that did the suspend. kASLR
should not be too different from that. (You just include kernel text
in the hibernation image. It is small enough to do that.)

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

On Fri, Jun 13, 2014 at 3:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>
>> >>> Any way we can make them work together instead?
>> >>
>> >> I'm sure there is, but I don't know the solution. :)
>> >>
>> >> At the very least this gets us one step closer (we can build them together).
>> >>
>> >
>> > But it is really invasive.
>>
>> Well, I don't agree there. I actually would like to be able to turn
>> off hibernation support on distro kernels regardless of kASLR, so I
>> think this is really killing two birds with one stone.
>>
>> > I have to admit to being somewhat fuzzy on what the core problem with
>> > hibernation and kASLR is... in both cases there is a set of pages that
>> > need to be installed, some of which will overlap the loader kernel.
>> > What am I missing?
>>
>> I don't know how resume works, but I have assumed that the newly
>> loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
>> modules, and userspace memory maps from disk. Since these things can
>> easily contain references to kernel text, if the newly loaded kernel
>> has moved with regard to the hibernated image, everything breaks.
>> IIUC, this is similar why you can't rebuild your kernel and resume
>> from a different version.
>
> x86-64 can resume from different kernel that did the suspend. kASLR
> should not be too different from that. (You just include kernel text
> in the hibernation image. It is small enough to do that.)

Oooh, that's very exciting! How does that work (what happens to the
kernel that booted first, etc)? I assume physical memory layout can't
change between hibernation and resume? Or, where should I be reading
code that does this?

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[H. Peter Anvin]

On 06/13/2014 10:32 AM, Kees Cook wrote:
>>
>> x86-64 can resume from different kernel that did the suspend. kASLR
>> should not be too different from that. (You just include kernel text
>> in the hibernation image. It is small enough to do that.)
> 
> Oooh, that's very exciting! How does that work (what happens to the
> kernel that booted first, etc)? I assume physical memory layout can't
> change between hibernation and resume? Or, where should I be reading
> code that does this?
> 

"Give me a lever and a place to stand, and I shall move the world."
Reshuffling memory in an arbitrary or near-arbitrary way really isn't
all that hard.  The exact data structures you need depends on if you
have any kind of page alignment you can rely on (makes it easier) and
how much spare memory you have (in case of hibernation, there is usually
tons of unused memory as it doesn't make sense to hibernate clean pages.)

	-hpa

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Pavel Machek]

On Fri 2014-06-13 10:32:56, Kees Cook wrote:
> On Fri, Jun 13, 2014 at 3:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >
> >> >>> Any way we can make them work together instead?
> >> >>
> >> >> I'm sure there is, but I don't know the solution. :)
> >> >>
> >> >> At the very least this gets us one step closer (we can build them together).
> >> >>
> >> >
> >> > But it is really invasive.
> >>
> >> Well, I don't agree there. I actually would like to be able to turn
> >> off hibernation support on distro kernels regardless of kASLR, so I
> >> think this is really killing two birds with one stone.
> >>
> >> > I have to admit to being somewhat fuzzy on what the core problem with
> >> > hibernation and kASLR is... in both cases there is a set of pages that
> >> > need to be installed, some of which will overlap the loader kernel.
> >> > What am I missing?
> >>
> >> I don't know how resume works, but I have assumed that the newly
> >> loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
> >> modules, and userspace memory maps from disk. Since these things can
> >> easily contain references to kernel text, if the newly loaded kernel
> >> has moved with regard to the hibernated image, everything breaks.
> >> IIUC, this is similar why you can't rebuild your kernel and resume
> >> from a different version.
> >
> > x86-64 can resume from different kernel that did the suspend. kASLR
> > should not be too different from that. (You just include kernel text
> > in the hibernation image. It is small enough to do that.)
> 
> Oooh, that's very exciting! How does that work (what happens to the
> kernel that booted first, etc)? I assume physical memory layout can't
> change between hibernation and resume? Or, where should I be reading
> code that does this?

I'm not sure what you mean by "physical memory layout can't
change". You may not remove RAM between suspend/resume, no.

It is Rafael's design, actually, you can see it in
arch/x86/power/hibernate_asm_64.S .

32bit version does have that ability, IIRC.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

On Fri, Jun 13, 2014 at 3:14 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, June 13, 2014 10:32:56 AM Kees Cook wrote:
>> On Fri, Jun 13, 2014 at 3:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
>> > Hi!
>> >
>> >
>> >> >>> Any way we can make them work together instead?
>> >> >>
>> >> >> I'm sure there is, but I don't know the solution. :)
>> >> >>
>> >> >> At the very least this gets us one step closer (we can build them together).
>> >> >>
>> >> >
>> >> > But it is really invasive.
>> >>
>> >> Well, I don't agree there. I actually would like to be able to turn
>> >> off hibernation support on distro kernels regardless of kASLR, so I
>> >> think this is really killing two birds with one stone.
>> >>
>> >> > I have to admit to being somewhat fuzzy on what the core problem with
>> >> > hibernation and kASLR is... in both cases there is a set of pages that
>> >> > need to be installed, some of which will overlap the loader kernel.
>> >> > What am I missing?
>> >>
>> >> I don't know how resume works, but I have assumed that the newly
>> >> loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
>> >> modules, and userspace memory maps from disk. Since these things can
>> >> easily contain references to kernel text, if the newly loaded kernel
>> >> has moved with regard to the hibernated image, everything breaks.
>> >> IIUC, this is similar why you can't rebuild your kernel and resume
>> >> from a different version.
>> >
>> > x86-64 can resume from different kernel that did the suspend. kASLR
>> > should not be too different from that. (You just include kernel text
>> > in the hibernation image. It is small enough to do that.)
>>
>> Oooh, that's very exciting! How does that work (what happens to the
>> kernel that booted first, etc)? I assume physical memory layout can't
>> change between hibernation and resume? Or, where should I be reading
>> code that does this?
>
> I guess it would help if you were a bit less sarcastic, but perhaps that's
> just me.

Oh, er, I think that got misunderstood. I'm very rarely sarcastic in
online communication. I wasn't being sarcastic here at all. I _do_
find it exciting that one can resume with a different kernel! That's
been a limitation that plagued me for years. I had no idea that
restriction got lifted. I really did mean I was excited. Sorry if that
was misunderstood!

> Anyway, the core hibernation code actually works with page frames rather
> than with virtual addresses.  Essentially, it creates a bitmap where each
> page frame is represented by a single bit and the bits representing free
> page frames are unset.  It then allocates as many new pages as there are
> set bits in the bitmap and copies the entire contents of the page frames
> represented by those bits to new pages it's just allocated. That covers
> the entire kernel with its data and all process memory and is saved to
> disk storage along with the PFNs of the page frames whose contents have
> been copied.
>
> During resume it simply restores the contents of the saved page frames
> into those same page frames if they are available at that time.  For the
> page frames that aren't free then it allocates memory to store their
> contents temporarily and creates a list of PFNs where that contents should
> be moved eventually.  Then, it quiesces all activity of the system and
> jumps to arch-specific code that copies data from the temporary memory to
> the target page frames (that generally overwrites the boot kernel, so there's
> no way back from it).  Finally, it jumps to a specific address where the
> hibernated kernel trampoline code should be present.
>
> I think what fails with kASLR is that last step, because everything else
> should be entirely agnostic to the way the virtual addresses are laid out.
> I'm not sure how to fix that at the moment, but it should be fixable at
> least on x86_64.

Very cool. How does the kernel doing the resume identify the
trampoline location in the hibernated kernel? If it can handle a
different kernel in the hibernation image, I assume there's been some
specific identification in the image instead of using what
kernel-doing-the-resume thinks the trampoline is (based on its own
offsets).

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Rafael J. Wysocki]

On Friday, June 13, 2014 10:32:56 AM Kees Cook wrote:
> On Fri, Jun 13, 2014 at 3:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >
> >> >>> Any way we can make them work together instead?
> >> >>
> >> >> I'm sure there is, but I don't know the solution. :)
> >> >>
> >> >> At the very least this gets us one step closer (we can build them together).
> >> >>
> >> >
> >> > But it is really invasive.
> >>
> >> Well, I don't agree there. I actually would like to be able to turn
> >> off hibernation support on distro kernels regardless of kASLR, so I
> >> think this is really killing two birds with one stone.
> >>
> >> > I have to admit to being somewhat fuzzy on what the core problem with
> >> > hibernation and kASLR is... in both cases there is a set of pages that
> >> > need to be installed, some of which will overlap the loader kernel.
> >> > What am I missing?
> >>
> >> I don't know how resume works, but I have assumed that the newly
> >> loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
> >> modules, and userspace memory maps from disk. Since these things can
> >> easily contain references to kernel text, if the newly loaded kernel
> >> has moved with regard to the hibernated image, everything breaks.
> >> IIUC, this is similar why you can't rebuild your kernel and resume
> >> from a different version.
> >
> > x86-64 can resume from different kernel that did the suspend. kASLR
> > should not be too different from that. (You just include kernel text
> > in the hibernation image. It is small enough to do that.)
> 
> Oooh, that's very exciting! How does that work (what happens to the
> kernel that booted first, etc)? I assume physical memory layout can't
> change between hibernation and resume? Or, where should I be reading
> code that does this?

I guess it would help if you were a bit less sarcastic, but perhaps that's
just me.

Anyway, the core hibernation code actually works with page frames rather
than with virtual addresses.  Essentially, it creates a bitmap where each
page frame is represented by a single bit and the bits representing free
page frames are unset.  It then allocates as many new pages as there are
set bits in the bitmap and copies the entire contents of the page frames
represented by those bits to new pages it's just allocated. That covers
the entire kernel with its data and all process memory and is saved to
disk storage along with the PFNs of the page frames whose contents have
been copied.

During resume it simply restores the contents of the saved page frames
into those same page frames if they are available at that time.  For the
page frames that aren't free then it allocates memory to store their
contents temporarily and creates a list of PFNs where that contents should
be moved eventually.  Then, it quiesces all activity of the system and
jumps to arch-specific code that copies data from the temporary memory to
the target page frames (that generally overwrites the boot kernel, so there's
no way back from it).  Finally, it jumps to a specific address where the
hibernated kernel trampoline code should be present.

I think what fails with kASLR is that last step, because everything else
should be entirely agnostic to the way the virtual addresses are laid out.
I'm not sure how to fix that at the moment, but it should be fixable at
least on x86_64.

Rafael

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Rafael J. Wysocki]

On Friday, June 13, 2014 03:07:19 PM Kees Cook wrote:
> On Fri, Jun 13, 2014 at 3:14 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Friday, June 13, 2014 10:32:56 AM Kees Cook wrote:
> >> On Fri, Jun 13, 2014 at 3:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> >> > Hi!
> >> >
> >> >
> >> >> >>> Any way we can make them work together instead?
> >> >> >>
> >> >> >> I'm sure there is, but I don't know the solution. :)
> >> >> >>
> >> >> >> At the very least this gets us one step closer (we can build them together).
> >> >> >>
> >> >> >
> >> >> > But it is really invasive.
> >> >>
> >> >> Well, I don't agree there. I actually would like to be able to turn
> >> >> off hibernation support on distro kernels regardless of kASLR, so I
> >> >> think this is really killing two birds with one stone.
> >> >>
> >> >> > I have to admit to being somewhat fuzzy on what the core problem with
> >> >> > hibernation and kASLR is... in both cases there is a set of pages that
> >> >> > need to be installed, some of which will overlap the loader kernel.
> >> >> > What am I missing?
> >> >>
> >> >> I don't know how resume works, but I have assumed that the newly
> >> >> loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
> >> >> modules, and userspace memory maps from disk. Since these things can
> >> >> easily contain references to kernel text, if the newly loaded kernel
> >> >> has moved with regard to the hibernated image, everything breaks.
> >> >> IIUC, this is similar why you can't rebuild your kernel and resume
> >> >> from a different version.
> >> >
> >> > x86-64 can resume from different kernel that did the suspend. kASLR
> >> > should not be too different from that. (You just include kernel text
> >> > in the hibernation image. It is small enough to do that.)
> >>
> >> Oooh, that's very exciting! How does that work (what happens to the
> >> kernel that booted first, etc)? I assume physical memory layout can't
> >> change between hibernation and resume? Or, where should I be reading
> >> code that does this?
> >
> > I guess it would help if you were a bit less sarcastic, but perhaps that's
> > just me.
> 
> Oh, er, I think that got misunderstood. I'm very rarely sarcastic in
> online communication. I wasn't being sarcastic here at all. I _do_
> find it exciting that one can resume with a different kernel! That's
> been a limitation that plagued me for years. I had no idea that
> restriction got lifted. I really did mean I was excited. Sorry if that
> was misunderstood!

Sorry about my misunderstanding. :-)

> > Anyway, the core hibernation code actually works with page frames rather
> > than with virtual addresses.  Essentially, it creates a bitmap where each
> > page frame is represented by a single bit and the bits representing free
> > page frames are unset.  It then allocates as many new pages as there are
> > set bits in the bitmap and copies the entire contents of the page frames
> > represented by those bits to new pages it's just allocated. That covers
> > the entire kernel with its data and all process memory and is saved to
> > disk storage along with the PFNs of the page frames whose contents have
> > been copied.
> >
> > During resume it simply restores the contents of the saved page frames
> > into those same page frames if they are available at that time.  For the
> > page frames that aren't free then it allocates memory to store their
> > contents temporarily and creates a list of PFNs where that contents should
> > be moved eventually.  Then, it quiesces all activity of the system and
> > jumps to arch-specific code that copies data from the temporary memory to
> > the target page frames (that generally overwrites the boot kernel, so there's
> > no way back from it).  Finally, it jumps to a specific address where the
> > hibernated kernel trampoline code should be present.
> >
> > I think what fails with kASLR is that last step, because everything else
> > should be entirely agnostic to the way the virtual addresses are laid out.
> > I'm not sure how to fix that at the moment, but it should be fixable at
> > least on x86_64.
> 
> Very cool. How does the kernel doing the resume identify the
> trampoline location in the hibernated kernel? If it can handle a
> different kernel in the hibernation image, I assume there's been some
> specific identification in the image instead of using what
> kernel-doing-the-resume thinks the trampoline is (based on its own
> offsets).

There is a simple mechanism to pass the address to jump to in the image
header.  Unfortunately, that *is* a virtual address if I remember correctly.

I'll have a closer look at that shortly (it's been quite some time since
I wrote that code).

Rafael

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

On Fri, Jun 13, 2014 at 3:54 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, June 13, 2014 03:07:19 PM Kees Cook wrote:
>> On Fri, Jun 13, 2014 at 3:14 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Friday, June 13, 2014 10:32:56 AM Kees Cook wrote:
>> >> On Fri, Jun 13, 2014 at 3:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
>> >> > Hi!
>> >> >
>> >> >
>> >> >> >>> Any way we can make them work together instead?
>> >> >> >>
>> >> >> >> I'm sure there is, but I don't know the solution. :)
>> >> >> >>
>> >> >> >> At the very least this gets us one step closer (we can build them together).
>> >> >> >>
>> >> >> >
>> >> >> > But it is really invasive.
>> >> >>
>> >> >> Well, I don't agree there. I actually would like to be able to turn
>> >> >> off hibernation support on distro kernels regardless of kASLR, so I
>> >> >> think this is really killing two birds with one stone.
>> >> >>
>> >> >> > I have to admit to being somewhat fuzzy on what the core problem with
>> >> >> > hibernation and kASLR is... in both cases there is a set of pages that
>> >> >> > need to be installed, some of which will overlap the loader kernel.
>> >> >> > What am I missing?
>> >> >>
>> >> >> I don't know how resume works, but I have assumed that the newly
>> >> >> loaded kernel stays in memory and pulls in the vmalloc, kmalloc,
>> >> >> modules, and userspace memory maps from disk. Since these things can
>> >> >> easily contain references to kernel text, if the newly loaded kernel
>> >> >> has moved with regard to the hibernated image, everything breaks.
>> >> >> IIUC, this is similar why you can't rebuild your kernel and resume
>> >> >> from a different version.
>> >> >
>> >> > x86-64 can resume from different kernel that did the suspend. kASLR
>> >> > should not be too different from that. (You just include kernel text
>> >> > in the hibernation image. It is small enough to do that.)
>> >>
>> >> Oooh, that's very exciting! How does that work (what happens to the
>> >> kernel that booted first, etc)? I assume physical memory layout can't
>> >> change between hibernation and resume? Or, where should I be reading
>> >> code that does this?
>> >
>> > I guess it would help if you were a bit less sarcastic, but perhaps that's
>> > just me.
>>
>> Oh, er, I think that got misunderstood. I'm very rarely sarcastic in
>> online communication. I wasn't being sarcastic here at all. I _do_
>> find it exciting that one can resume with a different kernel! That's
>> been a limitation that plagued me for years. I had no idea that
>> restriction got lifted. I really did mean I was excited. Sorry if that
>> was misunderstood!
>
> Sorry about my misunderstanding. :-)
>
>> > Anyway, the core hibernation code actually works with page frames rather
>> > than with virtual addresses.  Essentially, it creates a bitmap where each
>> > page frame is represented by a single bit and the bits representing free
>> > page frames are unset.  It then allocates as many new pages as there are
>> > set bits in the bitmap and copies the entire contents of the page frames
>> > represented by those bits to new pages it's just allocated. That covers
>> > the entire kernel with its data and all process memory and is saved to
>> > disk storage along with the PFNs of the page frames whose contents have
>> > been copied.
>> >
>> > During resume it simply restores the contents of the saved page frames
>> > into those same page frames if they are available at that time.  For the
>> > page frames that aren't free then it allocates memory to store their
>> > contents temporarily and creates a list of PFNs where that contents should
>> > be moved eventually.  Then, it quiesces all activity of the system and
>> > jumps to arch-specific code that copies data from the temporary memory to
>> > the target page frames (that generally overwrites the boot kernel, so there's
>> > no way back from it).  Finally, it jumps to a specific address where the
>> > hibernated kernel trampoline code should be present.
>> >
>> > I think what fails with kASLR is that last step, because everything else
>> > should be entirely agnostic to the way the virtual addresses are laid out.
>> > I'm not sure how to fix that at the moment, but it should be fixable at
>> > least on x86_64.
>>
>> Very cool. How does the kernel doing the resume identify the
>> trampoline location in the hibernated kernel? If it can handle a
>> different kernel in the hibernation image, I assume there's been some
>> specific identification in the image instead of using what
>> kernel-doing-the-resume thinks the trampoline is (based on its own
>> offsets).
>
> There is a simple mechanism to pass the address to jump to in the image
> header.  Unfortunately, that *is* a virtual address if I remember correctly.
>
> I'll have a closer look at that shortly (it's been quite some time since
> I wrote that code).

Thanks; I'm trying to get a test environment instrumented too so I can
look at this. (At the very least, it sounds like we'll still need my
patch series for other architectures.)

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

On Fri, Jun 13, 2014 at 5:14 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, June 13, 2014 03:59:57 PM Kees Cook wrote:
>> On Fri, Jun 13, 2014 at 3:54 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Friday, June 13, 2014 03:07:19 PM Kees Cook wrote:
>
> [cut]
>
>> > I'll have a closer look at that shortly (it's been quite some time since
>> > I wrote that code).
>>
>> Thanks; I'm trying to get a test environment instrumented too so I can
>> look at this. (At the very least, it sounds like we'll still need my
>> patch series for other architectures.)
>
> How can I obtain a kernel address of the beginning of a given page
> (as represented by struct page) on x86_64 today?

I don't know off the top of my head. I've used virt_to_phys, but
things like PFN_PHYS(page_to_pfn(page)) maybe? I'm not entirely clear
which you need, but mm.h seems to have the bulk of what I've seen.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Rafael J. Wysocki]

On Friday, June 13, 2014 03:59:57 PM Kees Cook wrote:
> On Fri, Jun 13, 2014 at 3:54 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Friday, June 13, 2014 03:07:19 PM Kees Cook wrote:

[cut]

> > I'll have a closer look at that shortly (it's been quite some time since
> > I wrote that code).
> 
> Thanks; I'm trying to get a test environment instrumented too so I can
> look at this. (At the very least, it sounds like we'll still need my
> patch series for other architectures.)

How can I obtain a kernel address of the beginning of a given page
(as represented by struct page) on x86_64 today?

Rafael

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Rafael J. Wysocki]

On Friday, June 13, 2014 05:08:21 PM Kees Cook wrote:
> On Fri, Jun 13, 2014 at 5:14 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Friday, June 13, 2014 03:59:57 PM Kees Cook wrote:
> >> On Fri, Jun 13, 2014 at 3:54 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >> > On Friday, June 13, 2014 03:07:19 PM Kees Cook wrote:
> >
> > [cut]
> >
> >> > I'll have a closer look at that shortly (it's been quite some time since
> >> > I wrote that code).
> >>
> >> Thanks; I'm trying to get a test environment instrumented too so I can
> >> look at this. (At the very least, it sounds like we'll still need my
> >> patch series for other architectures.)
> >
> > How can I obtain a kernel address of the beginning of a given page
> > (as represented by struct page) on x86_64 today?
> 
> I don't know off the top of my head. I've used virt_to_phys, but
> things like PFN_PHYS(page_to_pfn(page)) maybe? I'm not entirely clear
> which you need, but mm.h seems to have the bulk of what I've seen.

OK, I'm not sure how much sense this makes, but at least it should
illustrate the direction. :-)

---
 arch/x86/power/hibernate_64.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 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
@@ -115,7 +115,7 @@ struct restore_data_record {
 	unsigned long magic;
 };
 
-#define RESTORE_MAGIC	0x0123456789ABCDEFUL
+#define RESTORE_MAGIC	0x0123456789ABCDF0UL
 
 /**
  *	arch_hibernation_header_save - populate the architecture specific part
@@ -128,7 +128,8 @@ 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 = virt_to_phys((void *)restore_jump_address);
 	rdr->cr3 = restore_cr3;
 	rdr->magic = RESTORE_MAGIC;
 	return 0;
@@ -143,7 +144,7 @@ int arch_hibernation_header_restore(void
 {
 	struct restore_data_record *rdr = addr;
 
-	restore_jump_address = rdr->jump_address;
+	restore_jump_address = (unsigned long)phys_to_virt(rdr->jump_address);
 	restore_cr3 = rdr->cr3;
 	return (rdr->magic == RESTORE_MAGIC) ? 0 : -EINVAL;
 }

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[H. Peter Anvin]

On 06/13/2014 05:14 PM, Rafael J. Wysocki wrote:
> 
> How can I obtain a kernel address of the beginning of a given page
> (as represented by struct page) on x86_64 today?
> 

page_to_virt()

	-hpa

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Kees Cook]

On Fri, Jun 13, 2014 at 5:39 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, June 13, 2014 05:08:21 PM Kees Cook wrote:
>> On Fri, Jun 13, 2014 at 5:14 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Friday, June 13, 2014 03:59:57 PM Kees Cook wrote:
>> >> On Fri, Jun 13, 2014 at 3:54 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> >> > On Friday, June 13, 2014 03:07:19 PM Kees Cook wrote:
>> >
>> > [cut]
>> >
>> >> > I'll have a closer look at that shortly (it's been quite some time since
>> >> > I wrote that code).
>> >>
>> >> Thanks; I'm trying to get a test environment instrumented too so I can
>> >> look at this. (At the very least, it sounds like we'll still need my
>> >> patch series for other architectures.)
>> >
>> > How can I obtain a kernel address of the beginning of a given page
>> > (as represented by struct page) on x86_64 today?
>>
>> I don't know off the top of my head. I've used virt_to_phys, but
>> things like PFN_PHYS(page_to_pfn(page)) maybe? I'm not entirely clear
>> which you need, but mm.h seems to have the bulk of what I've seen.
>
> OK, I'm not sure how much sense this makes, but at least it should
> illustrate the direction. :-)
>
> ---
>  arch/x86/power/hibernate_64.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 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
> @@ -115,7 +115,7 @@ struct restore_data_record {
>         unsigned long magic;
>  };
>
> -#define RESTORE_MAGIC  0x0123456789ABCDEFUL
> +#define RESTORE_MAGIC  0x0123456789ABCDF0UL
>
>  /**
>   *     arch_hibernation_header_save - populate the architecture specific part
> @@ -128,7 +128,8 @@ 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 = virt_to_phys((void *)restore_jump_address);
>         rdr->cr3 = restore_cr3;
>         rdr->magic = RESTORE_MAGIC;
>         return 0;
> @@ -143,7 +144,7 @@ int arch_hibernation_header_restore(void
>  {
>         struct restore_data_record *rdr = addr;
>
> -       restore_jump_address = rdr->jump_address;
> +       restore_jump_address = (unsigned long)phys_to_virt(rdr->jump_address);
>         restore_cr3 = rdr->cr3;
>         return (rdr->magic == RESTORE_MAGIC) ? 0 : -EINVAL;
>  }
>

Hrm, well, at least for me, it doesn't look to be that simple. I tried
phys_to_virt, page_to_virt, and pfn_to_virt -- but it always fell
over. Is jump_address page aligned? Does the translation happen after
the image page tables have been flushed?

It seems to me like we do actually want to virtual address, so this
should already work without changes -- we want the image's expected
entry point, which has already been read out of the saved image? I
suspect there's some other assumption in the code that isn't jumping
out at me yet.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[H. Peter Anvin]

On 06/13/2014 05:39 PM, Rafael J. Wysocki wrote:
>  
> -#define RESTORE_MAGIC	0x0123456789ABCDEFUL
> +#define RESTORE_MAGIC	0x0123456789ABCDF0UL
>  

<bikeshed>

Please don't pick numbers like this for magic numbers... *everyone else
does, too.*  In general, picking a random number is a good starting
point; for extra bonus make sure it has at least one invalid UTF-8
sequence in it (so it cannot be confused with either ASCII or UTF-8 text.)

</bikeshed>

	-hpa

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Rafael J. Wysocki]

On Saturday, June 14, 2014 09:41:19 AM H. Peter Anvin wrote:
> On 06/13/2014 05:39 PM, Rafael J. Wysocki wrote:
> >  
> > -#define RESTORE_MAGIC	0x0123456789ABCDEFUL
> > +#define RESTORE_MAGIC	0x0123456789ABCDF0UL
> >  
> 
> <bikeshed>
> 
> Please don't pick numbers like this for magic numbers... *everyone else
> does, too.*  In general, picking a random number is a good starting
> point; for extra bonus make sure it has at least one invalid UTF-8
> sequence in it (so it cannot be confused with either ASCII or UTF-8 text.)
> 
> </bikeshed>

OK, when we find out how to make hibernation work with the address space
randomization.

Rafael

Re: [PATCH 0/2] make kASLR vs hibernation boot-time selectable

[Rafael J. Wysocki]

On Saturday, June 14, 2014 12:37:49 AM Kees Cook wrote:
> On Fri, Jun 13, 2014 at 5:39 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Friday, June 13, 2014 05:08:21 PM Kees Cook wrote:
> >> On Fri, Jun 13, 2014 at 5:14 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >> > On Friday, June 13, 2014 03:59:57 PM Kees Cook wrote:
> >> >> On Fri, Jun 13, 2014 at 3:54 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >> >> > On Friday, June 13, 2014 03:07:19 PM Kees Cook wrote:
> >> >
> >> > [cut]
> >> >
> >> >> > I'll have a closer look at that shortly (it's been quite some time since
> >> >> > I wrote that code).
> >> >>
> >> >> Thanks; I'm trying to get a test environment instrumented too so I can
> >> >> look at this. (At the very least, it sounds like we'll still need my
> >> >> patch series for other architectures.)
> >> >
> >> > How can I obtain a kernel address of the beginning of a given page
> >> > (as represented by struct page) on x86_64 today?
> >>
> >> I don't know off the top of my head. I've used virt_to_phys, but
> >> things like PFN_PHYS(page_to_pfn(page)) maybe? I'm not entirely clear
> >> which you need, but mm.h seems to have the bulk of what I've seen.
> >
> > OK, I'm not sure how much sense this makes, but at least it should
> > illustrate the direction. :-)
> >
> > ---
> >  arch/x86/power/hibernate_64.c |    7 ++++---
> >  1 file changed, 4 insertions(+), 3 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
> > @@ -115,7 +115,7 @@ struct restore_data_record {
> >         unsigned long magic;
> >  };
> >
> > -#define RESTORE_MAGIC  0x0123456789ABCDEFUL
> > +#define RESTORE_MAGIC  0x0123456789ABCDF0UL
> >
> >  /**
> >   *     arch_hibernation_header_save - populate the architecture specific part
> > @@ -128,7 +128,8 @@ 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 = virt_to_phys((void *)restore_jump_address);
> >         rdr->cr3 = restore_cr3;
> >         rdr->magic = RESTORE_MAGIC;
> >         return 0;
> > @@ -143,7 +144,7 @@ int arch_hibernation_header_restore(void
> >  {
> >         struct restore_data_record *rdr = addr;
> >
> > -       restore_jump_address = rdr->jump_address;
> > +       restore_jump_address = (unsigned long)phys_to_virt(rdr->jump_address);
> >         restore_cr3 = rdr->cr3;
> >         return (rdr->magic == RESTORE_MAGIC) ? 0 : -EINVAL;
> >  }
> >
> 
> Hrm, well, at least for me, it doesn't look to be that simple. I tried
> phys_to_virt, page_to_virt, and pfn_to_virt -- but it always fell
> over.

That's because of a missing piece I forgot about (below).

> Is jump_address page aligned?

It should be.

> Does the translation happen after the image page tables have been flushed?

I believe so, but it happens in the boot kernel still.

> It seems to me like we do actually want to virtual address, so this
> should already work without changes -- we want the image's expected
> entry point, which has already been read out of the saved image? I
> suspect there's some other assumption in the code that isn't jumping
> out at me yet.

We're jumping from the boot kernel into the image kernel.  The virtual address
comes from the image kernel, but the boot kernel has to use it.  The only way
we can ensure that we'll jump to the right place is to pass the physical address
in the header (otherwise we de facto assume that the virtual address of the
target page frame will be the same in both the boot and the image kernels).

The missing piece is that the code in swsusp_arch_resume() sets up temporary
page tables to ensure that they won't be overwritten while copying the last
remaining image kernel pages to the right page frames (those page tables
have to be stored in page frames that are free from the kernel image perspective).

But if the kernel address space is randomized, set_up_temporary_mappings()
really should duplicate the existing layout instead of creating a new one from
scratch.  Otherwise, virtual addresses before set_up_temporary_mappings() may
be different from the ones after it.

Rafael