diff mbox

[v2] kaslr: allow kASLR to be default over Hibernation

Message ID 20160412221659.GA18102@www.outflux.net (mailing list archive)
State Rejected, archived
Headers show

Commit Message

Kees Cook April 12, 2016, 10:16 p.m. UTC
Since kASLR and Hibernation can not currently coexist at runtime
on x86, the default behavior was to disable kASLR by default when
CONFIG_HIBERNATION was present (to retain original behavior).

The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
default when selected at build time. Since arm64 Hibernation does not
conflict with kASLR, this fixes the hibernation argument parsing to be
x86-specific. Additionally, since end users want to be able to select
kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
that is present only on x86.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
- make this x86-specific selectable, rather than global default
---
 Documentation/kernel-parameters.txt |  9 +++------
 arch/x86/Kconfig                    | 16 +++++++++++++++-
 arch/x86/boot/compressed/aslr.c     |  2 +-
 kernel/power/hibernate.c            | 31 +++++++++++++++++++++++++------
 4 files changed, 44 insertions(+), 14 deletions(-)

Comments

Pavel Machek April 14, 2016, 8:01 p.m. UTC | #1
Hi!

> Since kASLR and Hibernation can not currently coexist at runtime
> on x86, the default behavior was to disable kASLR by default when
> CONFIG_HIBERNATION was present (to retain original behavior).
> 
> The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
> default when selected at build time. Since arm64 Hibernation does not
> conflict with kASLR, this fixes the hibernation argument parsing to be
> x86-specific. Additionally, since end users want to be able to select
> kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
> that is present only on x86.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

I believe this is bad idea. arm64 shows that kaslr and hibernation can
coexist, and hibernation is still useful when your battery runs out.

									Pavel
Kees Cook April 14, 2016, 8:14 p.m. UTC | #2
On Thu, Apr 14, 2016 at 1:01 PM, Pavel Machek <pavel@denx.de> wrote:
> Hi!
>
>> Since kASLR and Hibernation can not currently coexist at runtime
>> on x86, the default behavior was to disable kASLR by default when
>> CONFIG_HIBERNATION was present (to retain original behavior).
>>
>> The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
>> default when selected at build time. Since arm64 Hibernation does not
>> conflict with kASLR, this fixes the hibernation argument parsing to be
>> x86-specific. Additionally, since end users want to be able to select
>> kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
>> that is present only on x86.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> I believe this is bad idea. arm64 shows that kaslr and hibernation can
> coexist, and hibernation is still useful when your battery runs out.

What? I'm confused -- this patch leaves the x86 behavior as-is by
default but allows hibernation to work with arm64. (For example, right
now, if you boot arm64 with "kaslr" kernel argument, hibernation will
get needlessly disabled.)

-Kees
Pavel Machek April 14, 2016, 8:34 p.m. UTC | #3
On Thu 2016-04-14 13:14:07, Kees Cook wrote:
> On Thu, Apr 14, 2016 at 1:01 PM, Pavel Machek <pavel@denx.de> wrote:
> > Hi!
> >
> >> Since kASLR and Hibernation can not currently coexist at runtime
> >> on x86, the default behavior was to disable kASLR by default when
> >> CONFIG_HIBERNATION was present (to retain original behavior).
> >>
> >> The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
> >> default when selected at build time. Since arm64 Hibernation does not
> >> conflict with kASLR, this fixes the hibernation argument parsing to be
> >> x86-specific. Additionally, since end users want to be able to select
> >> kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
> >> that is present only on x86.
> >>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > I believe this is bad idea. arm64 shows that kaslr and hibernation can
> > coexist, and hibernation is still useful when your battery runs out.
> 
> What? I'm confused -- this patch leaves the x86 behavior as-is by
> default but allows hibernation to work with arm64. (For example, right
> now, if you boot arm64 with "kaslr" kernel argument, hibernation will
> get needlessly disabled.)

So it is very different from the PATCH v1, still it shares the
subject?

This is the part I don't like:

> >> Since kASLR and Hibernation can not currently coexist at runtime
> >> on x86, the default behavior was to disable kASLR by default when
> >> CONFIG_HIBERNATION was present (to retain original behavior).

Now I notice that it is quite unclear if it actually changes
anything...

								Pavel
Kees Cook April 14, 2016, 10:42 p.m. UTC | #4
On Thu, Apr 14, 2016 at 1:34 PM, Pavel Machek <pavel@denx.de> wrote:
> On Thu 2016-04-14 13:14:07, Kees Cook wrote:
>> On Thu, Apr 14, 2016 at 1:01 PM, Pavel Machek <pavel@denx.de> wrote:
>> > Hi!
>> >
>> >> Since kASLR and Hibernation can not currently coexist at runtime
>> >> on x86, the default behavior was to disable kASLR by default when
>> >> CONFIG_HIBERNATION was present (to retain original behavior).
>> >>
>> >> The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
>> >> default when selected at build time. Since arm64 Hibernation does not
>> >> conflict with kASLR, this fixes the hibernation argument parsing to be
>> >> x86-specific. Additionally, since end users want to be able to select
>> >> kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
>> >> that is present only on x86.
>> >>
>> >> Signed-off-by: Kees Cook <keescook@chromium.org>
>> >
>> > I believe this is bad idea. arm64 shows that kaslr and hibernation can
>> > coexist, and hibernation is still useful when your battery runs out.
>>
>> What? I'm confused -- this patch leaves the x86 behavior as-is by
>> default but allows hibernation to work with arm64. (For example, right
>> now, if you boot arm64 with "kaslr" kernel argument, hibernation will
>> get needlessly disabled.)
>
> So it is very different from the PATCH v1, still it shares the
> subject?

It was similar, but not the same:

v1: Prefer kASLR over Hibernation
v2: kaslr: allow kASLR to be default over Hibernation

I'm happy to call it whatever you like, though! :)

> This is the part I don't like:
>
>> >> Since kASLR and Hibernation can not currently coexist at runtime
>> >> on x86, the default behavior was to disable kASLR by default when
>> >> CONFIG_HIBERNATION was present (to retain original behavior).
>
> Now I notice that it is quite unclear if it actually changes
> anything...

Okay, right. So, there are a few problems that this patch is solving,
and maybe it needs to be broken up into separate patches, but it
didn't seem like it to me at the time. Specifically:

1) The x86 hibernation and KASLR code don't play well together currently.

"1" was worked around so that both could be built in, but only one
would be active at a time. This lead to:

2) The general hibernation code contains kernel arguments that should
only affect x86.

And we have the desire by folks to have KASLR enabled by default on
x86, giving us:

3) There is no build-time way on x86 to switch the preference of KASLR
vs hibernation.

I think "2" should be solved for this release, since arm64 KASLR is
landing, and mistakenly booting an arm64 system with "kaslr" on the
command line will needlessly disable hibernation.

3 and 2 are a result of 1, and IIUC, you're saying you want to solve 1
to make everything else go away? My only concern with that idea is
that I don't (yet) have the knowledge of x86 hibernation internals to
fix this, and it'll take a while to get to having KASLR on by default
if we have to wait on me to fix hibernation. (I'm not saying I
won't/can't, it's just that it'll take me time to come up to speed on
it.)

That do you think?

-Kees
Ingo Molnar April 15, 2016, 9:20 a.m. UTC | #5
* Kees Cook <keescook@chromium.org> wrote:

> 1) The x86 hibernation and KASLR code don't play well together currently.

Please fix it, don't just work it around ...

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek April 15, 2016, 1:06 p.m. UTC | #6
Hi!

> > Now I notice that it is quite unclear if it actually changes
> > anything...
> 
> Okay, right. So, there are a few problems that this patch is solving,
> and maybe it needs to be broken up into separate patches, but it
> didn't seem like it to me at the time. Specifically:
> 
> 1) The x86 hibernation and KASLR code don't play well together currently.
> 
> "1" was worked around so that both could be built in, but only one
> would be active at a time. This lead to:

> 2) The general hibernation code contains kernel arguments that should
> only affect x86.
> 
> And we have the desire by folks to have KASLR enabled by default on
> x86, giving us:
> 
> 3) There is no build-time way on x86 to switch the preference of KASLR
> vs hibernation.
> 
> I think "2" should be solved for this release, since arm64 KASLR is
> landing, and mistakenly booting an arm64 system with "kaslr" on the
> command line will needlessly disable hibernation.

Yes, I have no problem with "2".

> 3 and 2 are a result of 1, and IIUC, you're saying you want to solve 1
> to make everything else go away? My only concern with that idea is
> that I don't (yet) have the knowledge of x86 hibernation internals to
> fix this, and it'll take a while to get to having KASLR on by default
> if we have to wait on me to fix hibernation. (I'm not saying I
> won't/can't, it's just that it'll take me time to come up to speed on
> it.)
> 
> That do you think?

I believe it is more important to get it right than to have a solution
right now. x86-64 should be best architecture to start...

I don't understand kaslr, but it looks to me like you need to store
the kaslr seed in the hibernation image, then use thet in
set_up_temporary_mappings(). It should not be that bad.

Best regards,
									Pavel
Emrah Demir April 15, 2016, 4:25 p.m. UTC | #7
On 2016-04-14 18:42, Kees Cook wrote:
> On Thu, Apr 14, 2016 at 1:34 PM, Pavel Machek <pavel@denx.de> wrote:
>> On Thu 2016-04-14 13:14:07, Kees Cook wrote:
>>> On Thu, Apr 14, 2016 at 1:01 PM, Pavel Machek <pavel@denx.de> wrote:
>>> > Hi!
>>> >
>>> >> Since kASLR and Hibernation can not currently coexist at runtime
>>> >> on x86, the default behavior was to disable kASLR by default when
>>> >> CONFIG_HIBERNATION was present (to retain original behavior).
>>> >>
>>> >> The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
>>> >> default when selected at build time. Since arm64 Hibernation does not
>>> >> conflict with kASLR, this fixes the hibernation argument parsing to be
>>> >> x86-specific. Additionally, since end users want to be able to select
>>> >> kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
>>> >> that is present only on x86.

>>> >
>>> > I believe this is bad idea. arm64 shows that kaslr and hibernation can
>>> > coexist, and hibernation is still useful when your battery runs out.
>>> 
>>> What? I'm confused -- this patch leaves the x86 behavior as-is by
>>> default but allows hibernation to work with arm64. (For example, 
>>> right
>>> now, if you boot arm64 with "kaslr" kernel argument, hibernation will
>>> get needlessly disabled.)
>> 

So what about making new kernel parameters. "hibernate" and 
"nohibernate"

This way before decompression it will look at the parameters and depend 
on parameters it will act differently

"kaslr" "nohibernate"  -->  kASLR will enable, no hibernate

"nokaslr" "nohibernate" --> Neither of them will work

"kaslr" "hiberante"   ---> Owner has to face consequences(ARM has 
advantages)

"nokaslr" "hibernate"  ---> kASLR disabled and hibernate will works.

Before writing something I want to know your ideas.


-Emrah



--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek April 16, 2016, 7:58 p.m. UTC | #8
On Fri 2016-04-15 12:25:19, Emrah Demir wrote:
> On 2016-04-14 18:42, Kees Cook wrote:
> >On Thu, Apr 14, 2016 at 1:34 PM, Pavel Machek <pavel@denx.de> wrote:
> >>On Thu 2016-04-14 13:14:07, Kees Cook wrote:
> >>>On Thu, Apr 14, 2016 at 1:01 PM, Pavel Machek <pavel@denx.de> wrote:
> >>>> Hi!
> >>>>
> >>>>> Since kASLR and Hibernation can not currently coexist at runtime
> >>>>> on x86, the default behavior was to disable kASLR by default when
> >>>>> CONFIG_HIBERNATION was present (to retain original behavior).
> >>>>>
> >>>>> The behavior of kASLR on arm64 (and soon MIPS) is to be enabled by
> >>>>> default when selected at build time. Since arm64 Hibernation does not
> >>>>> conflict with kASLR, this fixes the hibernation argument parsing to be
> >>>>> x86-specific. Additionally, since end users want to be able to select
> >>>>> kASLR on x86 by default at build time, create CONFIG_RANDOMIZE_BASE_ON
> >>>>> that is present only on x86.
> 
> >>>>
> >>>> I believe this is bad idea. arm64 shows that kaslr and hibernation can
> >>>> coexist, and hibernation is still useful when your battery runs out.
> >>>
> >>>What? I'm confused -- this patch leaves the x86 behavior as-is by
> >>>default but allows hibernation to work with arm64. (For example, right
> >>>now, if you boot arm64 with "kaslr" kernel argument, hibernation will
> >>>get needlessly disabled.)
> >>
> 
> So what about making new kernel parameters. "hibernate" and "nohibernate"
> 
> This way before decompression it will look at the parameters and depend on
> parameters it will act differently
> 
> "kaslr" "nohibernate"  -->  kASLR will enable, no hibernate
> 
> "nokaslr" "nohibernate" --> Neither of them will work
> 
> "kaslr" "hiberante"   ---> Owner has to face consequences(ARM has
> advantages)
> 
> "nokaslr" "hibernate"  ---> kASLR disabled and hibernate will works.
> 
> Before writing something I want to know your ideas.

Patch fixing hibernation with kaslr on x86 and x86-64 would be
welcome.

Adding command line options that will become useless when kernel is
fixed ... not so much.

Best regards,
								Pavel
diff mbox

Patch

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index ecc74fa4bfde..282e5c826c32 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1770,12 +1770,9 @@  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.
+	kaslr/nokaslr	[KNL] When CONFIG_RANDOMIZE_BASE is set, this
+			enables/disables kernel and module base offset ASLR
+			(Address Space Layout Randomization).
 
 	keepinitrd	[HW,ARM]
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2dc18605831f..e0fb1717fe3c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1932,7 +1932,7 @@  config RELOCATABLE
 	  (CONFIG_PHYSICAL_START) is used as the minimum location.
 
 config RANDOMIZE_BASE
-	bool "Randomize the address of the kernel image"
+	bool "Randomize the address of the kernel image (kASLR)"
 	depends on RELOCATABLE
 	default n
 	---help---
@@ -1955,6 +1955,20 @@  config RANDOMIZE_BASE
 
 	   If unsure, say N.
 
+config RANDOMIZE_BASE_ON
+	bool "Prefer kASLR over Hibernation"
+	depends on RANDOMIZE_BASE
+	depends on HIBERNATION
+	default n
+	---help---
+	  Currently Hibernation and kASLR are not compatible at runtime
+	  on x86. To enable kASLR by default (and disable Hibernation),
+	  enable this option. To enable Hibernation by default (and
+	  disable kASLR), disable this option. Regardless of this
+	  setting, the availability of kASLR (and therefore Hibernation)
+	  can be chosen at boot time with the "kaslr" or "nokaslr"
+	  kernel argument.
+
 config RANDOMIZE_BASE_MAX_OFFSET
 	hex "Maximum kASLR offset allowed" if EXPERT
 	depends on RANDOMIZE_BASE
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 6a9b96b4624d..8214b174b9bd 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -304,7 +304,7 @@  unsigned char *choose_kernel_location(struct boot_params *boot_params,
 	unsigned long choice = (unsigned long)output;
 	unsigned long random;
 
-#ifdef CONFIG_HIBERNATION
+#ifndef CONFIG_RANDOMIZE_BASE_ON
 	if (!cmdline_find_option_bool("kaslr")) {
 		debug_putstr("KASLR disabled by default...\n");
 		goto out;
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index fca9254280ee..526a6403fb2e 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -35,8 +35,13 @@ 
 
 
 static int nocompress;
+#ifndef CONFIG_RANDOMIZE_BASE_ON
 static int noresume;
 static int nohibernate;
+#else
+static int noresume = 1;
+static int nohibernate = 1;
+#endif
 static int resume_wait;
 static unsigned int resume_delay;
 static char resume_file[256] = CONFIG_PM_STD_PARTITION;
@@ -1154,11 +1159,6 @@  static int __init nohibernate_setup(char *str)
 	return 1;
 }
 
-static int __init kaslr_nohibernate_setup(char *str)
-{
-	return nohibernate_setup(str);
-}
-
 static int __init page_poison_nohibernate_setup(char *str)
 {
 #ifdef CONFIG_PAGE_POISONING_ZERO
@@ -1175,6 +1175,26 @@  static int __init page_poison_nohibernate_setup(char *str)
 	return 1;
 }
 
+/*
+ * Hibernation on x86 currently conflicts with kASLR, so only change
+ * hibernation boot defaults when seeing kaslr arguments on x86.
+ */
+#if defined(CONFIG_X86) && defined(CONFIG_RANDOMIZE_BASE)
+static int __init kaslr_nohibernate_setup(char *str)
+{
+	return nohibernate_setup(str);
+}
+
+static int __init nokaslr_hibernate_setup(char *str)
+{
+	noresume = 0;
+	nohibernate = 0;
+	return 1;
+}
+__setup("kaslr", kaslr_nohibernate_setup);
+__setup("nokaslr", nokaslr_hibernate_setup);
+#endif
+
 __setup("noresume", noresume_setup);
 __setup("resume_offset=", resume_offset_setup);
 __setup("resume=", resume_setup);
@@ -1182,5 +1202,4 @@  __setup("hibernate=", hibernate_setup);
 __setup("resumewait", resumewait_setup);
 __setup("resumedelay=", resumedelay_setup);
 __setup("nohibernate", nohibernate_setup);
-__setup("kaslr", kaslr_nohibernate_setup);
 __setup("page_poison=", page_poison_nohibernate_setup);