| Message ID | 20220309204537.390428-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: Only use current_stack_pointer on GCC | expand |
Nit: I think the subject needs to be updated (I assume this was written before Nick's fix?). On Wed, Mar 09, 2022 at 12:45:37PM -0800, Kees Cook wrote: > Unfortunately, Clang did not have support for "sp" as a global register > definition, and was crashing after the addition of current_stack_pointer. > This has been fixed in Clang 15, but earlier Clang versions need to > avoid this code, so add a versioned test and revert back to the > open-coded asm instances. Fixes Clang build error: > > fatal error: error in backend: Invalid register name global variable > > Fixes: 200ed341b864 ("mips: Implement "current_stack_pointer"") > Reported-by: Nathan Chancellor <nathan@kernel.org> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Yanteng Si <siyanteng01@gmail.com> > Cc: linux-mips@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> Thanks for sending this! Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > arch/mips/Kconfig | 2 +- > arch/mips/include/asm/thread_info.h | 2 ++ > arch/mips/kernel/irq.c | 3 ++- > arch/mips/lib/uncached.c | 4 +++- > 4 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index 3f58b45fc953..15769013f46e 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -4,7 +4,7 @@ config MIPS > default y > select ARCH_32BIT_OFF_T if !64BIT > select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT > - select ARCH_HAS_CURRENT_STACK_POINTER > + select ARCH_HAS_CURRENT_STACK_POINTER if !CC_IS_CLANG || CLANG_VERSION >= 150000 Nit: This can be 140000, as release/14.x has received the fix: https://github.com/llvm/llvm-project/commit/0826716786cd4a8c7cbcb8c01e4d9fac46b7a17a > select ARCH_HAS_DEBUG_VIRTUAL if !64BIT > select ARCH_HAS_FORTIFY_SOURCE > select ARCH_HAS_KCOV > diff --git a/arch/mips/include/asm/thread_info.h b/arch/mips/include/asm/thread_info.h > index 4463348d2372..ecae7470faa4 100644 > --- a/arch/mips/include/asm/thread_info.h > +++ b/arch/mips/include/asm/thread_info.h > @@ -69,7 +69,9 @@ static inline struct thread_info *current_thread_info(void) > return __current_thread_info; > } > > +#ifdef CONFIG_ARCH_HAS_CURRENT_STACK_POINTER > register unsigned long current_stack_pointer __asm__("sp"); > +#endif > > #endif /* !__ASSEMBLY__ */ > > diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c > index fc313c49a417..5e11582fe308 100644 > --- a/arch/mips/kernel/irq.c > +++ b/arch/mips/kernel/irq.c > @@ -75,8 +75,9 @@ void __init init_IRQ(void) > #ifdef CONFIG_DEBUG_STACKOVERFLOW > static inline void check_stack_overflow(void) > { > - unsigned long sp = current_stack_pointer; > + unsigned long sp; > > + __asm__ __volatile__("move %0, $sp" : "=r" (sp)); > sp &= THREAD_MASK; > > /* > diff --git a/arch/mips/lib/uncached.c b/arch/mips/lib/uncached.c > index f8d4ca046c3e..f80a67c092b6 100644 > --- a/arch/mips/lib/uncached.c > +++ b/arch/mips/lib/uncached.c > @@ -40,7 +40,9 @@ unsigned long run_uncached(void *func) > register long ret __asm__("$2"); > long lfunc = (long)func, ufunc; > long usp; > - long sp = current_stack_pointer; > + long sp; > + > + __asm__("move %0, $sp" : "=r" (sp)); > > if (sp >= (long)CKSEG0 && sp < (long)CKSEG2) > usp = CKSEG1ADDR(sp); > -- > 2.32.0 >
On Wed, Mar 09, 2022 at 01:51:13PM -0700, Nathan Chancellor wrote: > Nit: I think the subject needs to be updated (I assume this was written > before Nick's fix?). > > On Wed, Mar 09, 2022 at 12:45:37PM -0800, Kees Cook wrote: > > Unfortunately, Clang did not have support for "sp" as a global register > > definition, and was crashing after the addition of current_stack_pointer. > > This has been fixed in Clang 15, but earlier Clang versions need to > > avoid this code, so add a versioned test and revert back to the > > open-coded asm instances. Fixes Clang build error: > > > > fatal error: error in backend: Invalid register name global variable > > > > Fixes: 200ed341b864 ("mips: Implement "current_stack_pointer"") > > Reported-by: Nathan Chancellor <nathan@kernel.org> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Guenter Roeck <linux@roeck-us.net> > > Cc: Yanteng Si <siyanteng01@gmail.com> > > Cc: linux-mips@vger.kernel.org > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Thanks for sending this! > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > > --- > > arch/mips/Kconfig | 2 +- > > arch/mips/include/asm/thread_info.h | 2 ++ > > arch/mips/kernel/irq.c | 3 ++- > > arch/mips/lib/uncached.c | 4 +++- > > 4 files changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > > index 3f58b45fc953..15769013f46e 100644 > > --- a/arch/mips/Kconfig > > +++ b/arch/mips/Kconfig > > @@ -4,7 +4,7 @@ config MIPS > > default y > > select ARCH_32BIT_OFF_T if !64BIT > > select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT > > - select ARCH_HAS_CURRENT_STACK_POINTER > > + select ARCH_HAS_CURRENT_STACK_POINTER if !CC_IS_CLANG || CLANG_VERSION >= 150000 > > Nit: This can be 140000, as release/14.x has received the fix: > > https://github.com/llvm/llvm-project/commit/0826716786cd4a8c7cbcb8c01e4d9fac46b7a17a Oh! Excellent. Thanks; I missed that it made the branch. I'll send a v2.
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 3f58b45fc953..15769013f46e 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -4,7 +4,7 @@ config MIPS default y select ARCH_32BIT_OFF_T if !64BIT select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT - select ARCH_HAS_CURRENT_STACK_POINTER + select ARCH_HAS_CURRENT_STACK_POINTER if !CC_IS_CLANG || CLANG_VERSION >= 150000 select ARCH_HAS_DEBUG_VIRTUAL if !64BIT select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_KCOV diff --git a/arch/mips/include/asm/thread_info.h b/arch/mips/include/asm/thread_info.h index 4463348d2372..ecae7470faa4 100644 --- a/arch/mips/include/asm/thread_info.h +++ b/arch/mips/include/asm/thread_info.h @@ -69,7 +69,9 @@ static inline struct thread_info *current_thread_info(void) return __current_thread_info; } +#ifdef CONFIG_ARCH_HAS_CURRENT_STACK_POINTER register unsigned long current_stack_pointer __asm__("sp"); +#endif #endif /* !__ASSEMBLY__ */ diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c index fc313c49a417..5e11582fe308 100644 --- a/arch/mips/kernel/irq.c +++ b/arch/mips/kernel/irq.c @@ -75,8 +75,9 @@ void __init init_IRQ(void) #ifdef CONFIG_DEBUG_STACKOVERFLOW static inline void check_stack_overflow(void) { - unsigned long sp = current_stack_pointer; + unsigned long sp; + __asm__ __volatile__("move %0, $sp" : "=r" (sp)); sp &= THREAD_MASK; /* diff --git a/arch/mips/lib/uncached.c b/arch/mips/lib/uncached.c index f8d4ca046c3e..f80a67c092b6 100644 --- a/arch/mips/lib/uncached.c +++ b/arch/mips/lib/uncached.c @@ -40,7 +40,9 @@ unsigned long run_uncached(void *func) register long ret __asm__("$2"); long lfunc = (long)func, ufunc; long usp; - long sp = current_stack_pointer; + long sp; + + __asm__("move %0, $sp" : "=r" (sp)); if (sp >= (long)CKSEG0 && sp < (long)CKSEG2) usp = CKSEG1ADDR(sp);
Unfortunately, Clang did not have support for "sp" as a global register definition, and was crashing after the addition of current_stack_pointer. This has been fixed in Clang 15, but earlier Clang versions need to avoid this code, so add a versioned test and revert back to the open-coded asm instances. Fixes Clang build error: fatal error: error in backend: Invalid register name global variable Fixes: 200ed341b864 ("mips: Implement "current_stack_pointer"") Reported-by: Nathan Chancellor <nathan@kernel.org> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Marc Zyngier <maz@kernel.org> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Yanteng Si <siyanteng01@gmail.com> Cc: linux-mips@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/mips/Kconfig | 2 +- arch/mips/include/asm/thread_info.h | 2 ++ arch/mips/kernel/irq.c | 3 ++- arch/mips/lib/uncached.c | 4 +++- 4 files changed, 8 insertions(+), 3 deletions(-)