| Message ID | 20220224060448.1856091-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Headers | show |
| Series | m68k: Implement "current_stack_pointer" | expand |
Hi Kees, On Thu, Feb 24, 2022 at 7:04 AM Kees Cook <keescook@chromium.org> wrote: > To follow the existing per-arch conventions, add asm "sp" as > "current_stack_pointer". This will let it be used in non-arch places > (like HARDENED_USERCOPY). > > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: linux-m68k@lists.linux-m68k.org > Signed-off-by: Kees Cook <keescook@chromium.org> Thanks for your patch! > --- a/arch/m68k/include/asm/current.h > +++ b/arch/m68k/include/asm/current.h > @@ -24,6 +24,8 @@ static inline struct task_struct *get_current(void) > > #define current get_current() > > +register unsigned long current_stack_pointer __asm__("sp"); I don't know what HARDENED_USERCOPY does, so I don't know if you need "usp" (see rdusp()) or "sp"? > + > #endif /* CONFNIG_MMU */ > > #endif /* !(_M68K_CURRENT_H) */ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
From: Geert Uytterhoeven > Sent: 24 February 2022 08:59 ... > > +register unsigned long current_stack_pointer __asm__("sp"); > > I don't know what HARDENED_USERCOPY does, so I don't know if you need > "usp" (see rdusp()) or "sp"? HARDENED_USERCOPY significantly slows down some systems calls (especially things like sendmsg()) by trying to run-time verify that the kernel buffer doesn't overrun a stack frame or kmalloc()ed buffer. I've got measurable improvements by either using __copy_to/from_user() (which skips the tests) or user_access_begin() and __get_user(). At the moment the code for reading a compat iovec[] is actually faster than that for a native one! David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Thu, Feb 24, 2022 at 10:12 AM David Laight <David.Laight@aculab.com> wrote: > From: Geert Uytterhoeven > > Sent: 24 February 2022 08:59 > ... > > > +register unsigned long current_stack_pointer __asm__("sp"); > > > > I don't know what HARDENED_USERCOPY does, so I don't know if you need > > "usp" (see rdusp()) or "sp"? > > HARDENED_USERCOPY significantly slows down some systems calls > (especially things like sendmsg()) by trying to run-time verify > that the kernel buffer doesn't overrun a stack frame or kmalloc()ed Kernel stack frame of user stack frame? > buffer. > > I've got measurable improvements by either using __copy_to/from_user() > (which skips the tests) or user_access_begin() and __get_user(). > > At the moment the code for reading a compat iovec[] is actually > faster than that for a native one! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: 24 February 2022 09:17 > > On Thu, Feb 24, 2022 at 10:12 AM David Laight <David.Laight@aculab.com> wrote: > > From: Geert Uytterhoeven > > > Sent: 24 February 2022 08:59 > > ... > > > > +register unsigned long current_stack_pointer __asm__("sp"); > > > > > > I don't know what HARDENED_USERCOPY does, so I don't know if you need > > > "usp" (see rdusp()) or "sp"? > > > > HARDENED_USERCOPY significantly slows down some systems calls > > (especially things like sendmsg()) by trying to run-time verify > > that the kernel buffer doesn't overrun a stack frame or kmalloc()ed > > Kernel stack frame of user stack frame? Kernel, the kernel doesn't care if the user stack gets trashed. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi David, Kees, On Thu, Feb 24, 2022 at 10:54 AM David Laight <David.Laight@aculab.com> wrote: > From: Geert Uytterhoeven <geert@linux-m68k.org> > > Sent: 24 February 2022 09:17 > > > > On Thu, Feb 24, 2022 at 10:12 AM David Laight <David.Laight@aculab.com> wrote: > > > From: Geert Uytterhoeven > > > > Sent: 24 February 2022 08:59 > > > ... > > > > > +register unsigned long current_stack_pointer __asm__("sp"); > > > > > > > > I don't know what HARDENED_USERCOPY does, so I don't know if you need > > > > "usp" (see rdusp()) or "sp"? > > > > > > HARDENED_USERCOPY significantly slows down some systems calls > > > (especially things like sendmsg()) by trying to run-time verify > > > that the kernel buffer doesn't overrun a stack frame or kmalloc()ed > > > > Kernel stack frame of user stack frame? > > Kernel, the kernel doesn't care if the user stack gets trashed. OK. Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Kees: Do you want me to queue this in the m68k for-v5.18 branch, or do you want to take it yourself, together with the HARDENED_USERCOPY work? In case of the latter: Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Please let me know. Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Thu, Feb 24, 2022 at 10:56:09AM +0100, Geert Uytterhoeven wrote: > Hi David, Kees, > > On Thu, Feb 24, 2022 at 10:54 AM David Laight <David.Laight@aculab.com> wrote: > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > > Sent: 24 February 2022 09:17 > > > > > > On Thu, Feb 24, 2022 at 10:12 AM David Laight <David.Laight@aculab.com> wrote: > > > > From: Geert Uytterhoeven > > > > > Sent: 24 February 2022 08:59 > > > > ... > > > > > > +register unsigned long current_stack_pointer __asm__("sp"); > > > > > > > > > > I don't know what HARDENED_USERCOPY does, so I don't know if you need > > > > > "usp" (see rdusp()) or "sp"? > > > > > > > > HARDENED_USERCOPY significantly slows down some systems calls > > > > (especially things like sendmsg()) by trying to run-time verify > > > > that the kernel buffer doesn't overrun a stack frame or kmalloc()ed > > > > > > Kernel stack frame of user stack frame? > > > > Kernel, the kernel doesn't care if the user stack gets trashed. Right, this is strictly a kernel-side check in mm/usercopy.c: https://lore.kernel.org/linux-hardening/20220225173345.3358109-1-keescook@chromium.org/ > > OK. > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Kees: Do you want me to queue this in the m68k for-v5.18 branch, or do > you want to take it yourself, together with the HARDENED_USERCOPY work? > In case of the latter: > Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Please let me know. Thanks! Yeah, I'll take it via my tree, just so it's all together. Thanks! -Kees
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig index 936e1803c7c7..f0eac0e2f123 100644 --- a/arch/m68k/Kconfig +++ b/arch/m68k/Kconfig @@ -4,6 +4,7 @@ config M68K default y select ARCH_32BIT_OFF_T select ARCH_HAS_BINFMT_FLAT + select ARCH_HAS_CURRENT_STACK_POINTER select ARCH_HAS_DMA_PREP_COHERENT if HAS_DMA && MMU && !COLDFIRE select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA select ARCH_HAVE_NMI_SAFE_CMPXCHG if RMW_INSNS diff --git a/arch/m68k/include/asm/current.h b/arch/m68k/include/asm/current.h index 6390ef2f7f86..0c87a7047e15 100644 --- a/arch/m68k/include/asm/current.h +++ b/arch/m68k/include/asm/current.h @@ -24,6 +24,8 @@ static inline struct task_struct *get_current(void) #define current get_current() +register unsigned long current_stack_pointer __asm__("sp"); + #endif /* CONFNIG_MMU */ #endif /* !(_M68K_CURRENT_H) */
To follow the existing per-arch conventions, add asm "sp" as "current_stack_pointer". This will let it be used in non-arch places (like HARDENED_USERCOPY). Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: linux-m68k@lists.linux-m68k.org Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/m68k/Kconfig | 1 + arch/m68k/include/asm/current.h | 2 ++ 2 files changed, 3 insertions(+)