[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <202202241927.B5368AFC@keescook>
Date: Thu, 24 Feb 2022 19:27:22 -0800
From: Kees Cook <keescook@...omium.org>
To: Max Filippov <jcmvbkbc@...il.com>
Cc: Chris Zankel <chris@...kel.net>, Marc Zyngier <maz@...nel.org>,
"open list:TENSILICA XTENSA PORT (xtensa)"
<linux-xtensa@...ux-xtensa.org>,
LKML <linux-kernel@...r.kernel.org>,
linux-hardening@...r.kernel.org
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@...omium.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@...omium.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@...kel.net>
> > > > Cc: Max Filippov <jcmvbkbc@...il.com>
> > > > Cc: Marc Zyngier <maz@...nel.org>
> > > > Cc: linux-xtensa@...ux-xtensa.org
> > > > Signed-off-by: Kees Cook <keescook@...omium.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@...il.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
Powered by blists - more mailing lists