Messages in this thread Patch in this message | | | Date | Mon, 22 Jun 2020 15:04:40 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v4 3/5] stack: Optionally randomize kernel stack offset each syscall |
| |
On Mon, Jun 22, 2020 at 11:42:29PM +0200, Jann Horn wrote: > No, at least on x86-64 and x86 Linux overrides the normal ABI. From > arch/x86/Makefile:
Ah! Thanks for the pointer.
> > # For gcc stack alignment is specified with -mpreferred-stack-boundary, > # clang has the option -mstack-alignment for that purpose. > ifneq ($(call cc-option, -mpreferred-stack-boundary=4),) > cc_stack_align4 := -mpreferred-stack-boundary=2 > cc_stack_align8 := -mpreferred-stack-boundary=3 > else ifneq ($(call cc-option, -mstack-alignment=16),) > cc_stack_align4 := -mstack-alignment=4 > cc_stack_align8 := -mstack-alignment=8 > endif > [...] > ifeq ($(CONFIG_X86_32),y) > [...] > # Align the stack to the register width instead of using the default > # alignment of 16 bytes. This reduces stack usage and the number of > # alignment instructions. > KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align4)) > [...] > else > [...] > # By default gcc and clang use a stack alignment of 16 bytes for x86. > # However the standard kernel entry on x86-64 leaves the stack on an > # 8-byte boundary. If the compiler isn't informed about the actual > # alignment it will generate extra alignment instructions for the > # default alignment which keep the stack *mis*aligned. > # Furthermore an alignment to the register width reduces stack usage > # and the number of alignment instructions. > KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align8)) > [...] > endif
And it seems that only x86 does this. No other architecture specifies -mpreferred-stack-boundary...
> Normal x86-64 ABI has 16-byte stack alignment; Linux kernel x86-64 ABI > has 8-byte stack alignment. > Similarly, the normal Linux 32-bit x86 ABI is 16-byte aligned; > meanwhile Linux kernel x86 ABI has 4-byte stack alignment. > > This is because userspace code wants the stack to be sufficiently > aligned for fancy SSE instructions and such; the kernel, on the other > hand, never uses those in normal code, and cares about stack usage and > such very much.
This makes it nicer for Clang:
diff --git a/include/linux/randomize_kstack.h b/include/linux/randomize_kstack.h index 1df0dc52cadc..f7e1f68fb50c 100644 --- a/include/linux/randomize_kstack.h +++ b/include/linux/randomize_kstack.h @@ -10,6 +10,14 @@ DECLARE_STATIC_KEY_MAYBE(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, randomize_kstack_offset); DECLARE_PER_CPU(u32, kstack_offset); +#ifdef CONFIG_X86_64 +#define ARCH_STACK_ALIGN_MASK ~((1 << 8) - 1) +#elif defined(CONFIG_X86_32) +#define ARCH_STACK_ALIGN_MASK ~((1 << 4) - 1) +#else +#define ARCH_STACK_ALIGN_MASK ~(0) +#endif + /* * Do not use this anywhere else in the kernel. This is used here because * it provides an arch-agnostic way to grow the stack with correct @@ -23,7 +31,8 @@ void *__builtin_alloca(size_t size); if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, \ &randomize_kstack_offset)) { \ u32 offset = this_cpu_read(kstack_offset); \ - u8 *ptr = __builtin_alloca(offset & 0x3FF); \ + u8 *ptr = __builtin_alloca(offset & 0x3FF & \ + ARCH_STACK_ALIGN_MASK); \ asm volatile("" : "=m"(*ptr)); \ } \ } while (0)
But I don't like open-coding the x86-ony stack alignment... it should be in Kconfig or something, I think?
-- Kees Cook
| |