Messages in this thread | | | Date | Thu, 24 Feb 2022 19:27:22 -0800 | From | Kees Cook <> | Subject | Re: [PATCH] xtensa: Implement "current_stack_pointer" |
| |
On Wed, Feb 23, 2022 at 10:58:00PM -0800, Max Filippov wrote: > On Wed, Feb 23, 2022 at 10:43 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Wed, Feb 23, 2022 at 10:22:59PM -0800, Max Filippov wrote: > > > On Wed, Feb 23, 2022 at 10:05 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > > > To follow the existing per-arch conventions replace open-coded uses > > > > of asm "sp" as "current_stack_pointer". This will let it be used in > > > > non-arch places (like HARDENED_USERCOPY). > > > > > > > > Cc: Chris Zankel <chris@zankel.net> > > > > Cc: Max Filippov <jcmvbkbc@gmail.com> > > > > Cc: Marc Zyngier <maz@kernel.org> > > > > Cc: linux-xtensa@linux-xtensa.org > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > > --- > > > > arch/xtensa/Kconfig | 1 + > > > > arch/xtensa/include/asm/current.h | 2 ++ > > > > arch/xtensa/include/asm/stacktrace.h | 2 +- > > > > arch/xtensa/kernel/irq.c | 3 +-- > > > > 4 files changed, 5 insertions(+), 3 deletions(-) > > > > > > Acked-by: Max Filippov <jcmvbkbc@gmail.com> > > > > Thanks! And apologies, my cross-compiler tricked me into thinking this > > patch compiled without problems. It did, however. I've change the patch > > slightly to deal with the needed casts: > > > > diff --git a/arch/xtensa/include/asm/stacktrace.h b/arch/xtensa/include/asm/stacktrace.h > > index fe06e8ed162b..a85e785a6288 100644 > > --- a/arch/xtensa/include/asm/stacktrace.h > > +++ b/arch/xtensa/include/asm/stacktrace.h > > @@ -19,14 +19,14 @@ struct stackframe { > > > > static __always_inline unsigned long *stack_pointer(struct task_struct *task) > > { > > - unsigned long *sp; > > + unsigned long sp; > > > > if (!task || task == current) > > - __asm__ __volatile__ ("mov %0, a1\n" : "=a"(sp)); > > + sp = current_stack_pointer; > > else > > - sp = (unsigned long *)task->thread.sp; > > + sp = task->thread.sp; > > > > - return sp; > > + return (unsigned long *)sp; > > } > > > > void walk_stackframe(unsigned long *sp, > > > > Shall I send a v2, or just carry this fix in my tree? > > This additional change looks good to me, if you could > fold it into the original patch that'd be perfect. But separate > patch would also work.
Thanks!
-- Kees Cook
| |