Messages in this thread | | | Date | Wed, 30 Mar 2022 16:12:18 -0700 (PDT) | Subject | Re: [PATCH] riscv: Rename "sp_in_global" to "current_stack_pointer" | From | Palmer Dabbelt <> |
| |
On Tue, 22 Mar 2022 19:16:28 PDT (-0700), Palmer Dabbelt wrote: > On Wed, 23 Feb 2022 22:04:11 PST (-0800), keescook@chromium.org wrote: >> To follow the existing per-arch conventions, rename "sp_in_global" to >> "current_stack_pointer". This will let it be used in non-arch places >> (like HARDENED_USERCOPY). >> >> Cc: Paul Walmsley <paul.walmsley@sifive.com> >> Cc: Palmer Dabbelt <palmer@dabbelt.com> >> Cc: Albert Ou <aou@eecs.berkeley.edu> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Jisheng Zhang <jszhang@kernel.org> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Chen Huang <chenhuang5@huawei.com> >> Cc: linux-riscv@lists.infradead.org >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> arch/riscv/Kconfig | 1 + >> arch/riscv/include/asm/current.h | 2 ++ >> arch/riscv/kernel/stacktrace.c | 4 +--- >> 3 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 5adcbd9b5e88..b120c32697af 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -16,6 +16,7 @@ config RISCV >> select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION >> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2 >> select ARCH_HAS_BINFMT_FLAT >> + select ARCH_HAS_CURRENT_STACK_POINTER >> select ARCH_HAS_DEBUG_VM_PGTABLE >> select ARCH_HAS_DEBUG_VIRTUAL if MMU >> select ARCH_HAS_DEBUG_WX >> diff --git a/arch/riscv/include/asm/current.h b/arch/riscv/include/asm/current.h >> index 1de233d8e8de..21774d868c65 100644 >> --- a/arch/riscv/include/asm/current.h >> +++ b/arch/riscv/include/asm/current.h >> @@ -33,6 +33,8 @@ static __always_inline struct task_struct *get_current(void) >> >> #define current get_current() >> >> +register unsigned long current_stack_pointer __asm__("sp"); >> + >> #endif /* __ASSEMBLY__ */ >> >> #endif /* _ASM_RISCV_CURRENT_H */ >> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c >> index 201ee206fb57..278380e2e956 100644 >> --- a/arch/riscv/kernel/stacktrace.c >> +++ b/arch/riscv/kernel/stacktrace.c >> @@ -14,8 +14,6 @@ >> >> #include <asm/stacktrace.h> >> >> -register unsigned long sp_in_global __asm__("sp"); >> - >> #ifdef CONFIG_FRAME_POINTER >> >> void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, >> @@ -77,7 +75,7 @@ void notrace walk_stackframe(struct task_struct *task, >> sp = user_stack_pointer(regs); >> pc = instruction_pointer(regs); >> } else if (task == NULL || task == current) { >> - sp = sp_in_global; >> + sp = current_stack_pointer; >> pc = (unsigned long)walk_stackframe; >> } else { >> /* task blocked in __switch_to */ > > Sorry to be slow on this one. This looks good, but another use of > sp_in_global snuck into fixes and I don't merge fixes into for-next so > that makes this a bit of a headache. > > There's a bunch of stuff in flight, so I'm going to hold off on this > until after my first week's merge window PR. I intend on merging the > following, which is at palmer/riscv-sp for now (I'll probably just > cherry-pick that, I usually start basing stuff on Linus' merge of my PR > and just cherry-pick in single patches). > > commit 7c4f6bb6fb352cfa6028191c2888356b9157a332 > gpg: Signature made Tue 22 Mar 2022 11:12:19 AM PDT > gpg: using RSA key 2B3C3747446843B24A943A7A2E1319F35FBB1889 > gpg: issuer "palmer@dabbelt.com" > gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" [ultimate] > gpg: aka "Palmer Dabbelt <palmer@rivosinc.com>" [ultimate] > Author: Kees Cook <keescook@chromium.org> > Date: Wed Feb 23 22:04:11 2022 -0800 > > riscv: Rename "sp_in_global" to "current_stack_pointer" > > To follow the existing per-arch conventions, rename "sp_in_global" to > "current_stack_pointer". This will let it be used in non-arch places > (like HARDENED_USERCOPY). > > Signed-off-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 5adcbd9b5e88..b120c32697af 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -16,6 +16,7 @@ config RISCV > select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION > select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2 > select ARCH_HAS_BINFMT_FLAT > + select ARCH_HAS_CURRENT_STACK_POINTER > select ARCH_HAS_DEBUG_VM_PGTABLE > select ARCH_HAS_DEBUG_VIRTUAL if MMU > select ARCH_HAS_DEBUG_WX > diff --git a/arch/riscv/include/asm/current.h b/arch/riscv/include/asm/current.h > index 1de233d8e8de..21774d868c65 100644 > --- a/arch/riscv/include/asm/current.h > +++ b/arch/riscv/include/asm/current.h > @@ -33,6 +33,8 @@ static __always_inline struct task_struct *get_current(void) > > #define current get_current() > > +register unsigned long current_stack_pointer __asm__("sp"); > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_RISCV_CURRENT_H */ > diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c > index 14d2b53ec322..08d11a53f39e 100644 > --- a/arch/riscv/kernel/stacktrace.c > +++ b/arch/riscv/kernel/stacktrace.c > @@ -14,8 +14,6 @@ > > #include <asm/stacktrace.h> > > -register unsigned long sp_in_global __asm__("sp"); > - > #ifdef CONFIG_FRAME_POINTER > > void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, > @@ -30,7 +28,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, > pc = instruction_pointer(regs); > } else if (task == NULL || task == current) { > fp = (unsigned long)__builtin_frame_address(0); > - sp = sp_in_global; > + sp = current_stack_pointer; > pc = (unsigned long)walk_stackframe; > } else { > /* task blocked in __switch_to */ > @@ -78,7 +76,7 @@ void notrace walk_stackframe(struct task_struct *task, > sp = user_stack_pointer(regs); > pc = instruction_pointer(regs); > } else if (task == NULL || task == current) { > - sp = sp_in_global; > + sp = current_stack_pointer; > pc = (unsigned long)walk_stackframe; > } else { > /* task blocked in __switch_to */ > > Thanks!
Thanks, this is on for-next. I put HARDENED_USERCOPY=y test in for SLAB and SLUB, not sure if there's anything else that's interesting to test (we're missing some bits needed for STACKLEAK).
| |