[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 14 Jul 2017 14:44:46 -0700
From: Laura Abbott <labbott@...hat.com>
To: Alexander Popov <alex.popov@...ux.com>,
kernel-hardening@...ts.openwall.com, keescook@...omium.org,
pageexec@...email.hu, spender@...ecurity.net, tycho@...ker.com,
Mark Rutland <mark.rutland@....com>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>, x86@...nel.org,
Andy Lutomirski <luto@...capital.net>
Subject: Re: [PATCH RFC v3 1/1] gcc-plugins: Add stackleak feature erasing the
kernel stack at the end of syscalls
On 07/12/2017 11:17 AM, Alexander Popov wrote:
> This is the third version of the patch introducing STACKLEAK to the
> mainline kernel. STACKLEAK is a security feature developed by Grsecurity/PaX
> (kudos to them again), which:
> - reduces the information that can be revealed by some kernel stack leak bugs
> (e.g. CVE-2016-4569 and CVE-2016-4578);
> - blocks some uninitialized stack variable attacks (e.g. CVE-2010-2963);
> - introduces some runtime checks for kernel stack overflow detection.
>
> Changes in v3 (rebased on the recent rc)
>
> 1. Added the detailed comments describing the STACKLEAK gcc plugin.
> Read the plugin from bottom up, like you do for Linux kernel drivers.
> Additional information:
> - gcc internals documentation, which is available from gcc sources;
> - wonderful slides by Diego Novillo
> https://www.airs.com/dnovillo/200711-GCC-Internals/
> - nice introduction to gcc plugins at LWN
> https://lwn.net/Articles/457543/
>
> 2. Improved the commit message and Kconfig description according the
> feedback from Kees Cook. Also added the original notice describing
> the performance impact of the STACKLEAK feature.
>
> 3. Removed arch-specific ix86_cmodel check from stackleak_track_stack_gate().
> It caused skipping the kernel code instrumentation for i386.
>
> 4. Fixed a minor mistake in stackleak_tree_instrument_execute().
> First versions of the plugin used ENTRY_BLOCK_PTR_FOR_FN(cfun)->next_bb
> to get the basic block with the function prologue. That was not correct
> since the control flow graph edge from the ENTRY_BLOCK_PTR doesn't always
> go to that basic block.
>
> So later it was changed it to single_succ(ENTRY_BLOCK_PTR_FOR_FN(cfun)),
> but not completely. next_bb was still used for entry_bb assignment,
> which could cause the wrong value of the prologue_instrumented variable.
>
> I've reported this issue to Grsecurity and proposed the fix for it, but
> unfortunately didn't get any reply.
>
> 5. Introduced the STACKLEAK_POISON macro and renamed the config option
> according the feedback from Kees Cook.
>
> More things to be done:
> - carefully look into the assertions in track_stack() and check_alloca();
> - find answers to the questions marked with TODO in the comments;
> - think of erasing the kernel stack after invoking EFI runtime services;
> - update self-protection.rst.
>
> Parallel work (thanks for that!):
> - Tycho Andersen is developing tests for STACKLEAK;
> - Laura Abbott is working on porting STACKLEAK to arm64.
>
> -- >8 --
>
> The stackleak feature erases the kernel stack before returning from syscalls.
> That reduces the information which a kernel stack leak bug can reveal and
> blocks some uninitialized stack variable attacks. Moreover, stackleak
> provides runtime checks for kernel stack overflow detection.
>
> This feature consists of:
> - the architecture-specific code filling the used part of the kernel stack
> with a poison value before returning to the userspace;
> - the stackleak gcc plugin. It instruments the kernel code inserting the
> track_stack() call for tracking the lowest border of the kernel stack
> and check_alloca() call for checking alloca size.
>
> The stackleak feature is ported from grsecurity/PaX. For more information see:
> https://grsecurity.net/
> https://pax.grsecurity.net/
>
> This code is modified from Brad Spengler/PaX Team's code in the last public
> patch of grsecurity/PaX based on our understanding of the code. Changes or
> omissions from the original code are ours and don't reflect the original
> grsecurity/PaX code.
>
> Signed-off-by: Alexander Popov <alex.popov@...ux.com>
> ---
> arch/Kconfig | 27 +++
> arch/x86/Kconfig | 1 +
> arch/x86/entry/common.c | 17 +-
> arch/x86/entry/entry_32.S | 71 ++++++
> arch/x86/entry/entry_64.S | 99 ++++++++
> arch/x86/entry/entry_64_compat.S | 8 +
> arch/x86/include/asm/processor.h | 4 +
> arch/x86/kernel/asm-offsets.c | 9 +
> arch/x86/kernel/dumpstack_32.c | 12 +
> arch/x86/kernel/dumpstack_64.c | 33 +++
> arch/x86/kernel/process_32.c | 5 +
> arch/x86/kernel/process_64.c | 5 +
> fs/exec.c | 17 ++
> include/linux/compiler.h | 4 +
> scripts/Makefile.gcc-plugins | 3 +
> scripts/gcc-plugins/stackleak_plugin.c | 397 +++++++++++++++++++++++++++++++++
> 16 files changed, 709 insertions(+), 3 deletions(-)
> create mode 100644 scripts/gcc-plugins/stackleak_plugin.c
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index cae0958..0c9a0f5 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -361,6 +361,13 @@ config SECCOMP_FILTER
>
> See Documentation/prctl/seccomp_filter.txt for details.
>
> +config HAVE_ARCH_STACKLEAK
> + bool
> + help
> + An architecture should select this if it has the code which
> + fills the used part of the kernel stack with the STACKLEAK_POISON
> + value before returning from system calls.
> +
> config HAVE_GCC_PLUGINS
> bool
> help
> @@ -482,6 +489,26 @@ config GCC_PLUGIN_RANDSTRUCT_PERFORMANCE
> in structures. This reduces the performance hit of RANDSTRUCT
> at the cost of weakened randomization.
>
> +config GCC_PLUGIN_STACKLEAK
> + bool "Erase the kernel stack before returning from syscalls"
> + depends on GCC_PLUGINS
> + depends on HAVE_ARCH_STACKLEAK
> + help
> + This option makes the kernel erase the kernel stack before it
> + returns from a system call. That reduces the information which
> + a kernel stack leak bug can reveal and blocks some uninitialized
> + stack variable attacks. This option also provides runtime checks
> + for kernel stack overflow detection.
> +
> + The tradeoff is the performance impact: on a single CPU system kernel
> + compilation sees a 1% slowdown, other systems and workloads may vary
> + and you are advised to test this feature on your expected workload
> + before deploying it.
> +
> + This plugin was ported from grsecurity/PaX. More information at:
> + * https://grsecurity.net/
> + * https://pax.grsecurity.net/
> +
> config HAVE_CC_STACKPROTECTOR
> bool
> help
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94a1868..1359e01 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -112,6 +112,7 @@ config X86
> select HAVE_ARCH_MMAP_RND_COMPAT_BITS if MMU && COMPAT
> select HAVE_ARCH_COMPAT_MMAP_BASES if MMU && COMPAT
> select HAVE_ARCH_SECCOMP_FILTER
> + select HAVE_ARCH_STACKLEAK
> select HAVE_ARCH_TRACEHOOK
> select HAVE_ARCH_TRANSPARENT_HUGEPAGE
> select HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD if X86_64
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index cdefcfd..5e9cd0a 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -44,6 +44,12 @@ __visible inline void enter_from_user_mode(void)
> static inline void enter_from_user_mode(void) {}
> #endif
>
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> +asmlinkage void erase_kstack(void);
> +#else
> +static void erase_kstack(void) {}
> +#endif
> +
> static void do_audit_syscall_entry(struct pt_regs *regs, u32 arch)
> {
> #ifdef CONFIG_X86_64
> @@ -80,8 +86,10 @@ static long syscall_trace_enter(struct pt_regs *regs)
> emulated = true;
>
> if ((emulated || (work & _TIF_SYSCALL_TRACE)) &&
> - tracehook_report_syscall_entry(regs))
> + tracehook_report_syscall_entry(regs)) {
> + erase_kstack();
> return -1L;
> + }
>
> if (emulated)
> return -1L;
> @@ -115,9 +123,11 @@ static long syscall_trace_enter(struct pt_regs *regs)
> sd.args[5] = regs->bp;
> }
>
> - ret = __secure_computing(&sd);
> - if (ret == -1)
> + ret = secure_computing(&sd);
> + if (ret == -1) {
> + erase_kstack();
> return ret;
> + }
> }
> #endif
>
> @@ -126,6 +136,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
>
> do_audit_syscall_entry(regs, arch);
>
> + erase_kstack();
> return ret ?: regs->orig_ax;
> }
>
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index 48ef7bb..f3d2666 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -75,6 +75,73 @@
> #endif
> .endm
>
> +.macro erase_kstack
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + call erase_kstack
> +#endif
> +.endm
> +
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> +/*
> + * ebp: thread_info
> + */
> +ENTRY(erase_kstack)
> + pushl %edi
> + pushl %ecx
> + pushl %eax
> + pushl %ebp
> +
> + movl PER_CPU_VAR(current_task), %ebp
> + mov TASK_lowest_stack(%ebp), %edi
> + mov $STACKLEAK_POISON, %eax
> + std
> +
> +1:
> + mov %edi, %ecx
> + and $THREAD_SIZE_asm - 1, %ecx
> + shr $2, %ecx
> + repne scasl
> + jecxz 2f
> +
> + cmp $2*16, %ecx
> + jc 2f
> +
> + mov $2*16, %ecx
> + repe scasl
> + jecxz 2f
> + jne 1b
> +
> +2:
> + cld
> + or $2*4, %edi
> + mov %esp, %ecx
> + sub %edi, %ecx
> +
> + cmp $THREAD_SIZE_asm, %ecx
> + jb 3f
> + ud2
> +
> +3:
> + shr $2, %ecx
> + rep stosl
> +
> + /*
> + * TODO: sp0 on x86_32 is not reliable, right?
> + * Doubt because of the definition of cpu_current_top_of_stack
> + * in arch/x86/kernel/cpu/common.c.
> + */
> + mov TASK_thread_sp0(%ebp), %edi
> + sub $128, %edi
> + mov %edi, TASK_lowest_stack(%ebp)
> +
> + popl %ebp
> + popl %eax
> + popl %ecx
> + popl %edi
> + ret
> +ENDPROC(erase_kstack)
> +#endif
> +
> /*
> * User gs save/restore
> *
> @@ -445,6 +512,8 @@ ENTRY(entry_SYSENTER_32)
> ALTERNATIVE "testl %eax, %eax; jz .Lsyscall_32_done", \
> "jmp .Lsyscall_32_done", X86_FEATURE_XENPV
>
> + erase_kstack
> +
> /* Opportunistic SYSEXIT */
> TRACE_IRQS_ON /* User mode traces as IRQs on. */
> movl PT_EIP(%esp), %edx /* pt_regs->ip */
> @@ -531,6 +600,8 @@ ENTRY(entry_INT80_32)
> call do_int80_syscall_32
> .Lsyscall_32_done:
>
> + erase_kstack
> +
> restore_all:
> TRACE_IRQS_IRET
> .Lrestore_all_notrace:
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index a9a8027..c106925 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -57,6 +57,94 @@ ENDPROC(native_usergs_sysret64)
> #endif
> .endm
>
> +.macro erase_kstack
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + call erase_kstack
> +#endif
> +.endm
> +
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> +ENTRY(erase_kstack)
> + pushq %rdi
> + pushq %rcx
> + pushq %rax
> + pushq %r11
> +
> + movq PER_CPU_VAR(current_task), %r11
> + mov TASK_lowest_stack(%r11), %rdi
> + mov $STACKLEAK_POISON, %rax
> + std
> +
> +1:
> + /*
> + * Let's search for the poison value in the stack.
> + * Start from the lowest_stack and go to the bottom (see std above).
> + */
> + mov %edi, %ecx
> + and $THREAD_SIZE_asm - 1, %ecx
> + shr $3, %ecx
> + repne scasq
> + jecxz 2f /* Didn't find it. Go to poisoning. */
> +
> + /*
> + * Found the poison value in the stack. Go to poisoning if there are
> + * less than 16 qwords left.
> + */
> + cmp $2*8, %ecx
> + jc 2f
> +
> + /*
> + * Check that 16 further qwords contain poison (avoid false positives).
> + * If so, the part of the stack below the address in %rdi is likely
> + * to be poisoned. Otherwise we need to search deeper.
> + */
> + mov $2*8, %ecx
> + repe scasq
> + jecxz 2f /* Poison the upper part of the stack. */
> + jne 1b /* Search deeper. */
> +
> +2:
> + /*
> + * Prepare the counter for poisoning the kernel stack between
> + * %rdi and %rsp.
> + *
> + * TODO: Sorry, don't understand why the following OR instruction is
> + * needed. That may be connected to the thread.lowest_stack
> + * initialization in arch/x86/kernel/process_64.c, where it is set
> + * to the task_stack_page address + 2 * sizeof(unsigned long).
> + */
> + cld
> + or $2*8, %rdi
> + mov %esp, %ecx
> + sub %edi, %ecx
> +
> + /* Check that the counter value is sane. */
> + cmp $THREAD_SIZE_asm, %rcx
> + jb 3f
> + ud2
> +
> +3:
> + /*
> + * So let's write the poison value to the kernel stack. Start from the
> + * address in %rdi and move up (see cld above) to the address in %rsp
> + * (not included, used memory).
> + */
> + shr $3, %ecx
> + rep stosq
> +
> + /* Set the lowest_stack value to the top_of_stack - 256. */
> + mov TASK_thread_sp0(%r11), %rdi
> + sub $256, %rdi
> + mov %rdi, TASK_lowest_stack(%r11)
> +
> + popq %r11
> + popq %rax
> + popq %rcx
> + popq %rdi
> + ret
> +ENDPROC(erase_kstack)
> +#endif
> +
> /*
> * When dynamic function tracer is enabled it will add a breakpoint
> * to all locations that it is about to modify, sync CPUs, update
> @@ -217,6 +305,8 @@ entry_SYSCALL_64_fastpath:
> testl $_TIF_ALLWORK_MASK, TASK_TI_flags(%r11)
> jnz 1f
>
> + erase_kstack
> +
> LOCKDEP_SYS_EXIT
> TRACE_IRQS_ON /* user mode is traced as IRQs on */
> movq RIP(%rsp), %rcx
> @@ -245,6 +335,8 @@ entry_SYSCALL64_slow_path:
> call do_syscall_64 /* returns with IRQs disabled */
>
> return_from_SYSCALL_64:
> + erase_kstack
> +
> RESTORE_EXTRA_REGS
> TRACE_IRQS_IRETQ /* we're about to change IF */
>
> @@ -415,6 +507,7 @@ ENTRY(ret_from_fork)
> 2:
> movq %rsp, %rdi
> call syscall_return_slowpath /* returns with IRQs disabled */
> + erase_kstack
> TRACE_IRQS_ON /* user mode is traced as IRQS on */
> SWAPGS
> jmp restore_regs_and_iret
> @@ -527,6 +620,12 @@ ret_from_intr:
> GLOBAL(retint_user)
> mov %rsp,%rdi
> call prepare_exit_to_usermode
> +
> + /*
> + * TODO: Do we need to call erase_kstack here?
> + * The PaX patch has it here commented out.
> + */
> +
> TRACE_IRQS_IRETQ
> SWAPGS
> jmp restore_regs_and_iret
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index e1721da..05a75a6 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -18,6 +18,12 @@
>
> .section .entry.text, "ax"
>
> + .macro erase_kstack
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + call erase_kstack
> +#endif
> + .endm
> +
> /*
> * 32-bit SYSENTER entry.
> *
> @@ -229,6 +235,7 @@ ENTRY(entry_SYSCALL_compat)
>
> /* Opportunistic SYSRET */
> sysret32_from_system_call:
> + erase_kstack
> TRACE_IRQS_ON /* User mode traces as IRQs on. */
> movq RBX(%rsp), %rbx /* pt_regs->rbx */
> movq RBP(%rsp), %rbp /* pt_regs->rbp */
> @@ -337,6 +344,7 @@ ENTRY(entry_INT80_compat)
> .Lsyscall_32_done:
>
> /* Go back to user mode. */
> + erase_kstack
> TRACE_IRQS_ON
> SWAPGS
> jmp restore_regs_and_iret
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 6a79547..f67417d 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -470,6 +470,10 @@ struct thread_struct {
>
> mm_segment_t addr_limit;
>
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + unsigned long lowest_stack;
> +#endif
> +
> unsigned int sig_on_uaccess_err:1;
> unsigned int uaccess_err:1; /* uaccess failed */
>
> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index de827d6..4ed7451 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -37,6 +37,10 @@ void common(void) {
> BLANK();
> OFFSET(TASK_TI_flags, task_struct, thread_info.flags);
> OFFSET(TASK_addr_limit, task_struct, thread.addr_limit);
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + OFFSET(TASK_lowest_stack, task_struct, thread.lowest_stack);
> + OFFSET(TASK_thread_sp0, task_struct, thread.sp0);
> +#endif
>
> BLANK();
> OFFSET(crypto_tfm_ctx_offset, crypto_tfm, __crt_ctx);
> @@ -73,6 +77,11 @@ void common(void) {
> OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
> #endif
>
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + BLANK();
> + DEFINE(THREAD_SIZE_asm, THREAD_SIZE);
> +#endif
> +
> #ifdef CONFIG_XEN
> BLANK();
> OFFSET(XEN_vcpu_info_mask, vcpu_info, evtchn_upcall_mask);
> diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
> index e5f0b40..2eaa810 100644
> --- a/arch/x86/kernel/dumpstack_32.c
> +++ b/arch/x86/kernel/dumpstack_32.c
> @@ -162,3 +162,15 @@ void show_regs(struct pt_regs *regs)
> }
> pr_cont("\n");
> }
> +
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> +void __used check_alloca(unsigned long size)
> +{
> + unsigned long sp = (unsigned long)&sp, stack_left;
> +
> + /* all kernel stacks are of the same size */
> + stack_left = sp & (THREAD_SIZE - 1);
> + BUG_ON(stack_left < 256 || size >= stack_left - 256);
> +}
> +EXPORT_SYMBOL(check_alloca);
> +#endif
> diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
> index 3e1471d..dc1bb0b 100644
> --- a/arch/x86/kernel/dumpstack_64.c
> +++ b/arch/x86/kernel/dumpstack_64.c
> @@ -178,3 +178,36 @@ void show_regs(struct pt_regs *regs)
> }
> pr_cont("\n");
> }
> +
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> +void __used check_alloca(unsigned long size)
> +{
> + struct stack_info stack_info = {0};
> + unsigned long visit_mask = 0;
> + unsigned long sp = (unsigned long)&sp;
> + unsigned long stack_left;
> +
> + BUG_ON(get_stack_info(&sp, current, &stack_info, &visit_mask));
> +
> + switch (stack_info.type) {
> + case STACK_TYPE_TASK:
> + stack_left = sp & (THREAD_SIZE - 1);
> + break;
> +
> + case STACK_TYPE_IRQ:
> + stack_left = sp & (IRQ_STACK_SIZE - 1);
> + break;
> +
> + case STACK_TYPE_EXCEPTION ... STACK_TYPE_EXCEPTION_LAST:
> + stack_left = sp & (EXCEPTION_STKSZ - 1);
> + break;
> +
> + case STACK_TYPE_SOFTIRQ:
> + default:
> + BUG();
> + }
> +
> + BUG_ON(stack_left < 256 || size >= stack_left - 256);
> +}
> +EXPORT_SYMBOL(check_alloca);
> +#endif
> diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
> index c6d6dc5..26f7301 100644
> --- a/arch/x86/kernel/process_32.c
> +++ b/arch/x86/kernel/process_32.c
> @@ -136,6 +136,11 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
> p->thread.sp0 = (unsigned long) (childregs+1);
> memset(p->thread.ptrace_bps, 0, sizeof(p->thread.ptrace_bps));
>
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + p->thread.lowest_stack = (unsigned long)task_stack_page(p) +
> + 2 * sizeof(unsigned long);
> +#endif
> +
> if (unlikely(p->flags & PF_KTHREAD)) {
> /* kernel thread */
> memset(childregs, 0, sizeof(struct pt_regs));
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index c3169be..8cdee97 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -167,6 +167,11 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
> p->thread.sp = (unsigned long) fork_frame;
> p->thread.io_bitmap_ptr = NULL;
>
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> + p->thread.lowest_stack = (unsigned long)task_stack_page(p) +
> + 2 * sizeof(unsigned long);
> +#endif
> +
Do you know why this initialization value was chosen? In clear_kstack,
this gets reset to sp0. It seems like this forces a clear of the entire
stack on the first call of clear_kstack?
> savesegment(gs, p->thread.gsindex);
> p->thread.gsbase = p->thread.gsindex ? 0 : me->thread.gsbase;
> savesegment(fs, p->thread.fsindex);
> diff --git a/fs/exec.c b/fs/exec.c
> index 62175cb..22ba66d 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1949,3 +1949,20 @@ COMPAT_SYSCALL_DEFINE5(execveat, int, fd,
> argv, envp, flags);
> }
> #endif
> +
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> +void __used track_stack(void)
> +{
> + unsigned long sp = (unsigned long)&sp;
> +
Mark Rutland pointed out in the review of my arm64 draft that
using current_stack_pointer() is cleaner.
> + if (sp < current->thread.lowest_stack &&
> + sp >= (unsigned long)task_stack_page(current) +
> + 2 * sizeof(unsigned long)) {
> + current->thread.lowest_stack = sp;
> + }
> +
> + if (unlikely((sp & ~(THREAD_SIZE - 1)) < (THREAD_SIZE / 16)))
> + BUG();
> +}
All of the above checks have the x86-isms. Can this either be
cleaned up or made architecture specific?
> +EXPORT_SYMBOL(track_stack);
> +#endif
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 219f82f..a97d334 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -585,4 +585,8 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
> (_________p1); \
> })
>
> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> +# define STACKLEAK_POISON -0xBEEF
> +#endif
> +
Pulling out the poison to be common is certainly good, compiler.h doesn't seem
quite right though. I don't have a strong opinion if there are no other
objections or better ideas.
> #endif /* __LINUX_COMPILER_H */
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 2e0e2ea..aab824d 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -33,6 +33,9 @@ ifdef CONFIG_GCC_PLUGINS
> gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT) += -DRANDSTRUCT_PLUGIN
> gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT_PERFORMANCE) += -fplugin-arg-randomize_layout_plugin-performance-mode
>
> + gcc-plugin-$(CONFIG_GCC_PLUGIN_STACKLEAK) += stackleak_plugin.so
> + gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) += -DSTACKLEAK_PLUGIN -fplugin-arg-stackleak_plugin-track-lowest-sp=100
> +
Might be useful to make the bytes limit a Kconfig
for tuning.
Thanks,
Laura
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
