Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd
On 11/17/2014 06:23 PM, Greg Thelen wrote: > > On Mon, Nov 17 2014, Greg Thelen wrote: > [...] >> Given that bss and brk are nobits (i.e. only ALLOC) sections, does >> file_offset make sense as a load address. This fails with gold: >> It really doesn't. We have that information elsewhere. -hpa -- 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 v3] x86, kaslr: Prevent .bss from overlaping initrd
On Mon, Nov 17 2014, Greg Thelen wrote: [...] > Given that bss and brk are nobits (i.e. only ALLOC) sections, does > file_offset make sense as a load address. This fails with gold: > > $ git checkout v3.18-rc5 > $ make # with gold > [...] > ..bss and .brk lack common file offset > ..bss and .brk lack common file offset > ..bss and .brk lack common file offset > ..bss and .brk lack common file offset > MKPIGGY arch/x86/boot/compressed/piggy.S > Usage: arch/x86/boot/compressed/mkpiggy compressed_file run_size > make[2]: *** [arch/x86/boot/compressed/piggy.S] Error 1 > make[1]: *** [arch/x86/boot/compressed/vmlinux] Error 2 > make: *** [bzImage] Error 2 [...] I just saw http://www.spinics.net/lists/kernel/msg1869328.html which fixes things for me. Sorry for the noise. -- 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 v3] x86, kaslr: Prevent .bss from overlaping initrd
On Fri, Oct 31 2014, Junjie Mao wrote: > When choosing a random address, the current implementation does not take into > account the reversed space for .bss and .brk sections. Thus the relocated > kernel > may overlap other components in memory. Here is an example of the overlap > from a > x86_64 kernel in qemu (the ranges of physical addresses are presented): > > Physical Address > > 0x0fe0 --++ <-- randomized base >/ | relocated kernel | >vmlinux.bin| (from vmlinux.bin) | > 0x1336d000(an ELF file) ++-- >\ || \ > 0x1376d870 --++ | > |relocs table| | > 0x13c1c2a8++ .bss and .brk > || | > 0x13ce6000++ | > || / > 0x13f77000| initrd |-- > || > 0x13fef374++ > > The initrd image will then be overwritten by the memset during early > initialization: > > [1.655204] Unpacking initramfs... > [1.662831] Initramfs unpacking failed: junk in compressed archive > > This patch prevents the above situation by requiring a larger space when > looking > for a random kernel base, so that existing logic can effectively avoids the > overlap. > > Fixes: 82fa9637a2 ("x86, kaslr: Select random position from e820 maps") > Reported-by: Fengguang Wu > Signed-off-by: Junjie Mao > [kees: switched to perl to avoid hex translation pain in mawk vs gawk] > [kees: calculated overlap without relocs table] > Signed-off-by: Kees Cook > Cc: sta...@vger.kernel.org > --- > This version updates the commit log only. > > Kees, please help review the documentation. Thanks! > > Best Regards > Junjie Mao [...] > diff --git a/arch/x86/tools/calc_run_size.pl b/arch/x86/tools/calc_run_size.pl > new file mode 100644 > index ..0b0b124d3ece > --- /dev/null > +++ b/arch/x86/tools/calc_run_size.pl > @@ -0,0 +1,30 @@ > +#!/usr/bin/perl > +# > +# Calculate the amount of space needed to run the kernel, including room for > +# the .bss and .brk sections. > +# > +# Usage: > +# objdump -h a.out | perl calc_run_size.pl > +use strict; > + > +my $mem_size = 0; > +my $file_offset = 0; > + > +my $sections=" *[0-9]+ \.(?:bss|brk) +"; > +while (<>) { > + if (/^$sections([0-9a-f]+) +(?:[0-9a-f]+ +){2}([0-9a-f]+)/) { > + my $size = hex($1); > + my $offset = hex($2); > + $mem_size += $size; > + if ($file_offset == 0) { > + $file_offset = $offset; > + } elsif ($file_offset != $offset) { > + die ".bss and .brk lack common file offset\n"; > + } > + } > +} > + > +if ($file_offset == 0) { > + die "Never found .bss or .brk file offset\n"; > +} > +printf("%d\n", $mem_size + $file_offset); Given that bss and brk are nobits (i.e. only ALLOC) sections, does file_offset make sense as a load address. This fails with gold: $ git checkout v3.18-rc5 $ make # with gold [...] ..bss and .brk lack common file offset ..bss and .brk lack common file offset ..bss and .brk lack common file offset ..bss and .brk lack common file offset MKPIGGY arch/x86/boot/compressed/piggy.S Usage: arch/x86/boot/compressed/mkpiggy compressed_file run_size make[2]: *** [arch/x86/boot/compressed/piggy.S] Error 1 make[1]: *** [arch/x86/boot/compressed/vmlinux] Error 2 make: *** [bzImage] Error 2 In ld.bfd brk/bss file_offsets match, but they differ with ld.gold: $ objdump -h vmlinux.ld [...] 0 .text 00818bb3 8100 0100 0020 2**12 CONTENTS, ALLOC, LOAD, READONLY, CODE [...] 26 .bss 000e 81fe8000 01fe8000 013e8000 2**12 ALLOC 27 .brk 00026000 820c8000 020c8000 013e8000 2**0 ALLOC $ objdump -h vmlinux.ld | perl arch/x86/tools/calc_run_size.pl 21946368 # aka 0x14ee000 $ objdump -h vmlinux.gold [...] 0 .text 00818bb3 8100 0100 1000 2**12 CONTENTS, ALLOC, LOAD, READONLY, CODE [...] 26 .bss 000e 81feb000 01feb000 00e9 2**12 ALLOC 27 .brk 00026000 820cb000 020cb000 00f7 2**0 ALLOC -- 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 v3] x86, kaslr: Prevent .bss from overlaping initrd
On Fri, Oct 31, 2014 at 6:40 AM, Junjie Mao wrote: > When choosing a random address, the current implementation does not take into > account the reversed space for .bss and .brk sections. Thus the relocated > kernel > may overlap other components in memory. Here is an example of the overlap > from a > x86_64 kernel in qemu (the ranges of physical addresses are presented): > > Physical Address > > 0x0fe0 --++ <-- randomized base >/ | relocated kernel | >vmlinux.bin| (from vmlinux.bin) | > 0x1336d000(an ELF file) ++-- >\ || \ > 0x1376d870 --++ | > |relocs table| | > 0x13c1c2a8++ .bss and .brk > || | > 0x13ce6000++ | > || / > 0x13f77000| initrd |-- > || > 0x13fef374++ > > The initrd image will then be overwritten by the memset during early > initialization: > > [1.655204] Unpacking initramfs... > [1.662831] Initramfs unpacking failed: junk in compressed archive > > This patch prevents the above situation by requiring a larger space when > looking > for a random kernel base, so that existing logic can effectively avoids the > overlap. > > Fixes: 82fa9637a2 ("x86, kaslr: Select random position from e820 maps") > Reported-by: Fengguang Wu > Signed-off-by: Junjie Mao > [kees: switched to perl to avoid hex translation pain in mawk vs gawk] > [kees: calculated overlap without relocs table] > Signed-off-by: Kees Cook > Cc: sta...@vger.kernel.org > --- > This version updates the commit log only. > > Kees, please help review the documentation. Thanks! Looks great, thanks! Can someone from x86 pull this please? -Kees > > Best Regards > Junjie Mao > --- > 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| 13 ++--- > arch/x86/boot/compressed/mkpiggy.c | 9 +++-- > arch/x86/tools/calc_run_size.pl| 30 ++ > 6 files changed, 57 insertions(+), 9 deletions(-) > create mode 100644 arch/x86/tools/calc_run_size.pl > > diff --git a/arch/x86/boot/compressed/Makefile > b/arch/x86/boot/compressed/Makefile > index 0fcd9133790c..14fe7cba21d1 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -75,8 +75,10 @@ suffix-$(CONFIG_KERNEL_XZ) := xz > suffix-$(CONFIG_KERNEL_LZO):= lzo > suffix-$(CONFIG_KERNEL_LZ4):= lz4 > > +RUN_SIZE = $(shell objdump -h vmlinux | \ > +perl $(srctree)/arch/x86/tools/calc_run_size.pl) > quiet_cmd_mkpiggy = MKPIGGY $@ > - cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false ) > + cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false > ) > > targets += piggy.S > $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE > diff --git a/arch/x86/boot/compressed/head_32.S > b/arch/x86/boot/compressed/head_32.S > index cbed1407a5cd..1d7fbbcc196d 100644 > --- a/arch/x86/boot/compressed/head_32.S > +++ b/arch/x86/boot/compressed/head_32.S > @@ -207,7 +207,8 @@ relocated: > * Do the decompression, and jump to the new kernel.. > */ > /* push arguments for decompress_kernel: */ > - pushl $z_output_len /* decompressed length */ > + pushl $z_run_size /* size of kernel with .bss and .brk */ > + pushl $z_output_len /* decompressed length, end of relocs */ > lealz_extract_offset_negative(%ebx), %ebp > pushl %ebp/* output address */ > pushl $z_input_len/* input_len */ > @@ -217,7 +218,7 @@ relocated: > pushl %eax/* heap area */ > pushl %esi/* real mode pointer */ > calldecompress_kernel /* returns kernel location in %eax */ > - addl$24, %esp > + addl$28, %esp > > /* > * Jump to the decompressed kernel. > diff --git a/arch/x86/boot/compressed/head_64.S > b/arch/x86/boot/compressed/head_64.S > index 2884e0c3e8a5..6b1766c6c082 100644 > --- a/arch/x86/boot/compressed/head_64.S > +++ b/arch/x86/boot/compressed/head_64.S > @@ -402,13 +402,16 @@ 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
[PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd
When choosing a random address, the current implementation does not take into account the reversed space for .bss and .brk sections. Thus the relocated kernel may overlap other components in memory. Here is an example of the overlap from a x86_64 kernel in qemu (the ranges of physical addresses are presented): Physical Address 0x0fe0 --++ <-- randomized base / | relocated kernel | vmlinux.bin| (from vmlinux.bin) | 0x1336d000(an ELF file) ++-- \ || \ 0x1376d870 --++ | |relocs table| | 0x13c1c2a8++ .bss and .brk || | 0x13ce6000++ | || / 0x13f77000| initrd |-- || 0x13fef374++ The initrd image will then be overwritten by the memset during early initialization: [1.655204] Unpacking initramfs... [1.662831] Initramfs unpacking failed: junk in compressed archive This patch prevents the above situation by requiring a larger space when looking for a random kernel base, so that existing logic can effectively avoids the overlap. Fixes: 82fa9637a2 ("x86, kaslr: Select random position from e820 maps") Reported-by: Fengguang Wu Signed-off-by: Junjie Mao [kees: switched to perl to avoid hex translation pain in mawk vs gawk] [kees: calculated overlap without relocs table] Signed-off-by: Kees Cook Cc: sta...@vger.kernel.org --- This version updates the commit log only. Kees, please help review the documentation. Thanks! Best Regards Junjie Mao --- 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| 13 ++--- arch/x86/boot/compressed/mkpiggy.c | 9 +++-- arch/x86/tools/calc_run_size.pl| 30 ++ 6 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 arch/x86/tools/calc_run_size.pl diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 0fcd9133790c..14fe7cba21d1 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -75,8 +75,10 @@ suffix-$(CONFIG_KERNEL_XZ) := xz suffix-$(CONFIG_KERNEL_LZO):= lzo suffix-$(CONFIG_KERNEL_LZ4):= lz4 +RUN_SIZE = $(shell objdump -h vmlinux | \ +perl $(srctree)/arch/x86/tools/calc_run_size.pl) quiet_cmd_mkpiggy = MKPIGGY $@ - cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false ) + cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false ) targets += piggy.S $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S index cbed1407a5cd..1d7fbbcc196d 100644 --- a/arch/x86/boot/compressed/head_32.S +++ b/arch/x86/boot/compressed/head_32.S @@ -207,7 +207,8 @@ relocated: * Do the decompression, and jump to the new kernel.. */ /* push arguments for decompress_kernel: */ - pushl $z_output_len /* decompressed length */ + pushl $z_run_size /* size of kernel with .bss and .brk */ + pushl $z_output_len /* decompressed length, end of relocs */ lealz_extract_offset_negative(%ebx), %ebp pushl %ebp/* output address */ pushl $z_input_len/* input_len */ @@ -217,7 +218,7 @@ relocated: pushl %eax/* heap area */ pushl %esi/* real mode pointer */ calldecompress_kernel /* returns kernel location in %eax */ - addl$24, %esp + addl$28, %esp /* * Jump to the decompressed kernel. diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index 2884e0c3e8a5..6b1766c6c082 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -402,13 +402,16 @@ 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 */ leaqboot_heap(%rip), %rsi /* malloc area for uncompression */ leaqinput_data(%rip), %rdx /* input_data */ movl$z_input_len, %ecx /* input_len */ movq%rbp, %r8 /* output target address */ - movq$z_output_len, %r9 /* decompress