Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks

From: Brian Gerst
Date: Tue Sep 20 2016 - 21:11:03 EST


On Tue, Sep 20, 2016 at 4:02 PM, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> Thanks to all the recent x86 entry code refactoring, most tasks' kernel
> stacks start at the same offset right below their saved pt_regs,
> regardless of which syscall was used to enter the kernel. That creates
> a nice convention which makes it straightforward to identify the end of
> the stack, which can be useful for the unwinder to verify the stack is
> sane.
>
> Calling schedule_tail() directly breaks that convention because its an
> asmlinkage function so its argument has to be pushed on the stack. Add
> a wrapper which creates a proper "end of stack" frame header before the
> call.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
> arch/x86/entry/entry_32.S | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index deef561..f0a7444 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -44,6 +44,7 @@
> #include <asm/alternative-asm.h>
> #include <asm/asm.h>
> #include <asm/smap.h>
> +#include <asm/frame.h>
>
> .section .entry.text, "ax"
>
> @@ -237,6 +238,23 @@ ENTRY(__switch_to_asm)
> END(__switch_to_asm)
>
> /*
> + * The unwinder expects the last frame on the stack to always be at the same
> + * offset from the end of the page, which allows it to validate the stack.
> + * Calling schedule_tail() directly would break that convention because its an
> + * asmlinkage function so its argument has to be pushed on the stack. This
> + * wrapper creates a proper "end of stack" frame header before the call.
> + */
> +ENTRY(schedule_tail_wrapper)
> + FRAME_BEGIN
> +
> + pushl %eax
> + call schedule_tail
> + popl %eax
> +
> + FRAME_END
> + ret
> +ENDPROC(schedule_tail_wrapper)
> +/*
> * A newly forked process directly context switches into this address.
> *
> * eax: prev task we switched from
> @@ -244,9 +262,7 @@ END(__switch_to_asm)
> * edi: kernel thread arg
> */
> ENTRY(ret_from_fork)
> - pushl %eax
> - call schedule_tail
> - popl %eax
> + call schedule_tail_wrapper
>
> testl %ebx, %ebx
> jnz 1f /* kernel threads are uncommon */
> --
> 2.7.4
>

Dropping asmlinkage from schedule_tail() would be a better option if possible.

--
Brian Gerst