[<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
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
