Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Wed, 18 Feb 2015, Borislav Petkov wrote: > > Acked-by: Kees Cook > > Thanks Kees, I'll fold it into Jiri's patch and forward. Fine by me, thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Tue, Feb 17, 2015 at 07:33:40PM -0800, Kees Cook wrote: > You are the best. :) Of course, the bestest! :-P > Acked-by: Kees Cook Thanks Kees, I'll fold it into Jiri's patch and forward. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Tue, Feb 17, 2015 at 2:31 PM, Borislav Petkov wrote:
> On Tue, Feb 17, 2015 at 08:45:53AM -0800, Kees Cook wrote:
>> Maybe it should say:
>>
>> Kernel offset: disabled
>>
>> for maximum clarity?
>
> I.e.:
>
> ---
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 78c91bbf50e2..16b6043cb073 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -843,10 +843,14 @@ static void __init trim_low_memory_range(void)
> static int
> dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
> {
> - pr_emerg("Kernel Offset: 0x%lx from 0x%lx "
> -"(relocation range: 0x%lx-0x%lx)\n",
> -(unsigned long)&_text - __START_KERNEL, __START_KERNEL,
> -__START_KERNEL_map, MODULES_VADDR-1);
> + if (kaslr_enabled)
> + pr_emerg("Kernel Offset: 0x%lx from 0x%lx (relocation range:
> 0x%lx-0x%lx)\n",
> +(unsigned long)&_text - __START_KERNEL,
> +__START_KERNEL,
> +__START_KERNEL_map,
> +MODULES_VADDR-1);
> + else
> + pr_emerg("Kernel Offset: disabled\n");
>
> return 0;
> }
> ---
>
> ?
You are the best. :)
Acked-by: Kees Cook
-Kees
>
> --
> Regards/Gruss,
> Boris.
>
> ECO tip #101: Trim your mails when you reply.
> --
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Tue, Feb 17, 2015 at 08:45:53AM -0800, Kees Cook wrote:
> Maybe it should say:
>
> Kernel offset: disabled
>
> for maximum clarity?
I.e.:
---
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 78c91bbf50e2..16b6043cb073 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -843,10 +843,14 @@ static void __init trim_low_memory_range(void)
static int
dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
{
- pr_emerg("Kernel Offset: 0x%lx from 0x%lx "
-"(relocation range: 0x%lx-0x%lx)\n",
-(unsigned long)&_text - __START_KERNEL, __START_KERNEL,
-__START_KERNEL_map, MODULES_VADDR-1);
+ if (kaslr_enabled)
+ pr_emerg("Kernel Offset: 0x%lx from 0x%lx (relocation range:
0x%lx-0x%lx)\n",
+(unsigned long)&_text - __START_KERNEL,
+__START_KERNEL,
+__START_KERNEL_map,
+MODULES_VADDR-1);
+ else
+ pr_emerg("Kernel Offset: disabled\n");
return 0;
}
---
?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Tue, Feb 17, 2015 at 4:39 AM, Borislav Petkov wrote: > On Tue, Feb 17, 2015 at 01:21:20PM +0100, Jiri Kosina wrote: >> I don't have strong feelings either way. It seems slightly nicer >> to have a predictable oops output format no matter the CONFIG_ >> options and command-line contents, but if you feel like seeing the >> 'Kernel offset: 0' in 'nokaslr' and !CONFIG_RANDOMIZE_BASE cases is >> unnecessary noise, feel free to make this change to my patch. > > Well, wouldn't it be wrong to print this line if kaslr is disabled? > Because of the ambiguity in that case: that line could mean either we > randomized to 0 or kaslr is disabled but you can't know that from the > "0" in there, right? Maybe it should say: Kernel offset: disabled for maximum clarity? -Kees > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Tue, Feb 17, 2015 at 01:21:20PM +0100, Jiri Kosina wrote: > I don't have strong feelings either way. It seems slightly nicer > to have a predictable oops output format no matter the CONFIG_ > options and command-line contents, but if you feel like seeing the > 'Kernel offset: 0' in 'nokaslr' and !CONFIG_RANDOMIZE_BASE cases is > unnecessary noise, feel free to make this change to my patch. Well, wouldn't it be wrong to print this line if kaslr is disabled? Because of the ambiguity in that case: that line could mean either we randomized to 0 or kaslr is disabled but you can't know that from the "0" in there, right? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Tue, 17 Feb 2015, Borislav Petkov wrote:
> > Commit e2b32e678 ("x86, kaslr: randomize module base load address") makes
> > the base address for module to be unconditionally randomized in case when
> > CONFIG_RANDOMIZE_BASE is defined and "nokaslr" option isn't present on the
> > commandline.
> >
> > This is not consistent with how choose_kernel_location() decides whether
> > it will randomize kernel load base.
> >
> > Namely, CONFIG_HIBERNATION disables kASLR (unless "kaslr" option is
> > explicitly specified on kernel commandline), which makes the state space
> > larger than what module loader is looking at. IOW CONFIG_HIBERNATION &&
> > CONFIG_RANDOMIZE_BASE is a valid config option, kASLR wouldn't be applied
> > by default in that case, but module loader is not aware of that.
> >
> > Instead of fixing the logic in module.c, this patch takes more generic
> > aproach. It introduces a new bootparam setup data_type SETUP_KASLR and
> > uses that to pass the information whether kaslr has been applied during
> > kernel decompression, and sets a global 'kaslr_enabled' variable
> > accordingly, so that any kernel code (module loading, livepatching, ...)
> > can make decisions based on its value.
> >
> > x86 module loader is converted to make use of this flag.
> >
> > Signed-off-by: Jiri Kosina
> > ---
> >
> > v1 -> v2:
> >
> > Originally I just calculated the fact on the fly from difference between
> > __START_KERNEL and &text, but Kees correctly pointed out that this doesn't
> > properly catch the case when the offset is randomized to zero. I don't see
>
> Yeah, about that. I think we want to do the thing in addition so that
> we don't have the misleading "Kernel Offset:..." line in splats in case
> kaslr is off.
>
> Right?
I don't have strong feelings either way. It seems slightly nicer to have a
predictable oops output format no matter the CONFIG_ options and
command-line contents, but if you feel like seeing the 'Kernel offset: 0'
in 'nokaslr' and !CONFIG_RANDOMIZE_BASE cases is unnecessary noise, feel
free to make this change to my patch.
Thanks,
--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Fri, Feb 13, 2015 at 04:04:55PM +0100, Jiri Kosina wrote:
> Commit e2b32e678 ("x86, kaslr: randomize module base load address") makes
> the base address for module to be unconditionally randomized in case when
> CONFIG_RANDOMIZE_BASE is defined and "nokaslr" option isn't present on the
> commandline.
>
> This is not consistent with how choose_kernel_location() decides whether
> it will randomize kernel load base.
>
> Namely, CONFIG_HIBERNATION disables kASLR (unless "kaslr" option is
> explicitly specified on kernel commandline), which makes the state space
> larger than what module loader is looking at. IOW CONFIG_HIBERNATION &&
> CONFIG_RANDOMIZE_BASE is a valid config option, kASLR wouldn't be applied
> by default in that case, but module loader is not aware of that.
>
> Instead of fixing the logic in module.c, this patch takes more generic
> aproach. It introduces a new bootparam setup data_type SETUP_KASLR and
> uses that to pass the information whether kaslr has been applied during
> kernel decompression, and sets a global 'kaslr_enabled' variable
> accordingly, so that any kernel code (module loading, livepatching, ...)
> can make decisions based on its value.
>
> x86 module loader is converted to make use of this flag.
>
> Signed-off-by: Jiri Kosina
> ---
>
> v1 -> v2:
>
> Originally I just calculated the fact on the fly from difference between
> __START_KERNEL and &text, but Kees correctly pointed out that this doesn't
> properly catch the case when the offset is randomized to zero. I don't see
Yeah, about that. I think we want to do the thing in addition so that
we don't have the misleading "Kernel Offset:..." line in splats in case
kaslr is off.
Right?
---
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index ab4734e5411d..a203da9cc445 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1275,6 +1275,9 @@ static struct notifier_block kernel_offset_notifier = {
static int __init register_kernel_offset_dumper(void)
{
+ if (!kaslr_enabled)
+ return 0;
+
atomic_notifier_chain_register(&panic_notifier_list,
&kernel_offset_notifier);
return 0;
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Mon, Feb 16, 2015 at 11:27:42AM -0800, Kees Cook wrote: > > Well, we can apply the rule of the last setting sticks and since the > > kernel is always going to be adding the last setup_data element of > > type SETUP_KASLR (the boot loader ones will be somewhere on the list > > in-between and we add to the end), we're fine, no? > > Sounds good to me! Ok, thanks. I'll pick it up and route it through the proper channels. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Mon, Feb 16, 2015 at 3:55 AM, Borislav Petkov wrote: > On Fri, Feb 13, 2015 at 03:25:26PM -0800, Kees Cook wrote: >> No, no; I agree: a malicious boot loader is a lost cause. I mean >> mostly from a misbehavior perspective. Like, someone sees "kaslr" in >> the setup args and thinks they can set it to 1 and boot a kernel, etc. >> Or they set it to 0, but they lack HIBERNATION and "1" gets appended, >> but the setup_data parser sees the boot-loader one set to 0, etc. I'm >> just curious if we should avoid getting some poor system into a >> confusing state. > > Well, we can apply the rule of the last setting sticks and since the > kernel is always going to be adding the last setup_data element of > type SETUP_KASLR (the boot loader ones will be somewhere on the list > in-between and we add to the end), we're fine, no? Sounds good to me! -Kees > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Fri, Feb 13, 2015 at 03:25:26PM -0800, Kees Cook wrote: > No, no; I agree: a malicious boot loader is a lost cause. I mean > mostly from a misbehavior perspective. Like, someone sees "kaslr" in > the setup args and thinks they can set it to 1 and boot a kernel, etc. > Or they set it to 0, but they lack HIBERNATION and "1" gets appended, > but the setup_data parser sees the boot-loader one set to 0, etc. I'm > just curious if we should avoid getting some poor system into a > confusing state. Well, we can apply the rule of the last setting sticks and since the kernel is always going to be adding the last setup_data element of type SETUP_KASLR (the boot loader ones will be somewhere on the list in-between and we add to the end), we're fine, no? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Fri, Feb 13, 2015 at 2:20 PM, Jiri Kosina wrote:
> On Fri, 13 Feb 2015, Kees Cook wrote:
>
>> > Commit e2b32e678 ("x86, kaslr: randomize module base load address") makes
>> > the base address for module to be unconditionally randomized in case when
>> > CONFIG_RANDOMIZE_BASE is defined and "nokaslr" option isn't present on the
>> > commandline.
>> >
>> > This is not consistent with how choose_kernel_location() decides whether
>> > it will randomize kernel load base.
>> >
>> > Namely, CONFIG_HIBERNATION disables kASLR (unless "kaslr" option is
>> > explicitly specified on kernel commandline), which makes the state space
>> > larger than what module loader is looking at. IOW CONFIG_HIBERNATION &&
>> > CONFIG_RANDOMIZE_BASE is a valid config option, kASLR wouldn't be applied
>> > by default in that case, but module loader is not aware of that.
>> >
>> > Instead of fixing the logic in module.c, this patch takes more generic
>> > aproach. It introduces a new bootparam setup data_type SETUP_KASLR and
>> > uses that to pass the information whether kaslr has been applied during
>> > kernel decompression, and sets a global 'kaslr_enabled' variable
>> > accordingly, so that any kernel code (module loading, livepatching, ...)
>> > can make decisions based on its value.
>> >
>> > x86 module loader is converted to make use of this flag.
>> >
>> > Signed-off-by: Jiri Kosina
>>
>> Thanks for working on this! If others are happy with the setup_data
>> approach, I think this is fine.
>
> This is for x86 folks to decide. I hope my original CC covers this, so
> let's wait for their verdict.
>
>> My only concern is confusion over seeing SETUP_KASLR that was added by a
>> boot loader.
>
> Well, so you are concerned about bootloader that is evil on purpose?
No, no; I agree: a malicious boot loader is a lost cause. I mean
mostly from a misbehavior perspective. Like, someone sees "kaslr" in
the setup args and thinks they can set it to 1 and boot a kernel, etc.
Or they set it to 0, but they lack HIBERNATION and "1" gets appended,
but the setup_data parser sees the boot-loader one set to 0, etc. I'm
just curious if we should avoid getting some poor system into a
confusing state.
>
> If you have such bootloader, you are screwed anyway, because it's free to
> setup asynchronous events that will corrupt your kernel anyway (DMA that
> will happen only after the loaded kernel is already active, for example).
> If you want to avoid evil bootloaders, secure boot is currently The
> option, I am afraid.
>
>> Another way to handle it might be to do some kind of relocs-like poking
>> of a value into the decompressed kernel?
>
> This is so hackish that I'd like to avoid it in favor of the boot params
> aproach as much as possbile :)
Yeah, I think so too. :)
>
> [ ... snip ... ]
>> > diff --git a/arch/x86/boot/compressed/aslr.c
>> > b/arch/x86/boot/compressed/aslr.c
>> > index bb13763..d9d1da9 100644
>> > --- a/arch/x86/boot/compressed/aslr.c
>> > +++ b/arch/x86/boot/compressed/aslr.c
>> > @@ -14,6 +14,13 @@
>> > static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
>> > LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
>> >
>> > +struct kaslr_setup_data {
>>
>> Should this be "static"?
>
> Good catch. So let's wait what x86 folks have to say. I'll either update
> in in v3, or hopefully someone will fix this when applying the patch for
> -tip.
Great!
-Kees
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Fri, 13 Feb 2015, Kees Cook wrote:
> > Commit e2b32e678 ("x86, kaslr: randomize module base load address") makes
> > the base address for module to be unconditionally randomized in case when
> > CONFIG_RANDOMIZE_BASE is defined and "nokaslr" option isn't present on the
> > commandline.
> >
> > This is not consistent with how choose_kernel_location() decides whether
> > it will randomize kernel load base.
> >
> > Namely, CONFIG_HIBERNATION disables kASLR (unless "kaslr" option is
> > explicitly specified on kernel commandline), which makes the state space
> > larger than what module loader is looking at. IOW CONFIG_HIBERNATION &&
> > CONFIG_RANDOMIZE_BASE is a valid config option, kASLR wouldn't be applied
> > by default in that case, but module loader is not aware of that.
> >
> > Instead of fixing the logic in module.c, this patch takes more generic
> > aproach. It introduces a new bootparam setup data_type SETUP_KASLR and
> > uses that to pass the information whether kaslr has been applied during
> > kernel decompression, and sets a global 'kaslr_enabled' variable
> > accordingly, so that any kernel code (module loading, livepatching, ...)
> > can make decisions based on its value.
> >
> > x86 module loader is converted to make use of this flag.
> >
> > Signed-off-by: Jiri Kosina
>
> Thanks for working on this! If others are happy with the setup_data
> approach, I think this is fine.
This is for x86 folks to decide. I hope my original CC covers this, so
let's wait for their verdict.
> My only concern is confusion over seeing SETUP_KASLR that was added by a
> boot loader.
Well, so you are concerned about bootloader that is evil on purpose?
If you have such bootloader, you are screwed anyway, because it's free to
setup asynchronous events that will corrupt your kernel anyway (DMA that
will happen only after the loaded kernel is already active, for example).
If you want to avoid evil bootloaders, secure boot is currently The
option, I am afraid.
> Another way to handle it might be to do some kind of relocs-like poking
> of a value into the decompressed kernel?
This is so hackish that I'd like to avoid it in favor of the boot params
aproach as much as possbile :)
[ ... snip ... ]
> > diff --git a/arch/x86/boot/compressed/aslr.c
> > b/arch/x86/boot/compressed/aslr.c
> > index bb13763..d9d1da9 100644
> > --- a/arch/x86/boot/compressed/aslr.c
> > +++ b/arch/x86/boot/compressed/aslr.c
> > @@ -14,6 +14,13 @@
> > static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> > LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> >
> > +struct kaslr_setup_data {
>
> Should this be "static"?
Good catch. So let's wait what x86 folks have to say. I'll either update
in in v3, or hopefully someone will fix this when applying the patch for
-tip.
Thanks,
--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86, kaslr: propagate base load address calculation
On Fri, Feb 13, 2015 at 7:04 AM, Jiri Kosina wrote:
> Commit e2b32e678 ("x86, kaslr: randomize module base load address") makes
> the base address for module to be unconditionally randomized in case when
> CONFIG_RANDOMIZE_BASE is defined and "nokaslr" option isn't present on the
> commandline.
>
> This is not consistent with how choose_kernel_location() decides whether
> it will randomize kernel load base.
>
> Namely, CONFIG_HIBERNATION disables kASLR (unless "kaslr" option is
> explicitly specified on kernel commandline), which makes the state space
> larger than what module loader is looking at. IOW CONFIG_HIBERNATION &&
> CONFIG_RANDOMIZE_BASE is a valid config option, kASLR wouldn't be applied
> by default in that case, but module loader is not aware of that.
>
> Instead of fixing the logic in module.c, this patch takes more generic
> aproach. It introduces a new bootparam setup data_type SETUP_KASLR and
> uses that to pass the information whether kaslr has been applied during
> kernel decompression, and sets a global 'kaslr_enabled' variable
> accordingly, so that any kernel code (module loading, livepatching, ...)
> can make decisions based on its value.
>
> x86 module loader is converted to make use of this flag.
>
> Signed-off-by: Jiri Kosina
Thanks for working on this! If others are happy with the setup_data
approach, I think this is fine. My only concern is confusion over
seeing SETUP_KASLR that was added by a boot loader.
Another way to handle it might be to do some kind of relocs-like
poking of a value into the decompressed kernel?
> ---
>
> v1 -> v2:
>
> Originally I just calculated the fact on the fly from difference between
> __START_KERNEL and &text, but Kees correctly pointed out that this doesn't
> properly catch the case when the offset is randomized to zero. I don't see
> a better option how to propagate the information from
> choose_kernel_location() to the decompressed kernel than introducing new
> bootparam setup type. Comments welcome.
>
> arch/x86/boot/compressed/aslr.c | 34 +-
> arch/x86/boot/compressed/misc.c | 3 ++-
> arch/x86/boot/compressed/misc.h | 6 --
> arch/x86/include/asm/page_types.h | 3 +++
> arch/x86/include/uapi/asm/bootparam.h | 1 +
> arch/x86/kernel/module.c | 11 ++-
> arch/x86/kernel/setup.c | 10 ++
> 7 files changed, 55 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
> index bb13763..d9d1da9 100644
> --- a/arch/x86/boot/compressed/aslr.c
> +++ b/arch/x86/boot/compressed/aslr.c
> @@ -14,6 +14,13 @@
> static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
>
> +struct kaslr_setup_data {
Should this be "static"?
> +__u64 next;
> +__u32 type;
> +__u32 len;
> +__u8 data[1];
> +} kaslr_setup_data;
> +
> #define I8254_PORT_CONTROL 0x43
> #define I8254_PORT_COUNTER00x40
> #define I8254_CMD_READBACK 0xC0
> @@ -295,7 +302,29 @@ static unsigned long find_random_addr(unsigned long
> minimum,
> return slots_fetch_random();
> }
>
> -unsigned char *choose_kernel_location(unsigned char *input,
> +static void add_kaslr_setup_data(struct boot_params *params, __u8 enabled)
> +{
> + struct setup_data *data;
> +
> + kaslr_setup_data.type = SETUP_KASLR;
> + kaslr_setup_data.len = 1;
> + kaslr_setup_data.next = 0;
> + kaslr_setup_data.data[0] = enabled;
> +
> + data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
> +
> + while (data && data->next)
> + data = (struct setup_data *)(unsigned long)data->next;
> +
> + if (data)
> + data->next = (unsigned long)&kaslr_setup_data;
> + else
> + params->hdr.setup_data = (unsigned long)&kaslr_setup_data;
> +
> +}
> +
> +unsigned char *choose_kernel_location(struct boot_params *params,
> + unsigned char *input,
> unsigned long input_size,
> unsigned char *output,
> unsigned long output_size)
> @@ -306,14 +335,17 @@ unsigned char *choose_kernel_location(unsigned char
> *input,
> #ifdef CONFIG_HIBERNATION
> if (!cmdline_find_option_bool("kaslr")) {
> debug_putstr("KASLR disabled by default...\n");
> + add_kaslr_setup_data(params, 0);
> goto out;
> }
> #else
> if (cmdline_find_option_bool("nokaslr")) {
> debug_putstr("KASLR disabled by cmdline...\n");
> + add_kaslr_setup_data(params, 0);
> goto out;
> }
> #endif
> + add_kaslr_setup_data(params, 1);
>
> /* Record the various known unsafe memory ranges. */
> mem_avoid_init
[PATCH v2] x86, kaslr: propagate base load address calculation
Commit e2b32e678 ("x86, kaslr: randomize module base load address") makes
the base address for module to be unconditionally randomized in case when
CONFIG_RANDOMIZE_BASE is defined and "nokaslr" option isn't present on the
commandline.
This is not consistent with how choose_kernel_location() decides whether
it will randomize kernel load base.
Namely, CONFIG_HIBERNATION disables kASLR (unless "kaslr" option is
explicitly specified on kernel commandline), which makes the state space
larger than what module loader is looking at. IOW CONFIG_HIBERNATION &&
CONFIG_RANDOMIZE_BASE is a valid config option, kASLR wouldn't be applied
by default in that case, but module loader is not aware of that.
Instead of fixing the logic in module.c, this patch takes more generic
aproach. It introduces a new bootparam setup data_type SETUP_KASLR and
uses that to pass the information whether kaslr has been applied during
kernel decompression, and sets a global 'kaslr_enabled' variable
accordingly, so that any kernel code (module loading, livepatching, ...)
can make decisions based on its value.
x86 module loader is converted to make use of this flag.
Signed-off-by: Jiri Kosina
---
v1 -> v2:
Originally I just calculated the fact on the fly from difference between
__START_KERNEL and &text, but Kees correctly pointed out that this doesn't
properly catch the case when the offset is randomized to zero. I don't see
a better option how to propagate the information from
choose_kernel_location() to the decompressed kernel than introducing new
bootparam setup type. Comments welcome.
arch/x86/boot/compressed/aslr.c | 34 +-
arch/x86/boot/compressed/misc.c | 3 ++-
arch/x86/boot/compressed/misc.h | 6 --
arch/x86/include/asm/page_types.h | 3 +++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/module.c | 11 ++-
arch/x86/kernel/setup.c | 10 ++
7 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index bb13763..d9d1da9 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -14,6 +14,13 @@
static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
+struct kaslr_setup_data {
+__u64 next;
+__u32 type;
+__u32 len;
+__u8 data[1];
+} kaslr_setup_data;
+
#define I8254_PORT_CONTROL 0x43
#define I8254_PORT_COUNTER00x40
#define I8254_CMD_READBACK 0xC0
@@ -295,7 +302,29 @@ static unsigned long find_random_addr(unsigned long
minimum,
return slots_fetch_random();
}
-unsigned char *choose_kernel_location(unsigned char *input,
+static void add_kaslr_setup_data(struct boot_params *params, __u8 enabled)
+{
+ struct setup_data *data;
+
+ kaslr_setup_data.type = SETUP_KASLR;
+ kaslr_setup_data.len = 1;
+ kaslr_setup_data.next = 0;
+ kaslr_setup_data.data[0] = enabled;
+
+ data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
+
+ while (data && data->next)
+ data = (struct setup_data *)(unsigned long)data->next;
+
+ if (data)
+ data->next = (unsigned long)&kaslr_setup_data;
+ else
+ params->hdr.setup_data = (unsigned long)&kaslr_setup_data;
+
+}
+
+unsigned char *choose_kernel_location(struct boot_params *params,
+ unsigned char *input,
unsigned long input_size,
unsigned char *output,
unsigned long output_size)
@@ -306,14 +335,17 @@ unsigned char *choose_kernel_location(unsigned char
*input,
#ifdef CONFIG_HIBERNATION
if (!cmdline_find_option_bool("kaslr")) {
debug_putstr("KASLR disabled by default...\n");
+ add_kaslr_setup_data(params, 0);
goto out;
}
#else
if (cmdline_find_option_bool("nokaslr")) {
debug_putstr("KASLR disabled by cmdline...\n");
+ add_kaslr_setup_data(params, 0);
goto out;
}
#endif
+ add_kaslr_setup_data(params, 1);
/* Record the various known unsafe memory ranges. */
mem_avoid_init((unsigned long)input, input_size,
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index dcc1c53..5aecf56 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -399,7 +399,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode,
memptr heap,
* the entire decompressed kernel plus relocation table, or the
* entire decompressed kernel plus .bss and .brk sections.
*/
- output = choose_kernel_location(input_data, input_len, output,
+ output = choose_kernel_location(r
