Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752557Ab3GHP0Q (ORCPT ); Mon, 8 Jul 2013 11:26:16 -0400 Received: from mail-pb0-f54.google.com ([209.85.160.54]:54337 "EHLO mail-pb0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751743Ab3GHP0O (ORCPT ); Mon, 8 Jul 2013 11:26:14 -0400 Message-ID: <51DADA09.4070202@gmail.com> Date: Mon, 08 Jul 2013 23:26:01 +0800 From: Zhang Yanfei User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.5) Gecko/20120607 Thunderbird/10.0.5 MIME-Version: 1.0 To: Kees Cook , "H. Peter Anvin" CC: Zhang Yanfei , LKML , Thomas Gleixner , Ingo Molnar , "x86@kernel.org" , Yinghai Lu , Matt Fleming , Borislav Petkov , Alexander Duyck , Jacob Shin , Pekka Enberg Subject: Re: [RESEND][PATCH] x86, relocs: move ELF relocation handling to C References: <20130702182206.GA4055@www.outflux.net> <51D398FE.7060700@cn.fujitsu.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 13564 Lines: 372 On 07/08/2013 09:25 PM, Kees Cook wrote: > On Tue, Jul 2, 2013 at 8:22 PM, Zhang Yanfei wrote: >> Hello Kees, >> >> On 07/03/2013 02:22 AM, Kees Cook wrote: >>> Moves the relocation handling into C, after decompression. This requires >>> that the decompressed size is passed to the decompression routine as >>> well so that relocations can be found. Only kernels that need relocation >>> support will use the code (currently just x86_32), but this is laying >>> the ground work for 64-bit support in support of KASLR. >>> >>> Based on work by Neill Clift and Michael Davidson. >>> >>> Signed-off-by: Kees Cook >>> --- >>> arch/x86/Kconfig | 4 +- >>> arch/x86/Makefile | 8 ++-- >>> arch/x86/boot/compressed/head_32.S | 31 ++------------ >>> arch/x86/boot/compressed/head_64.S | 1 + >>> arch/x86/boot/compressed/misc.c | 77 +++++++++++++++++++++++++++++++++- >>> arch/x86/include/asm/page_32_types.h | 2 + >>> arch/x86/include/asm/page_64_types.h | 5 --- >>> arch/x86/include/asm/page_types.h | 6 +++ >>> 8 files changed, 94 insertions(+), 40 deletions(-) >>> >>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>> index fe120da..f11ad6f 100644 >>> --- a/arch/x86/Kconfig >>> +++ b/arch/x86/Kconfig >>> @@ -1695,13 +1695,13 @@ config RELOCATABLE >>> it has been loaded at and the compile time physical address >>> (CONFIG_PHYSICAL_START) is ignored. >>> >>> -# Relocation on x86-32 needs some additional build support >>> +# Relocation on x86 needs some additional build support >>> config X86_NEED_RELOCS >>> def_bool y >>> depends on X86_32 && RELOCATABLE >> >> Here it still actually depends on x86_32, so why change the comment? > > Yeah, I can leave this as-is. > >>> config PHYSICAL_ALIGN >>> - hex "Alignment value to which kernel should be aligned" if X86_32 >>> + hex "Alignment value to which kernel should be aligned" >>> default "0x1000000" >>> range 0x2000 0x1000000 >> >> If you open this option for x86_64, the range should be modified a bit >> to indicate the fact that on x86_64, the alignment of the kernel should >> be at least 2M and the alignment itself should be 2M aligned. >> >> otherwise, someone specifies an incorrect value here, say 0x300000, >> which will cause the error: >> >> #error "Invalid value for CONFIG_PHYSICAL_ALIGN" >> >> when compiling the kernel. > > Are you looking for this to be added to the Kconfig help text, or do > you mean there should be build-time sanity checks for the value? I don't know. The sanity check is already in arch/x86/include/asm/boot.h: /* Minimum kernel alignment, as a power of two */ #ifdef CONFIG_X86_64 #define MIN_KERNEL_ALIGN_LG2 PMD_SHIFT #else #define MIN_KERNEL_ALIGN_LG2 (PAGE_SHIFT + THREAD_SIZE_ORDER) #endif #define MIN_KERNEL_ALIGN (_AC(1, UL) << MIN_KERNEL_ALIGN_LG2) #if (CONFIG_PHYSICAL_ALIGN & (CONFIG_PHYSICAL_ALIGN-1)) || \ (CONFIG_PHYSICAL_ALIGN < MIN_KERNEL_ALIGN) #error "Invalid value for CONFIG_PHYSICAL_ALIGN" #endif But just from the config: range 0x2000 0x1000000 is just a align restriction for x86_32, not for x86_64. So we should not emit this hint for x86_64, or change this hint to: config PHYSICAL_ALIGN hex "Alignment value to which kernel should be aligned" default "0x1000000" range 0x2000 0x1000000 if X86_32 range 0x200000 0x1000000 if X86_64 And add the description in the Kconfig help text? Peter? > >>> ---help--- >>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile >>> index 5c47726..43f8cef 100644 >>> --- a/arch/x86/Makefile >>> +++ b/arch/x86/Makefile >>> @@ -16,6 +16,10 @@ endif >>> # e.g.: obj-y += foo_$(BITS).o >>> export BITS >>> >>> +ifdef CONFIG_X86_NEED_RELOCS >>> + LDFLAGS_vmlinux := --emit-relocs >>> +endif >>> + >>> ifeq ($(CONFIG_X86_32),y) >>> BITS := 32 >>> UTS_MACHINE := i386 >>> @@ -25,10 +29,6 @@ ifeq ($(CONFIG_X86_32),y) >>> KBUILD_AFLAGS += $(biarch) >>> KBUILD_CFLAGS += $(biarch) >>> >>> - ifdef CONFIG_RELOCATABLE >>> - LDFLAGS_vmlinux := --emit-relocs >>> - endif >>> - >>> KBUILD_CFLAGS += -msoft-float -mregparm=3 -freg-struct-return >>> >>> # Never want PIC in a 32-bit kernel, prevent breakage with GCC built >>> diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S >>> index 1e3184f..5d6f689 100644 >>> --- a/arch/x86/boot/compressed/head_32.S >>> +++ b/arch/x86/boot/compressed/head_32.S >>> @@ -181,8 +181,9 @@ relocated: >>> /* >>> * Do the decompression, and jump to the new kernel.. >>> */ >>> - leal z_extract_offset_negative(%ebx), %ebp >>> /* push arguments for decompress_kernel: */ >>> + pushl $z_output_len /* decompressed length */ >>> + leal z_extract_offset_negative(%ebx), %ebp >>> pushl %ebp /* output address */ >>> pushl $z_input_len /* input_len */ >>> leal input_data(%ebx), %eax >>> @@ -191,33 +192,7 @@ relocated: >>> pushl %eax /* heap area */ >>> pushl %esi /* real mode pointer */ >>> call decompress_kernel >>> - addl $20, %esp >>> - >>> -#if CONFIG_RELOCATABLE >>> -/* >>> - * Find the address of the relocations. >>> - */ >>> - leal z_output_len(%ebp), %edi >>> - >>> -/* >>> - * Calculate the delta between where vmlinux was compiled to run >>> - * and where it was actually loaded. >>> - */ >>> - movl %ebp, %ebx >>> - subl $LOAD_PHYSICAL_ADDR, %ebx >>> - jz 2f /* Nothing to be done if loaded at compiled addr. */ >>> -/* >>> - * Process relocations. >>> - */ >>> - >>> -1: subl $4, %edi >>> - movl (%edi), %ecx >>> - testl %ecx, %ecx >>> - jz 2f >>> - addl %ebx, -__PAGE_OFFSET(%ebx, %ecx) >>> - jmp 1b >>> -2: >>> -#endif >>> + addl $24, %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 16f24e6..6632693 100644 >>> --- a/arch/x86/boot/compressed/head_64.S >>> +++ b/arch/x86/boot/compressed/head_64.S >>> @@ -340,6 +340,7 @@ relocated: >>> 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 */ >>> call decompress_kernel >>> popq %rsi >>> >>> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c >>> index 7cb56c6..b756a04 100644 >>> --- a/arch/x86/boot/compressed/misc.c >>> +++ b/arch/x86/boot/compressed/misc.c >>> @@ -267,6 +267,79 @@ static void error(char *x) >>> asm("hlt"); >>> } >>> >>> +#if CONFIG_X86_NEED_RELOCS >>> +static void handle_relocations(void *output, unsigned long output_len) >>> +{ >>> + int *reloc; >>> + unsigned long delta, map, ptr; >>> + unsigned long min_addr = (unsigned long)output; >>> + unsigned long max_addr = min_addr + output_len; >>> + >>> + /* >>> + * Calculate the delta between where vmlinux was linked to load >>> + * and where it was actually loaded. >>> + */ >>> + delta = min_addr - LOAD_PHYSICAL_ADDR; >>> + if (!delta) { >>> + debug_putstr("No relocation needed... "); >>> + return; >>> + } >>> + debug_putstr("Performing relocations... "); >>> + >>> + /* >>> + * The kernel contains a table of relocation addresses. Those >>> + * addresses have the final load address of the kernel in virtual >>> + * memory. We are currently working in the self map. So we need to >>> + * create an adjustment for kernel memory addresses to the self map. >>> + * This will involve subtracting out the base address of the kernel. >>> + */ >>> + map = delta - __START_KERNEL_map; >>> + >>> + /* >>> + * Process relocations: 32 bit relocations first then 64 bit after. >>> + * Two sets of binary relocations are added to the end of the kernel >>> + * before compression. Each relocation table entry is the kernel >>> + * address of the location which needs to be updated stored as a >>> + * 32-bit value which is sign extended to 64 bits. >>> + * >>> + * Format is: >>> + * >>> + * kernel bits... >>> + * 0 - zero terminator for 64 bit relocations >>> + * 64 bit relocation repeated >>> + * 0 - zero terminator for 32 bit relocations >>> + * 32 bit relocation repeated >>> + * >>> + * So we work backwards from the end of the decompressed image. >>> + */ >>> + for (reloc = output + output_len - sizeof(*reloc); *reloc; reloc--) { >>> + int extended = *reloc; >>> + extended += map; >>> + >>> + ptr = (unsigned long)extended; >>> + if (ptr < min_addr || ptr > max_addr) >>> + error("32-bit relocation outside of kernel!\n"); >>> + >>> + *(uint32_t *)ptr += delta; >>> + } >>> +#ifdef CONFIG_X86_64 >>> + for (reloc--; *reloc; reloc--) { >>> + long extended = *reloc; >>> + extended += map; >>> + >>> + ptr = (unsigned long)extended; >>> + if (ptr < min_addr || ptr > max_addr) >>> + error("64-bit relocation outside of kernel!\n"); >>> + >>> + *(uint64_t *)ptr += delta; >>> + } >>> +#endif >>> +} >>> +#else >>> +static inline void handle_relocations(void *output, unsigned long output_len) >>> +{ } >>> +#endif >>> + >>> static void parse_elf(void *output) >>> { >>> #ifdef CONFIG_X86_64 >>> @@ -321,7 +394,8 @@ static void parse_elf(void *output) >>> asmlinkage void decompress_kernel(void *rmode, memptr heap, >>> unsigned char *input_data, >>> unsigned long input_len, >>> - unsigned char *output) >>> + unsigned char *output, >>> + unsigned long output_len) >>> { >>> real_mode = rmode; >>> >>> @@ -361,6 +435,7 @@ asmlinkage void decompress_kernel(void *rmode, memptr heap, >>> debug_putstr("\nDecompressing Linux... "); >>> decompress(input_data, input_len, NULL, NULL, output, NULL, error); >>> parse_elf(output); >>> + handle_relocations(output, output_len); >>> debug_putstr("done.\nBooting the kernel.\n"); >>> return; >>> } >>> diff --git a/arch/x86/include/asm/page_32_types.h b/arch/x86/include/asm/page_32_types.h >>> index ef17af0..f48b17d 100644 >>> --- a/arch/x86/include/asm/page_32_types.h >>> +++ b/arch/x86/include/asm/page_32_types.h >>> @@ -15,6 +15,8 @@ >>> */ >>> #define __PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL) >>> >>> +#define __START_KERNEL_map __PAGE_OFFSET >>> + >>> #define THREAD_SIZE_ORDER 1 >>> #define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER) >>> >>> diff --git a/arch/x86/include/asm/page_64_types.h b/arch/x86/include/asm/page_64_types.h >>> index 6c896fb..43dcd80 100644 >>> --- a/arch/x86/include/asm/page_64_types.h >>> +++ b/arch/x86/include/asm/page_64_types.h >>> @@ -32,11 +32,6 @@ >>> */ >>> #define __PAGE_OFFSET _AC(0xffff880000000000, UL) >>> >>> -#define __PHYSICAL_START ((CONFIG_PHYSICAL_START + \ >>> - (CONFIG_PHYSICAL_ALIGN - 1)) & \ >>> - ~(CONFIG_PHYSICAL_ALIGN - 1)) >>> - >>> -#define __START_KERNEL (__START_KERNEL_map + __PHYSICAL_START) >>> #define __START_KERNEL_map _AC(0xffffffff80000000, UL) >>> >>> /* See Documentation/x86/x86_64/mm.txt for a description of the memory map. */ >>> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h >>> index 54c9787..086c2fa 100644 >>> --- a/arch/x86/include/asm/page_types.h >>> +++ b/arch/x86/include/asm/page_types.h >>> @@ -33,6 +33,12 @@ >>> (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \ >>> VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC) >>> >>> +#define __PHYSICAL_START ((CONFIG_PHYSICAL_START + \ >>> + (CONFIG_PHYSICAL_ALIGN - 1)) & \ >>> + ~(CONFIG_PHYSICAL_ALIGN - 1)) >> >> I know you just moved the code. Could this macro be like this: >> >> #define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, CONFIG_PHYSICAL_ALIGN) > > Yeah, I'll fix that too. > > Thanks! > > -Kees > >> >>> + >>> +#define __START_KERNEL (__START_KERNEL_map + __PHYSICAL_START) >>> + >>> #ifdef CONFIG_X86_64 >>> #include >>> #else >>> >> >> >> -- >> Thanks. >> Zhang Yanfei > > > > -- > Kees Cook > Chrome OS Security > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Thanks. Zhang Yanfei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/