| 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: <CAKv+Gu-O+6TzfnWkNUaVU+w-e4TtCs08atMhcWCcYOQwyZ2B=A@mail.gmail.com> Date: Thu, 14 Jan 2016 10:05:42 +0100 From: Ard Biesheuvel <ard.biesheuvel@...aro.org> To: Mark Rutland <mark.rutland@....com> Cc: "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, kernel-hardening@...ts.openwall.com, Will Deacon <will.deacon@....com>, Catalin Marinas <catalin.marinas@....com>, Leif Lindholm <leif.lindholm@...aro.org>, Kees Cook <keescook@...omium.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Stuart Yoder <stuart.yoder@...escale.com>, Sharma Bhupesh <bhupesh.sharma@...escale.com>, Arnd Bergmann <arnd@...db.de>, Marc Zyngier <marc.zyngier@....com>, Christoffer Dall <christoffer.dall@...aro.org> Subject: Re: [PATCH v3 11/21] arm64: avoid R_AARCH64_ABS64 relocations for Image header fields On 14 January 2016 at 09:51, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote: > On 13 January 2016 at 19:48, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote: >> On 13 January 2016 at 19:12, Mark Rutland <mark.rutland@....com> wrote: >>> On Mon, Jan 11, 2016 at 02:19:04PM +0100, Ard Biesheuvel wrote: >>>> Unfortunately, the current way of using the linker to emit build time >>>> constants into the Image header will no longer work once we switch to >>>> the use of PIE executables. The reason is that such constants are emitted >>>> into the binary using R_AARCH64_ABS64 relocations, which we will resolve >>>> at runtime, not at build time, and the places targeted by those >>>> relocations will contain zeroes before that. >>>> >>>> So move back to assembly time constants or R_AARCH64_ABS32 relocations >>>> (which, interestingly enough, do get resolved at build time) >>> >>> To me it seems very odd that ABS64 and ABS32 are treated differently, >>> and it makes me somewhat uncomfortable becuase it feels like a bug. >>> >>> Do we know whether the inconsistency between ABS64 and ABS32 was >>> deliberate? >>> >>> I couldn't spot anything declaring a difference in the AArch64 ELF >>> spec, and I'm not sure where else to look. >>> >> >> My assumption is that PIE only defers resolving R_AARCH64_ABS64 >> relocations since those are the only ones that can be used to refer to >> memory addresses >> > > OK, digging into the binutils source code, it turns out that indeed, > ABSnn relocations where nn equals the ELFnn memory size are treated > differently, but only if they have default visibility. This is simply > a result of the fact the code path is shared between shared libraries > and PIE executables, since PIE executable are fully linked. It also > means that we can simply work around it by emitting the linker symbols > as hidden. > ... and the bad news is that, while emitting the symbols as hidden turns them from R_AARCH64_ABS64 into a R_AARCH64_RELATIVE relocations, it does not actually force the value to be emitted at build time. So I am going to stick with the patch, but elaborate in a comment about why R_AARCH64_ABSnn are treated differently if nn equals the pointer size. (look at elfNN_aarch64_final_link_relocate() in binutils if you are keen to look at the code yourself) -- Ard. > >>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org> >>>> --- >>>> arch/arm64/include/asm/assembler.h | 15 ++++++++ >>>> arch/arm64/kernel/head.S | 17 +++++++-- >>>> arch/arm64/kernel/image.h | 37 ++++++-------------- >>>> 3 files changed, 40 insertions(+), 29 deletions(-) >>>> >>>> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h >>>> index d8bfcc1ce923..e211af783a3d 100644 >>>> --- a/arch/arm64/include/asm/assembler.h >>>> +++ b/arch/arm64/include/asm/assembler.h >>>> @@ -222,4 +222,19 @@ lr .req x30 // link register >>>> .size __pi_##x, . - x; \ >>>> ENDPROC(x) >>>> >>>> + .macro le16, val >>>> + .byte \val & 0xff >>>> + .byte (\val >> 8) & 0xff >>>> + .endm >>>> + >>>> + .macro le32, val >>>> + le16 \val >>>> + le16 (\val >> 16) >>>> + .endm >>>> + >>>> + .macro le64, val >>>> + le32 \val >>>> + le32 (\val >> 32) >>>> + .endm >>>> + >>>> #endif /* __ASM_ASSEMBLER_H */ >>>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S >>>> index 350515276541..211f75e673f4 100644 >>>> --- a/arch/arm64/kernel/head.S >>>> +++ b/arch/arm64/kernel/head.S >>>> @@ -51,6 +51,17 @@ >>>> #define KERNEL_START _text >>>> #define KERNEL_END _end >>>> >>>> +#ifdef CONFIG_CPU_BIG_ENDIAN >>>> +#define __HEAD_FLAG_BE 1 >>>> +#else >>>> +#define __HEAD_FLAG_BE 0 >>>> +#endif >>>> + >>>> +#define __HEAD_FLAG_PAGE_SIZE ((PAGE_SHIFT - 10) / 2) >>>> + >>>> +#define __HEAD_FLAGS ((__HEAD_FLAG_BE << 0) | \ >>>> + (__HEAD_FLAG_PAGE_SIZE << 1)) >>>> + >>>> /* >>>> * Kernel startup entry point. >>>> * --------------------------- >>>> @@ -83,9 +94,9 @@ efi_head: >>>> b stext // branch to kernel start, magic >>>> .long 0 // reserved >>>> #endif >>>> - .quad _kernel_offset_le // Image load offset from start of RAM, little-endian >>>> - .quad _kernel_size_le // Effective size of kernel image, little-endian >>>> - .quad _kernel_flags_le // Informative flags, little-endian >>>> + le64 TEXT_OFFSET // Image load offset from start of RAM, little-endian >>>> + .long _kernel_size_le, 0 // Effective size of kernel image, little-endian >>>> + le64 __HEAD_FLAGS // Informative flags, little-endian >>>> .quad 0 // reserved >>>> .quad 0 // reserved >>>> .quad 0 // reserved >>>> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h >>>> index bc2abb8b1599..bb6b0e69d0a4 100644 >>>> --- a/arch/arm64/kernel/image.h >>>> +++ b/arch/arm64/kernel/image.h >>>> @@ -26,41 +26,26 @@ >>>> * There aren't any ELF relocations we can use to endian-swap values known only >>>> * at link time (e.g. the subtraction of two symbol addresses), so we must get >>>> * the linker to endian-swap certain values before emitting them. >>>> + * Note that this will not work for 64-bit values: these are resolved using >>>> + * R_AARCH64_ABS64 relocations, which are fixed up at runtime rather than at >>>> + * build time when building the PIE executable (for KASLR). >>>> */ >>>> #ifdef CONFIG_CPU_BIG_ENDIAN >>>> -#define DATA_LE64(data) \ >>>> - ((((data) & 0x00000000000000ff) << 56) | \ >>>> - (((data) & 0x000000000000ff00) << 40) | \ >>>> - (((data) & 0x0000000000ff0000) << 24) | \ >>>> - (((data) & 0x00000000ff000000) << 8) | \ >>>> - (((data) & 0x000000ff00000000) >> 8) | \ >>>> - (((data) & 0x0000ff0000000000) >> 24) | \ >>>> - (((data) & 0x00ff000000000000) >> 40) | \ >>>> - (((data) & 0xff00000000000000) >> 56)) >>>> +#define DATA_LE32(data) \ >>>> + ((((data) & 0x000000ff) << 24) | \ >>>> + (((data) & 0x0000ff00) << 8) | \ >>>> + (((data) & 0x00ff0000) >> 8) | \ >>>> + (((data) & 0xff000000) >> 24)) >>>> #else >>>> -#define DATA_LE64(data) ((data) & 0xffffffffffffffff) >>>> +#define DATA_LE32(data) ((data) & 0xffffffff) >>>> #endif >>>> >>>> -#ifdef CONFIG_CPU_BIG_ENDIAN >>>> -#define __HEAD_FLAG_BE 1 >>>> -#else >>>> -#define __HEAD_FLAG_BE 0 >>>> -#endif >>>> - >>>> -#define __HEAD_FLAG_PAGE_SIZE ((PAGE_SHIFT - 10) / 2) >>>> - >>>> -#define __HEAD_FLAGS ((__HEAD_FLAG_BE << 0) | \ >>>> - (__HEAD_FLAG_PAGE_SIZE << 1)) >>>> - >>>> /* >>>> * These will output as part of the Image header, which should be little-endian >>>> - * regardless of the endianness of the kernel. While constant values could be >>>> - * endian swapped in head.S, all are done here for consistency. >>>> + * regardless of the endianness of the kernel. >>>> */ >>>> #define HEAD_SYMBOLS \ >>>> - _kernel_size_le = DATA_LE64(_end - _text); \ >>>> - _kernel_offset_le = DATA_LE64(TEXT_OFFSET); \ >>>> - _kernel_flags_le = DATA_LE64(__HEAD_FLAGS); >>>> + _kernel_size_le = DATA_LE32(_end - _text); >>>> >>>> #ifdef CONFIG_EFI >>>> >>>> -- >>>> 2.5.0 >>>>
Powered by blists - more mailing lists