Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 2, 2015 at 12:25 PM, Borislav Petkov wrote:
> unsigned char *choose_kernel_location(struct boot_params *params,
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
> b/arch/x86/boot/compressed/vmlinux.lds.S
> index 34d047c98284..26d62f4b27b9 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -29,6 +29,10 @@ SECTIONS
> .rodata..compressed : {
> *(.rodata..compressed)
> }
> + .setup_data : {
> + _setup_data = . ;
> + *(.setup_data)
> + }
> .text : {
> _text = .; /* Text */
> *(.text)
That does not help, we will still have overlap between copied Zo
vmlinux and decompressed Vo vmlinux
because the Zo vmlinux is copied to middle of buffer instead of end of
the buffer.
definition:
Zo vmliunx: arch/x86/boot/compressed/vmlinux, from
arch/x86/boot/compressed/vmlinux.lds.S
Vo vmlinux: vmlinux from arch/x86/kernel/vmlinux.lds.S
BTW, found more problem about run_size for kaslr, we should use
init_size instead.
Here the three patches that should go into v4.0. Help Ingo will be
happy with updated change log.
Thanks
Yinghai
Subject: [PATCH] x86, boot, aslr: Use init_size instead of run_size
commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
introduced one run_size for kaslr.
We do not need to have home grown run_size.
We should use real runtime size (include copy/decompress) aka init_size
Please check arch/x86/boot/header.S about init_size for detail.
Fixes: e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
Cc: "H. Peter Anvin"
Cc: Josh Triplett
Cc: Matt Fleming
Cc: Kees Cook
Cc: Andrew Morton
Cc: Ard Biesheuvel
Cc: Junjie Mao
Signed-off-by: Yinghai Lu
---
arch/x86/boot/compressed/Makefile |4 ---
arch/x86/boot/compressed/head_32.S |5 +---
arch/x86/boot/compressed/head_64.S |5
arch/x86/boot/compressed/misc.c| 15 ++---
arch/x86/boot/compressed/mkpiggy.c |9 +--
arch/x86/tools/calc_run_size.sh| 42 -
6 files changed, 13 insertions(+), 67 deletions(-)
Index: linux-2.6/arch/x86/boot/compressed/Makefile
===
--- linux-2.6.orig/arch/x86/boot/compressed/Makefile
+++ linux-2.6/arch/x86/boot/compressed/Makefile
@@ -92,10 +92,8 @@ suffix-$(CONFIG_KERNEL_XZ) := xz
suffix-$(CONFIG_KERNEL_LZO) := lzo
suffix-$(CONFIG_KERNEL_LZ4) := lz4
-RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \
- $(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh)
quiet_cmd_mkpiggy = MKPIGGY $@
- cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false )
+ cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
targets += piggy.S
$(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
Index: linux-2.6/arch/x86/boot/compressed/head_32.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_32.S
+++ linux-2.6/arch/x86/boot/compressed/head_32.S
@@ -208,8 +208,7 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
/* push arguments for decompress_kernel: */
- pushl $z_run_size /* size of kernel with .bss and .brk */
- pushl $z_output_len /* decompressed length, end of relocs */
+ pushl $z_output_len /* decompressed length */
leal z_extract_offset_negative(%ebx), %ebp
pushl %ebp /* output address */
pushl $z_input_len /* input_len */
@@ -219,7 +218,7 @@ relocated:
pushl %eax /* heap area */
pushl %esi /* real mode pointer */
call decompress_kernel /* returns kernel location in %eax */
- addl $28, %esp
+ addl $24, %esp
/*
* Jump to the decompressed kernel.
Index: linux-2.6/arch/x86/boot/compressed/head_64.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_64.S
+++ linux-2.6/arch/x86/boot/compressed/head_64.S
@@ -403,16 +403,13 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
pushq %rsi /* Save the real mode argument */
- movq $z_run_size, %r9 /* size of kernel with .bss and .brk */
- pushq %r9
movq %rsi, %rdi /* real mode address */
leaq boot_heap(%rip), %rsi /* malloc area for uncompression */
leaq input_data(%rip), %rdx /* input_data */
movl $z_input_len, %ecx /* input_len */
movq %rbp, %r8 /* output target address */
- movq $z_output_len, %r9 /* decompressed length, end of relocs */
+ movq $z_output_len, %r9 /* decompressed length */
call decompress_kernel /* returns kernel location in %rax */
- popq %r9
popq %rsi
/*
Index: linux-2.6/arch/x86/boot/compressed/misc.c
===
--- linux-2.6.orig/arch/x86/boot/compressed/misc.c
+++ linux-2.6/arch/x86/boot/compressed/misc.c
@@ -370,10 +370,10 @@ asmlinkage __visible void *decompress_ke
Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 02, 2015 at 10:58:23AM -0800, Yinghai Lu wrote:
> On Mon, Mar 2, 2015 at 6:53 AM, Borislav Petkov wrote:
> > Well, it seems to work here but it still doesn't look reliable enough to
> > me. And this addon_zo thing of arbitrary 256K is strange.
>
> Thanks for check that out.
>
> That is not arbitrary number. Need to make that bigger than _end - _rodata
>
> > objdump -t arch/x86/boot/compressed/vmlinux | grep \ _end
> 00996000 g .pgtable _end
> > objdump -t arch/x86/boot/compressed/vmlinux | grep \ _rodata
> 00981780 g .rodata _rodata
>
> We only get that size after arch/x86/boot/compressed/vmlinux
>
> but need number during building kernel vmlinux.
>
> other way would be just increase init_size in arch/x86/boot/header.S
> instead of put it BRK area.
Ok, I'll take a look at this more tomorrow, on a clear head, but why
can't we make a special, explicit section which is not touched by
the early decompression and relocation code and which we can use for
setup_data only?
IOW, something like that although that doesn't work yet:
---
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 7083c16cccba..01d5ddf1d22f 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -14,12 +14,12 @@
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;
+extern char _setup_data[];
+struct setup_data __attribute__((section (".setup_data"))) ksd = {
+ .type = SETUP_KASLR,
+ .len= 1,
+ .next = 0,
+};
#define I8254_PORT_CONTROL 0x43
#define I8254_PORT_COUNTER00x40
@@ -306,10 +306,7 @@ 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;
+ ksd.data[0] = enabled;
data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
@@ -317,10 +314,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)_setup_data;
+ data->next = (unsigned long)
else
- params->hdr.setup_data = (unsigned long)_setup_data;
-
+ params->hdr.setup_data = (unsigned long)
}
unsigned char *choose_kernel_location(struct boot_params *params,
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
b/arch/x86/boot/compressed/vmlinux.lds.S
index 34d047c98284..26d62f4b27b9 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -29,6 +29,10 @@ SECTIONS
.rodata..compressed : {
*(.rodata..compressed)
}
+ .setup_data : {
+ _setup_data = . ;
+ *(.setup_data)
+ }
.text : {
_text = .; /* Text */
*(.text)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc9317286e..0978c61f84bd 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 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 2, 2015 at 6:53 AM, Borislav Petkov wrote: > Well, it seems to work here but it still doesn't look reliable enough to > me. And this addon_zo thing of arbitrary 256K is strange. Thanks for check that out. That is not arbitrary number. Need to make that bigger than _end - _rodata > objdump -t arch/x86/boot/compressed/vmlinux | grep \ _end 00996000 g .pgtable _end > objdump -t arch/x86/boot/compressed/vmlinux | grep \ _rodata 00981780 g .rodata _rodata We only get that size after arch/x86/boot/compressed/vmlinux but need number during building kernel vmlinux. other way would be just increase init_size in arch/x86/boot/header.S instead of put it BRK area. Thanks Yinghai -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 02, 2015 at 03:04:30AM -0800, Yinghai Lu wrote: > We can not assume that range is safe to use. > > Please check attach one that should fix the problem really. Well, it seems to work here but it still doesn't look reliable enough to me. And this addon_zo thing of arbitrary 256K is strange. What we should do instead and IMHO is have a special setup_data section located right above .text where struct setup_data things are put in so that they never get overwritten. This should be the cleanest IMHO. It is hpa's call in the end of the day anyway. Thanks. -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, 28 Feb, at 06:17:32PM, Yinghai Lu wrote:
> We should access variable with referrence instead of using physical
> address as value.
>
> Cc: Matt Fleming
> Cc: Borislav Petkov
> Signed-off-by: Yinghai Lu
> ---
> arch/x86/kernel/setup.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 98dc931..05d444f 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -429,7 +429,13 @@ 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));
> + /* kaslr_setup_data is defined in aslr.c */
> + unsigned char *data;
> + unsigned long offset = sizeof(struct setup_data);
> +
> + data = early_memremap(pa_data, offset + 1);
> + kaslr_enabled = *(data + offset);
> + early_memunmap(data, offset + 1);
> }
ACK in principle, but could we change 'offset' to 'size' or 'len' along
with the commit log updates people have suggested?
--
Matt Fleming, Intel Open Source Technology Center
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 2, 2015 at 12:56 AM, Borislav Petkov wrote:
> On Sun, Mar 01, 2015 at 12:41:10PM -0800, Yinghai Lu wrote:
>> Does not look safe yet...
>
> Why?
We can not assume that range is safe to use.
Please check attach one that should fix the problem really.
Thanks
Yinghai
Subject: [PATCH] x86, boot: keep data from boot stage to kernel stage.
Need to
1. move compressed kernel close the end of buffer instead of middle of it.
2. use BRK to extend init_size so no one from kernel bss and brk will
touch the data region from boot/compressed/misc.c
Signed-off-by: Yinghai Lu
---
arch/x86/boot/compressed/head_32.S | 13 +++--
arch/x86/boot/compressed/head_64.S | 10 --
arch/x86/boot/compressed/misc.c|4
arch/x86/boot/compressed/mkpiggy.c |3 ---
arch/x86/include/asm/boot.h|2 ++
arch/x86/kernel/asm-offsets.c |1 +
arch/x86/kernel/setup.c|3 +++
7 files changed, 29 insertions(+), 7 deletions(-)
Index: linux-2.6/arch/x86/boot/compressed/head_64.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_64.S
+++ linux-2.6/arch/x86/boot/compressed/head_64.S
@@ -102,7 +102,10 @@ ENTRY(startup_32)
1:
/* Target address to relocate to for decompression */
- addl $z_extract_offset, %ebx
+ movl BP_init_size(%esi), %eax
+ subl $_end, %eax
+ andl $(~(4096-1)), %eax
+ addl %eax, %ebx
/*
* Prepare for entering 64 bit mode
@@ -330,7 +333,10 @@ preferred_addr:
1:
/* Target address to relocate to for decompression */
- leaq z_extract_offset(%rbp), %rbx
+ movl BP_init_size(%rsi), %ebx
+ subl $_end, %ebx
+ andl $(~(4096-1)), %ebx
+ addq %rbp, %rbx
/* Set up the stack */
leaq boot_stack_end(%rbx), %rsp
Index: linux-2.6/arch/x86/kernel/asm-offsets.c
===
--- linux-2.6.orig/arch/x86/kernel/asm-offsets.c
+++ linux-2.6/arch/x86/kernel/asm-offsets.c
@@ -66,6 +66,7 @@ void common(void) {
OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
OFFSET(BP_version, boot_params, hdr.version);
OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
+ OFFSET(BP_init_size, boot_params, hdr.init_size);
OFFSET(BP_pref_address, boot_params, hdr.pref_address);
OFFSET(BP_code32_start, boot_params, hdr.code32_start);
Index: linux-2.6/arch/x86/kernel/setup.c
===
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -111,6 +111,7 @@
#include
#include
#include
+#include
/*
* max_low_pfn_mapped: highest direct mapped pfn under 4GB
@@ -144,6 +145,8 @@ int default_check_phys_apicid_present(in
}
#endif
+RESERVE_BRK(addon_zo, BOOT_ADDON_ZO_SIZE);
+
struct boot_params boot_params;
/*
Index: linux-2.6/arch/x86/boot/compressed/head_32.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_32.S
+++ linux-2.6/arch/x86/boot/compressed/head_32.S
@@ -148,7 +148,10 @@ preferred_addr:
1:
/* Target address to relocate to for decompression */
- addl $z_extract_offset, %ebx
+ movlBP_init_size(%esi), %eax
+ subl$_end, %eax
+ andl$(~(4096-1)), %eax
+ addl%eax, %ebx
/* Set up the stack */
leal boot_stack_end(%ebx), %esp
@@ -210,8 +213,14 @@ relocated:
/* push arguments for decompress_kernel: */
pushl $z_run_size /* size of kernel with .bss and .brk */
pushl $z_output_len /* decompressed length, end of relocs */
- leal z_extract_offset_negative(%ebx), %ebp
+
+ movlBP_init_size(%esi), %eax
+ subl$_end, %eax
+ andl$(~(4096-1)), %eax
+ movl %ebx, %ebp
+ subl %eax, %ebp
pushl %ebp /* output address */
+
pushl $z_input_len /* input_len */
leal input_data(%ebx), %eax
pushl %eax /* input_data */
Index: linux-2.6/arch/x86/boot/compressed/mkpiggy.c
===
--- linux-2.6.orig/arch/x86/boot/compressed/mkpiggy.c
+++ linux-2.6/arch/x86/boot/compressed/mkpiggy.c
@@ -85,9 +85,6 @@ int main(int argc, char *argv[])
printf("z_output_len = %lu\n", (unsigned long)olen);
printf(".globl z_extract_offset\n");
printf("z_extract_offset = 0x%lx\n", offs);
- /* z_extract_offset_negative allows simplification of head_32.S */
- printf(".globl z_extract_offset_negative\n");
- printf("z_extract_offset_negative = -0x%lx\n", offs);
printf(".globl z_run_size\n");
printf("z_run_size = %lu\n", run_size);
Index: linux-2.6/arch/x86/boot/compressed/misc.c
===
--- linux-2.6.orig/arch/x86/boot/compressed/misc.c
+++ linux-2.6/arch/x86/boot/compressed/misc.c
@@ -366,6 +366,8 @@ static void parse_elf(void *output)
free(phdrs);
}
+extern char _rodata[], _end[];
+
asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
unsigned char *input_data,
unsigned long
Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, 28 Feb 2015, Yinghai Lu wrote:
> We should access variable with referrence instead of using physical
> address as value.
>
> Cc: Matt Fleming
> Cc: Borislav Petkov
> Signed-off-by: Yinghai Lu
Acked-by: Jiri Kosina
Thanks for fixing my brainfart. This is not fixing the problem with the
later variable overwrite, but definitely needs to go in as well.
Thanks.
> ---
> arch/x86/kernel/setup.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 98dc931..05d444f 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -429,7 +429,13 @@ 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));
> + /* kaslr_setup_data is defined in aslr.c */
> + unsigned char *data;
> + unsigned long offset = sizeof(struct setup_data);
> +
> + data = early_memremap(pa_data, offset + 1);
> + kaslr_enabled = *(data + offset);
> + early_memunmap(data, offset + 1);
> }
>
> static void __init parse_setup_data(void)
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 12:41:10PM -0800, Yinghai Lu wrote: > Does not look safe yet... Why? -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 2, 2015 at 12:25 PM, Borislav Petkov b...@suse.de wrote:
unsigned char *choose_kernel_location(struct boot_params *params,
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
b/arch/x86/boot/compressed/vmlinux.lds.S
index 34d047c98284..26d62f4b27b9 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -29,6 +29,10 @@ SECTIONS
.rodata..compressed : {
*(.rodata..compressed)
}
+ .setup_data : {
+ _setup_data = . ;
+ *(.setup_data)
+ }
.text : {
_text = .; /* Text */
*(.text)
That does not help, we will still have overlap between copied Zo
vmlinux and decompressed Vo vmlinux
because the Zo vmlinux is copied to middle of buffer instead of end of
the buffer.
definition:
Zo vmliunx: arch/x86/boot/compressed/vmlinux, from
arch/x86/boot/compressed/vmlinux.lds.S
Vo vmlinux: vmlinux from arch/x86/kernel/vmlinux.lds.S
BTW, found more problem about run_size for kaslr, we should use
init_size instead.
Here the three patches that should go into v4.0. Help Ingo will be
happy with updated change log.
Thanks
Yinghai
Subject: [PATCH] x86, boot, aslr: Use init_size instead of run_size
commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd)
introduced one run_size for kaslr.
We do not need to have home grown run_size.
We should use real runtime size (include copy/decompress) aka init_size
Please check arch/x86/boot/header.S about init_size for detail.
Fixes: e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd)
Cc: H. Peter Anvin h...@zytor.com
Cc: Josh Triplett j...@joshtriplett.org
Cc: Matt Fleming matt.flem...@intel.com
Cc: Kees Cook keesc...@chromium.org
Cc: Andrew Morton a...@linux-foundation.org
Cc: Ard Biesheuvel ard.biesheu...@linaro.org
Cc: Junjie Mao eternal@gmail.com
Signed-off-by: Yinghai Lu ying...@kernel.org
---
arch/x86/boot/compressed/Makefile |4 ---
arch/x86/boot/compressed/head_32.S |5 +---
arch/x86/boot/compressed/head_64.S |5
arch/x86/boot/compressed/misc.c| 15 ++---
arch/x86/boot/compressed/mkpiggy.c |9 +--
arch/x86/tools/calc_run_size.sh| 42 -
6 files changed, 13 insertions(+), 67 deletions(-)
Index: linux-2.6/arch/x86/boot/compressed/Makefile
===
--- linux-2.6.orig/arch/x86/boot/compressed/Makefile
+++ linux-2.6/arch/x86/boot/compressed/Makefile
@@ -92,10 +92,8 @@ suffix-$(CONFIG_KERNEL_XZ) := xz
suffix-$(CONFIG_KERNEL_LZO) := lzo
suffix-$(CONFIG_KERNEL_LZ4) := lz4
-RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \
- $(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh)
quiet_cmd_mkpiggy = MKPIGGY $@
- cmd_mkpiggy = $(obj)/mkpiggy $ $(RUN_SIZE) $@ || ( rm -f $@ ; false )
+ cmd_mkpiggy = $(obj)/mkpiggy $ $@ || ( rm -f $@ ; false )
targets += piggy.S
$(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
Index: linux-2.6/arch/x86/boot/compressed/head_32.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_32.S
+++ linux-2.6/arch/x86/boot/compressed/head_32.S
@@ -208,8 +208,7 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
/* push arguments for decompress_kernel: */
- pushl $z_run_size /* size of kernel with .bss and .brk */
- pushl $z_output_len /* decompressed length, end of relocs */
+ pushl $z_output_len /* decompressed length */
leal z_extract_offset_negative(%ebx), %ebp
pushl %ebp /* output address */
pushl $z_input_len /* input_len */
@@ -219,7 +218,7 @@ relocated:
pushl %eax /* heap area */
pushl %esi /* real mode pointer */
call decompress_kernel /* returns kernel location in %eax */
- addl $28, %esp
+ addl $24, %esp
/*
* Jump to the decompressed kernel.
Index: linux-2.6/arch/x86/boot/compressed/head_64.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_64.S
+++ linux-2.6/arch/x86/boot/compressed/head_64.S
@@ -403,16 +403,13 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
pushq %rsi /* Save the real mode argument */
- movq $z_run_size, %r9 /* size of kernel with .bss and .brk */
- pushq %r9
movq %rsi, %rdi /* real mode address */
leaq boot_heap(%rip), %rsi /* malloc area for uncompression */
leaq input_data(%rip), %rdx /* input_data */
movl $z_input_len, %ecx /* input_len */
movq %rbp, %r8 /* output target address */
- movq $z_output_len, %r9 /* decompressed length, end of relocs */
+ movq $z_output_len, %r9 /* decompressed length */
call decompress_kernel /* returns kernel location in %rax */
- popq %r9
popq %rsi
/*
Index: linux-2.6/arch/x86/boot/compressed/misc.c
===
---
Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 02, 2015 at 10:58:23AM -0800, Yinghai Lu wrote:
On Mon, Mar 2, 2015 at 6:53 AM, Borislav Petkov b...@suse.de wrote:
Well, it seems to work here but it still doesn't look reliable enough to
me. And this addon_zo thing of arbitrary 256K is strange.
Thanks for check that out.
That is not arbitrary number. Need to make that bigger than _end - _rodata
objdump -t arch/x86/boot/compressed/vmlinux | grep \ _end
00996000 g .pgtable _end
objdump -t arch/x86/boot/compressed/vmlinux | grep \ _rodata
00981780 g .rodata _rodata
We only get that size after arch/x86/boot/compressed/vmlinux
but need number during building kernel vmlinux.
other way would be just increase init_size in arch/x86/boot/header.S
instead of put it BRK area.
Ok, I'll take a look at this more tomorrow, on a clear head, but why
can't we make a special, explicit section which is not touched by
the early decompression and relocation code and which we can use for
setup_data only?
IOW, something like that although that doesn't work yet:
---
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 7083c16cccba..01d5ddf1d22f 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -14,12 +14,12 @@
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;
+extern char _setup_data[];
+struct setup_data __attribute__((section (.setup_data))) ksd = {
+ .type = SETUP_KASLR,
+ .len= 1,
+ .next = 0,
+};
#define I8254_PORT_CONTROL 0x43
#define I8254_PORT_COUNTER00x40
@@ -306,10 +306,7 @@ 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;
+ ksd.data[0] = enabled;
data = (struct setup_data *)(unsigned long)params-hdr.setup_data;
@@ -317,10 +314,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,
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
b/arch/x86/boot/compressed/vmlinux.lds.S
index 34d047c98284..26d62f4b27b9 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -29,6 +29,10 @@ SECTIONS
.rodata..compressed : {
*(.rodata..compressed)
}
+ .setup_data : {
+ _setup_data = . ;
+ *(.setup_data)
+ }
.text : {
_text = .; /* Text */
*(.text)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc9317286e..0978c61f84bd 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 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 02, 2015 at 03:04:30AM -0800, Yinghai Lu wrote: We can not assume that range is safe to use. Please check attach one that should fix the problem really. Well, it seems to work here but it still doesn't look reliable enough to me. And this addon_zo thing of arbitrary 256K is strange. What we should do instead and IMHO is have a special setup_data section located right above .text where struct setup_data things are put in so that they never get overwritten. This should be the cleanest IMHO. It is hpa's call in the end of the day anyway. Thanks. -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, 28 Feb, at 06:17:32PM, Yinghai Lu wrote:
We should access variable with referrence instead of using physical
address as value.
Cc: Matt Fleming matt.flem...@intel.com
Cc: Borislav Petkov b...@suse.de
Signed-off-by: Yinghai Lu ying...@kernel.org
---
arch/x86/kernel/setup.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..05d444f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ 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));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
ACK in principle, but could we change 'offset' to 'size' or 'len' along
with the commit log updates people have suggested?
--
Matt Fleming, Intel Open Source Technology Center
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 2, 2015 at 12:56 AM, Borislav Petkov b...@suse.de wrote:
On Sun, Mar 01, 2015 at 12:41:10PM -0800, Yinghai Lu wrote:
Does not look safe yet...
Why?
We can not assume that range is safe to use.
Please check attach one that should fix the problem really.
Thanks
Yinghai
Subject: [PATCH] x86, boot: keep data from boot stage to kernel stage.
Need to
1. move compressed kernel close the end of buffer instead of middle of it.
2. use BRK to extend init_size so no one from kernel bss and brk will
touch the data region from boot/compressed/misc.c
Signed-off-by: Yinghai Lu ying...@kernel.org
---
arch/x86/boot/compressed/head_32.S | 13 +++--
arch/x86/boot/compressed/head_64.S | 10 --
arch/x86/boot/compressed/misc.c|4
arch/x86/boot/compressed/mkpiggy.c |3 ---
arch/x86/include/asm/boot.h|2 ++
arch/x86/kernel/asm-offsets.c |1 +
arch/x86/kernel/setup.c|3 +++
7 files changed, 29 insertions(+), 7 deletions(-)
Index: linux-2.6/arch/x86/boot/compressed/head_64.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_64.S
+++ linux-2.6/arch/x86/boot/compressed/head_64.S
@@ -102,7 +102,10 @@ ENTRY(startup_32)
1:
/* Target address to relocate to for decompression */
- addl $z_extract_offset, %ebx
+ movl BP_init_size(%esi), %eax
+ subl $_end, %eax
+ andl $(~(4096-1)), %eax
+ addl %eax, %ebx
/*
* Prepare for entering 64 bit mode
@@ -330,7 +333,10 @@ preferred_addr:
1:
/* Target address to relocate to for decompression */
- leaq z_extract_offset(%rbp), %rbx
+ movl BP_init_size(%rsi), %ebx
+ subl $_end, %ebx
+ andl $(~(4096-1)), %ebx
+ addq %rbp, %rbx
/* Set up the stack */
leaq boot_stack_end(%rbx), %rsp
Index: linux-2.6/arch/x86/kernel/asm-offsets.c
===
--- linux-2.6.orig/arch/x86/kernel/asm-offsets.c
+++ linux-2.6/arch/x86/kernel/asm-offsets.c
@@ -66,6 +66,7 @@ void common(void) {
OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
OFFSET(BP_version, boot_params, hdr.version);
OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
+ OFFSET(BP_init_size, boot_params, hdr.init_size);
OFFSET(BP_pref_address, boot_params, hdr.pref_address);
OFFSET(BP_code32_start, boot_params, hdr.code32_start);
Index: linux-2.6/arch/x86/kernel/setup.c
===
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -111,6 +111,7 @@
#include asm/mce.h
#include asm/alternative.h
#include asm/prom.h
+#include asm/boot.h
/*
* max_low_pfn_mapped: highest direct mapped pfn under 4GB
@@ -144,6 +145,8 @@ int default_check_phys_apicid_present(in
}
#endif
+RESERVE_BRK(addon_zo, BOOT_ADDON_ZO_SIZE);
+
struct boot_params boot_params;
/*
Index: linux-2.6/arch/x86/boot/compressed/head_32.S
===
--- linux-2.6.orig/arch/x86/boot/compressed/head_32.S
+++ linux-2.6/arch/x86/boot/compressed/head_32.S
@@ -148,7 +148,10 @@ preferred_addr:
1:
/* Target address to relocate to for decompression */
- addl $z_extract_offset, %ebx
+ movlBP_init_size(%esi), %eax
+ subl$_end, %eax
+ andl$(~(4096-1)), %eax
+ addl%eax, %ebx
/* Set up the stack */
leal boot_stack_end(%ebx), %esp
@@ -210,8 +213,14 @@ relocated:
/* push arguments for decompress_kernel: */
pushl $z_run_size /* size of kernel with .bss and .brk */
pushl $z_output_len /* decompressed length, end of relocs */
- leal z_extract_offset_negative(%ebx), %ebp
+
+ movlBP_init_size(%esi), %eax
+ subl$_end, %eax
+ andl$(~(4096-1)), %eax
+ movl %ebx, %ebp
+ subl %eax, %ebp
pushl %ebp /* output address */
+
pushl $z_input_len /* input_len */
leal input_data(%ebx), %eax
pushl %eax /* input_data */
Index: linux-2.6/arch/x86/boot/compressed/mkpiggy.c
===
--- linux-2.6.orig/arch/x86/boot/compressed/mkpiggy.c
+++ linux-2.6/arch/x86/boot/compressed/mkpiggy.c
@@ -85,9 +85,6 @@ int main(int argc, char *argv[])
printf(z_output_len = %lu\n, (unsigned long)olen);
printf(.globl z_extract_offset\n);
printf(z_extract_offset = 0x%lx\n, offs);
- /* z_extract_offset_negative allows simplification of head_32.S */
- printf(.globl z_extract_offset_negative\n);
- printf(z_extract_offset_negative = -0x%lx\n, offs);
printf(.globl z_run_size\n);
printf(z_run_size = %lu\n, run_size);
Index: linux-2.6/arch/x86/boot/compressed/misc.c
===
--- linux-2.6.orig/arch/x86/boot/compressed/misc.c
+++ linux-2.6/arch/x86/boot/compressed/misc.c
@@ -366,6 +366,8 @@ static void parse_elf(void *output)
free(phdrs);
}
+extern char _rodata[], _end[];
+
asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, 28 Feb 2015, Yinghai Lu wrote:
We should access variable with referrence instead of using physical
address as value.
Cc: Matt Fleming matt.flem...@intel.com
Cc: Borislav Petkov b...@suse.de
Signed-off-by: Yinghai Lu ying...@kernel.org
Acked-by: Jiri Kosina jkos...@suse.cz
Thanks for fixing my brainfart. This is not fixing the problem with the
later variable overwrite, but definitely needs to go in as well.
Thanks.
---
arch/x86/kernel/setup.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..05d444f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ 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));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
static void __init parse_setup_data(void)
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 12:41:10PM -0800, Yinghai Lu wrote: Does not look safe yet... Why? -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Mon, Mar 2, 2015 at 6:53 AM, Borislav Petkov b...@suse.de wrote: Well, it seems to work here but it still doesn't look reliable enough to me. And this addon_zo thing of arbitrary 256K is strange. Thanks for check that out. That is not arbitrary number. Need to make that bigger than _end - _rodata objdump -t arch/x86/boot/compressed/vmlinux | grep \ _end 00996000 g .pgtable _end objdump -t arch/x86/boot/compressed/vmlinux | grep \ _rodata 00981780 g .rodata _rodata We only get that size after arch/x86/boot/compressed/vmlinux but need number during building kernel vmlinux. other way would be just increase init_size in arch/x86/boot/header.S instead of put it BRK area. Thanks Yinghai -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 1, 2015 at 12:29 PM, Borislav Petkov wrote:
> On Sun, Mar 01, 2015 at 12:24:08PM -0800, Yinghai Lu wrote:
>> static allocation in misc.c can not be used to kernel/head_64.S stage safely.
>
> Correct. One possibility that works is sticking it right below
> LOAD_PHYSICAL_ADDR:
>
> +static void add_kaslr_setup_data(struct boot_params *params,
> +u8 *output, __u8 enabled)
> {
> + /*
> +* Stick it right under LOAD_PHYSICAL_ADDR
> +*/
> + ksd = (struct setup_data *)(output - 2 * sizeof(struct setup_data));
>
> output is LOAD_PHYSICAL_ADDR AFAICT and is the minimum kASLR considers
> so right below it should work:
Does not look safe yet...
Let me think about it more.
Thanks
Yinghai
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 12:24:08PM -0800, Yinghai Lu wrote:
> static allocation in misc.c can not be used to kernel/head_64.S stage safely.
Correct. One possibility that works is sticking it right below
LOAD_PHYSICAL_ADDR:
+static void add_kaslr_setup_data(struct boot_params *params,
+u8 *output, __u8 enabled)
{
+ /*
+* Stick it right under LOAD_PHYSICAL_ADDR
+*/
+ ksd = (struct setup_data *)(output - 2 * sizeof(struct setup_data));
output is LOAD_PHYSICAL_ADDR AFAICT and is the minimum kASLR considers
so right below it should work:
https://lkml.kernel.org/r/20150228105049.ga11...@pd.tnic
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 1, 2015 at 11:49 AM, Borislav Petkov wrote:
> On Sun, Mar 01, 2015 at 11:27:48AM -0800, Yinghai Lu wrote:
>> other 7 should also address the problem in
>>http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com
>
> No, they don't:
>
> [0.00] parse_setup_data: data: 0x2206e50 (va: ff200e50) {
> next: 0x0, type: 0x0, len: 16, data[0]: 0x0 }
> [0.00] Unknown setup_data type: 0 ignored!
>
> --
ok, i get your point.
static allocation in misc.c can not be used to kernel/head_64.S stage safely.
Thanks
Yinghai
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 11:27:48AM -0800, Yinghai Lu wrote:
> other 7 should also address the problem in
>http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com
No, they don't:
[0.00] parse_setup_data: data: 0x2206e50 (va: ff200e50) { next:
0x0, type: 0x0, len: 16, data[0]: 0x0 }
[0.00] Unknown setup_data type: 0 ignored!
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 04:23:51PM +0100, Ingo Molnar wrote: > I think that's a different bug. > > parse_kaslr_setup() is simply bogus, it does: > > kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); Well, we found that while debugging the other issue too: https://lkml.kernel.org/r/20150228105049.ga11...@pd.tnic Scroll to the end. > So I'm inclined to apply Yinghai's fix, with a better changelog that > explains what happened ... > > Agreed? Yes, this fix needs a commit message which actually explains what's it fixing before it gets applied. Thanks. -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 1, 2015 at 7:23 AM, Ingo Molnar wrote: > > I think that's a different bug. > > parse_kaslr_setup() is simply bogus, it does: > > kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); > > which makes no sense whatsoever: it randomly enables (or disables, > depending on the physical address of the setup page) KASLR when it > meets a SETUP_KASLR record. > Yes, you are right. there are 8 patches in this series. x86, kaslr: get kaslr_enabled back correctly x86: Kill E820_RESERVED_KERN x86, efi: copy SETUP_EFI data and access directly x86, of: let add_dtb reserve by itself x86, boot: Add add_pci handler for SETUP_PCI x86: kill not used setup_data handling code x86, pci: convert SETUP_PCI data to list x86, pci: export SETUP_PCI data via sysfs other 7 should also address the problem in http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com the root cause is E820_RESERVED_KERN and setup_data handling will make E820 are not page aligned anymore. We should change setup_data handling. Please check updated version at git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-4.0 (with a fix for config without CONFIG_PCI). Thanks Yinghai -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
(Cc:-ed Jiri and Kees as well.) * Borislav Petkov wrote: > On Sat, Feb 28, 2015 at 06:40:39PM -0800, Yinghai Lu wrote: > > > > oh, no. the offending commit already got into linus tree. > > We're working on it, follow this thread: > > http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com I think that's a different bug. parse_kaslr_setup() is simply bogus, it does: kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); which makes no sense whatsoever: it randomly enables (or disables, depending on the physical address of the setup page) KASLR when it meets a SETUP_KASLR record. So I'm inclined to apply Yinghai's fix, with a better changelog that explains what happened ... Agreed? Thanks, Ingo -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, Feb 28, 2015 at 06:40:39PM -0800, Yinghai Lu wrote: > oh, no. the offending commit already got into linus tree. We're working on it, follow this thread: http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, Feb 28, 2015 at 06:40:39PM -0800, Yinghai Lu wrote: oh, no. the offending commit already got into linus tree. We're working on it, follow this thread: http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 1, 2015 at 7:23 AM, Ingo Molnar mi...@kernel.org wrote: I think that's a different bug. parse_kaslr_setup() is simply bogus, it does: kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); which makes no sense whatsoever: it randomly enables (or disables, depending on the physical address of the setup page) KASLR when it meets a SETUP_KASLR record. Yes, you are right. there are 8 patches in this series. x86, kaslr: get kaslr_enabled back correctly x86: Kill E820_RESERVED_KERN x86, efi: copy SETUP_EFI data and access directly x86, of: let add_dtb reserve by itself x86, boot: Add add_pci handler for SETUP_PCI x86: kill not used setup_data handling code x86, pci: convert SETUP_PCI data to list x86, pci: export SETUP_PCI data via sysfs other 7 should also address the problem in http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com the root cause is E820_RESERVED_KERN and setup_data handling will make E820 are not page aligned anymore. We should change setup_data handling. Please check updated version at git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-4.0 (with a fix for config without CONFIG_PCI). Thanks Yinghai -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 04:23:51PM +0100, Ingo Molnar wrote: I think that's a different bug. parse_kaslr_setup() is simply bogus, it does: kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); Well, we found that while debugging the other issue too: https://lkml.kernel.org/r/20150228105049.ga11...@pd.tnic Scroll to the end. So I'm inclined to apply Yinghai's fix, with a better changelog that explains what happened ... Agreed? Yes, this fix needs a commit message which actually explains what's it fixing before it gets applied. Thanks. -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 11:27:48AM -0800, Yinghai Lu wrote:
other 7 should also address the problem in
http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com
No, they don't:
[0.00] parse_setup_data: data: 0x2206e50 (va: ff200e50) { next:
0x0, type: 0x0, len: 16, data[0]: 0x0 }
[0.00] Unknown setup_data type: 0 ignored!
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
(Cc:-ed Jiri and Kees as well.) * Borislav Petkov b...@suse.de wrote: On Sat, Feb 28, 2015 at 06:40:39PM -0800, Yinghai Lu wrote: oh, no. the offending commit already got into linus tree. We're working on it, follow this thread: http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com I think that's a different bug. parse_kaslr_setup() is simply bogus, it does: kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data)); which makes no sense whatsoever: it randomly enables (or disables, depending on the physical address of the setup page) KASLR when it meets a SETUP_KASLR record. So I'm inclined to apply Yinghai's fix, with a better changelog that explains what happened ... Agreed? Thanks, Ingo -- 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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 1, 2015 at 11:49 AM, Borislav Petkov b...@suse.de wrote:
On Sun, Mar 01, 2015 at 11:27:48AM -0800, Yinghai Lu wrote:
other 7 should also address the problem in
http://lkml.kernel.org/r/1424929021.10337.24.ca...@intel.com
No, they don't:
[0.00] parse_setup_data: data: 0x2206e50 (va: ff200e50) {
next: 0x0, type: 0x0, len: 16, data[0]: 0x0 }
[0.00] Unknown setup_data type: 0 ignored!
--
ok, i get your point.
static allocation in misc.c can not be used to kernel/head_64.S stage safely.
Thanks
Yinghai
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 01, 2015 at 12:24:08PM -0800, Yinghai Lu wrote:
static allocation in misc.c can not be used to kernel/head_64.S stage safely.
Correct. One possibility that works is sticking it right below
LOAD_PHYSICAL_ADDR:
+static void add_kaslr_setup_data(struct boot_params *params,
+u8 *output, __u8 enabled)
{
+ /*
+* Stick it right under LOAD_PHYSICAL_ADDR
+*/
+ ksd = (struct setup_data *)(output - 2 * sizeof(struct setup_data));
output is LOAD_PHYSICAL_ADDR AFAICT and is the minimum kASLR considers
so right below it should work:
https://lkml.kernel.org/r/20150228105049.ga11...@pd.tnic
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sun, Mar 1, 2015 at 12:29 PM, Borislav Petkov b...@suse.de wrote:
On Sun, Mar 01, 2015 at 12:24:08PM -0800, Yinghai Lu wrote:
static allocation in misc.c can not be used to kernel/head_64.S stage safely.
Correct. One possibility that works is sticking it right below
LOAD_PHYSICAL_ADDR:
+static void add_kaslr_setup_data(struct boot_params *params,
+u8 *output, __u8 enabled)
{
+ /*
+* Stick it right under LOAD_PHYSICAL_ADDR
+*/
+ ksd = (struct setup_data *)(output - 2 * sizeof(struct setup_data));
output is LOAD_PHYSICAL_ADDR AFAICT and is the minimum kASLR considers
so right below it should work:
Does not look safe yet...
Let me think about it more.
Thanks
Yinghai
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, Feb 28, 2015 at 6:17 PM, Yinghai Lu wrote:
> We should access variable with referrence instead of using physical
> address as value.
>
> Cc: Matt Fleming
> Cc: Borislav Petkov
> Signed-off-by: Yinghai Lu
> ---
> arch/x86/kernel/setup.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 98dc931..05d444f 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -429,7 +429,13 @@ 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));
> + /* kaslr_setup_data is defined in aslr.c */
> + unsigned char *data;
> + unsigned long offset = sizeof(struct setup_data);
> +
> + data = early_memremap(pa_data, offset + 1);
> + kaslr_enabled = *(data + offset);
> + early_memunmap(data, offset + 1);
> }
>
> static void __init parse_setup_data(void)
> --
oh, no. the offending commit already got into linus tree.
commit f47233c2d34f243ecdaac179c3408a39ff9216a7
Author: Jiri Kosina
Date: Fri Feb 13 16:04:55 2015 +0100
x86/mm/ASLR: Propagate base load address calculation
--
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/
[PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly
We should access variable with referrence instead of using physical
address as value.
Cc: Matt Fleming
Cc: Borislav Petkov
Signed-off-by: Yinghai Lu
---
arch/x86/kernel/setup.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..05d444f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ 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));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
static void __init parse_setup_data(void)
--
1.8.4.5
--
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/
[PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly
We should access variable with referrence instead of using physical
address as value.
Cc: Matt Fleming matt.flem...@intel.com
Cc: Borislav Petkov b...@suse.de
Signed-off-by: Yinghai Lu ying...@kernel.org
---
arch/x86/kernel/setup.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..05d444f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ 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));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
static void __init parse_setup_data(void)
--
1.8.4.5
--
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 1/8] x86, kaslr: get kaslr_enabled back correctly
On Sat, Feb 28, 2015 at 6:17 PM, Yinghai Lu ying...@kernel.org wrote:
We should access variable with referrence instead of using physical
address as value.
Cc: Matt Fleming matt.flem...@intel.com
Cc: Borislav Petkov b...@suse.de
Signed-off-by: Yinghai Lu ying...@kernel.org
---
arch/x86/kernel/setup.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 98dc931..05d444f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ 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));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}
static void __init parse_setup_data(void)
--
oh, no. the offending commit already got into linus tree.
commit f47233c2d34f243ecdaac179c3408a39ff9216a7
Author: Jiri Kosina jkos...@suse.cz
Date: Fri Feb 13 16:04:55 2015 +0100
x86/mm/ASLR: Propagate base load address calculation
--
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/
