| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150228105049.GA11038@pd.tnic>
Date: Sat, 28 Feb 2015 11:50:49 +0100
From: Borislav Petkov <bp@...e.de>
To: "H. Peter Anvin" <hpa@...or.com>,
Kees Cook <keescook@...omium.org>,
Jiri Kosina <jkosina@...e.cz>
Cc: Ingo Molnar <mingo@...nel.org>, Huang Ying <ying.huang@...el.com>,
Jiri Kosina <jkosina@...e.cz>,
LKML <linux-kernel@...r.kernel.org>, LKP ML <lkp@...org>,
x86-ml <x86@...nel.org>, Dave Young <dyoung@...hat.com>,
Matt Fleming <matt@...eblueprint.co.uk>,
Andy Lutomirski <luto@...capital.net>
Subject: Re: [LKP] [x86/mm/ASLR] f47233c2d34: WARNING: CPU: 0 PID: 1 at
arch/x86/mm/ioremap.c:63 __ioremap_check_ram+0x445/0x4a0()
On Thu, Feb 26, 2015 at 01:16:17PM +0100, Borislav Petkov wrote:
> The proper fix should be to say, don't ioremap setup_data which is
> kernel memory but I'm not sure I have a good idea at the moment how to
> do that *without* ioremapping the thing to inspect it first...
>
> More hmm...
Yeah, too many hmms means this still needed staring at to find out what
exactly the problem is. And the problem is that allocating that struct
setup_data statically in arch/x86/boot/compressed/aslr.c works only by
chance, when the kernel decompressing doesn't overwrite that memory.
One thing that we could do is to stick it right below LOAD_PHYSICAL_ADDR
(16M by default) which is the miminum physical address for the kernel to
be loaded at and kaslr pays attention to.
I.e., this struct setup_data thing lands then here:
[ 0.000000] parse_setup_data: data: 0xffffe0 (va: ffffffffff200fe0) { next: 0x0, type: 0x5, len: 17, data[0]: 0x0 }
which is 16M - 2*sizeof(struct setup_data). Yeah, I left some room
there.
Now, this approach works but I'm not sure whether this is how we want to
be passing setup_data stuff from arch/x86/boot/ to kernel proper so I'd
like to hear some more experienced opinions please...
Thanks!
---
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 7083c16cccba..9f64c64e3ebe 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -14,12 +14,7 @@
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;
+struct setup_data *ksd;
#define I8254_PORT_CONTROL 0x43
#define I8254_PORT_COUNTER0 0x40
@@ -302,14 +297,20 @@ static unsigned long find_random_addr(unsigned long minimum,
return slots_fetch_random();
}
-static void add_kaslr_setup_data(struct boot_params *params, __u8 enabled)
+static void add_kaslr_setup_data(struct boot_params *params,
+ u8 *output, __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;
+ /*
+ * Stick it right under LOAD_PHYSICAL_ADDR
+ */
+ ksd = (struct setup_data *)(output - 2 * sizeof(struct setup_data));
+
+ ksd->type = SETUP_KASLR;
+ ksd->len = 1;
+ ksd->next = 0;
+ ksd->data[0] = enabled;
data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
@@ -317,10 +318,9 @@ static void add_kaslr_setup_data(struct boot_params *params, __u8 enabled)
data = (struct setup_data *)(unsigned long)data->next;
if (data)
- data->next = (unsigned long)&kaslr_setup_data;
+ data->next = (unsigned long)ksd;
else
- params->hdr.setup_data = (unsigned long)&kaslr_setup_data;
-
+ params->hdr.setup_data = (unsigned long)ksd;
}
unsigned char *choose_kernel_location(struct boot_params *params,
@@ -335,17 +335,17 @@ unsigned char *choose_kernel_location(struct boot_params *params,
#ifdef CONFIG_HIBERNATION
if (!cmdline_find_option_bool("kaslr")) {
debug_putstr("KASLR disabled by default...\n");
- add_kaslr_setup_data(params, 0);
+ add_kaslr_setup_data(params, output, 0);
goto out;
}
#else
if (cmdline_find_option_bool("nokaslr")) {
debug_putstr("KASLR disabled by cmdline...\n");
- add_kaslr_setup_data(params, 0);
+ add_kaslr_setup_data(params, output, 0);
goto out;
}
#endif
- add_kaslr_setup_data(params, 1);
+ add_kaslr_setup_data(params, output, 1);
/* Record the various known unsafe memory ranges. */
mem_avoid_init((unsigned long)input, input_size,
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc9317286e..d3b34df6e539 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,11 @@ static void __init reserve_initrd(void)
static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
{
- kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
+ struct setup_data *kdata;
+
+ kdata = early_memremap(pa_data, data_len);
+ kaslr_enabled = kdata->data[0];
+ early_iounmap(kdata, data_len);
}
static void __init parse_setup_data(void)
---
--
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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists