Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 23 Feb 2022 22:43:15 -0800
From:   Kees Cook <keescook@chromium.org>
To:     Max Filippov <jcmvbkbc@gmail.com>
Cc:     Chris Zankel <chris@zankel.net>, Marc Zyngier <maz@kernel.org>,
        "open list:TENSILICA XTENSA PORT (xtensa)" 
        <linux-xtensa@linux-xtensa.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-hardening@vger.kernel.org
Subject: Re: [PATCH] xtensa: Implement "current_stack_pointer"
Message-ID: <202202232239.FF2343D42@keescook>
References: <20220224060503.1856302-1-keescook@chromium.org>
 <CAMo8BfJFJE-n3=AF+pb9_6oF3gzxX7a+7aBrASHjjNX5byqDqw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMo8BfJFJE-n3=AF+pb9_6oF3gzxX7a+7aBrASHjjNX5byqDqw@mail.gmail.com>
Precedence: bulk

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?

Sorry for the glitch!

-Kees

-- 
Kees Cook