Messages in this thread | | | From | Brian Gerst <> | Date | Tue, 20 Sep 2016 21:10:55 -0400 | Subject | Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks |
| |
On Tue, Sep 20, 2016 at 4:02 PM, Josh Poimboeuf <jpoimboe@redhat.com> 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@redhat.com> > --- > 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
| |