[PATCH] m68k: Implement "current_stack_pointer"

[PATCH] m68k: Implement "current_stack_pointer"

[Kees Cook]

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(+)

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) */
-- 
2.30.2

Re: [PATCH] m68k: Implement "current_stack_pointer"

[Geert Uytterhoeven]

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

RE: [PATCH] m68k: Implement "current_stack_pointer"

[David Laight]

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)

Re: [PATCH] m68k: Implement "current_stack_pointer"

[Geert Uytterhoeven]

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

RE: [PATCH] m68k: Implement "current_stack_pointer"

[David Laight]

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)

Re: [PATCH] m68k: Implement "current_stack_pointer"

[Geert Uytterhoeven]

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

Re: [PATCH] m68k: Implement "current_stack_pointer"

[Kees Cook]

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


-- 
Kees Cook