Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 16 Feb 2017 16:47:33 -0800
From: Kees Cook <keescook@...omium.org>
To: James Morse <james.morse@....com>
Cc: "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, Will Deacon <will.deacon@....com>, 
	Catalin Marinas <catalin.marinas@....com>, Mark Rutland <mark.rutland@....com>, 
	Pratyush Anand <panand@...hat.com>, keun-o.park@...kmatter.ae
Subject: Re: [PATCH v4 2/3] arm64: Add arch_within_stack_frames() for hardened usercopy

On Thu, Feb 16, 2017 at 10:29 AM, James Morse <james.morse@....com> wrote:
> Hardened usercopy tests that an object being copied to/from userspace
> doesn't overlap multiple stack frames.
>
> Add arch_within_stack_frames() to do this using arm64's stackwalker. The
> callback looks for 'fp' appearing with the range occupied by the object.
>
> (This isn't enough to trip the lkdtm tests on arm64)
>
> CC: Sahara <keun-o.park@...kmatter.ae>
> Based-on-a-patch-from: Sahara <keun-o.park@...kmatter.ae>
> Signed-off-by: James Morse <james.morse@....com>
> ---
>  arch/arm64/Kconfig                   |  1 +
>  arch/arm64/include/asm/thread_info.h |  7 ++++-
>  arch/arm64/kernel/stacktrace.c       | 54 ++++++++++++++++++++++++++++++++++--
>  3 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 111742126897..378caa9c0563 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -67,6 +67,7 @@ config ARM64
>         select HAVE_ARCH_TRACEHOOK
>         select HAVE_ARCH_TRANSPARENT_HUGEPAGE
>         select HAVE_ARM_SMCCC
> +       select HAVE_ARCH_WITHIN_STACK_FRAMES
>         select HAVE_EBPF_JIT
>         select HAVE_C_RECORDMCOUNT
>         select HAVE_CC_STACKPROTECTOR
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 46c3b93cf865..3540c46027fc 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -68,7 +68,12 @@ struct thread_info {
>  #define thread_saved_fp(tsk)   \
>         ((unsigned long)(tsk->thread.cpu_context.fp))
>
> -#endif
> +
> +extern enum stack_type arch_within_stack_frames(const void * const stack,
> +                                               const void * const stackend,
> +                                               const void *obj,
> +                                               unsigned long len);

The caller of arch_within_stack_frames expects this to be inlined...
could that be changed and then move the special stack check from the
third patch into check_stack_object() directly?

Regardless, I'm fine with reusing the existing walker. I just want to
avoid special cases in the uaccess code (so we can consolidate it in
the future).

-Kees

-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists

Your e-mail address:

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.