From: Kees Cook <keescook@chromium.org>
To: Keun-O Park <kpark3469@gmail.com>
Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>,
James Morse <james.morse@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
Mark Rutland <mark.rutland@arm.com>,
keun-o.park@darkmatter.ae,
Sodagudi Prasad <psodagud@codeaurora.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/4] arm64: usercopy: implement arch_within_stack_frames
Date: Wed, 4 Apr 2018 15:55:50 -0700 [thread overview]
Message-ID: <CAGXu5jKidijcESX2vguQ=m4Fx1S7eKOKXrLTEb3Of1FN9jQbbg@mail.gmail.com> (raw)
In-Reply-To: <1519899591-29761-3-git-send-email-kpark3469@gmail.com>
On Thu, Mar 1, 2018 at 2:19 AM, <kpark3469@gmail.com> wrote:
> From: James Morse <james.morse@arm.com>
>
> This implements arch_within_stack_frames() for arm64 that should
> validate if a given object is contained by a kernel stack frame.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Reviewed-by: Sahara <keun-o.park@darkmatter.ae>
Looks good to me. Does this end up passing the LKDTM
USERCOPY_STACK_FRAME_TO and USERCOPY_STACK_FRAME_FROM tests?
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/stacktrace.c | 76 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5361287..b6c3b52 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -127,6 +127,7 @@ config ARM64
> select HAVE_SYSCALL_TRACEPOINTS
> select HAVE_KPROBES
> select HAVE_KRETPROBES
> + select HAVE_ARCH_WITHIN_STACK_FRAMES
> select IOMMU_DMA if IOMMU_SUPPORT
> select IRQ_DOMAIN
> select IRQ_FORCED_THREADING
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index fbc366d..6d37fad 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -26,6 +26,11 @@
> #include <asm/irq.h>
> #include <asm/stack_pointer.h>
>
> +#define FAKE_FRAME(frame, my_func) do { \
> + frame.fp = (unsigned long)__builtin_frame_address(0); \
> + frame.pc = (unsigned long)my_func; \
> +} while (0)
> +
> /*
> * AArch64 PCS assigns the frame pointer to x29.
> *
> @@ -94,6 +99,77 @@ void notrace walk_stackframe(struct task_struct *tsk, struct stackframe *frame,
> }
> }
>
> +struct check_frame_arg {
> + unsigned long obj_start;
> + unsigned long obj_end;
> + unsigned long frame_start;
> + int discard_frames;
> + int err;
> +};
> +
> +static int check_frame(struct stackframe *frame, void *d)
> +{
> + struct check_frame_arg *arg = d;
> + unsigned long frame_end = frame->fp;
> +
> + /* object overlaps multiple frames */
> + if (arg->obj_start < frame->fp && frame->fp < arg->obj_end) {
> + arg->err = BAD_STACK;
> + return 1;
> + }
> +
> + /*
> + * Discard frames and check object is in a frame written early
> + * enough.
> + */
> + if (arg->discard_frames)
> + arg->discard_frames--;
> + else if ((arg->frame_start <= arg->obj_start &&
> + arg->obj_start < frame_end) &&
> + (arg->frame_start < arg->obj_end && arg->obj_end <= frame_end))
> + return 1;
> +
> + /* object exists in a previous frame */
> + if (arg->obj_end < arg->frame_start) {
> + arg->err = BAD_STACK;
> + return 1;
> + }
> +
> + arg->frame_start = frame_end + 0x10;
> +
> + return 0;
> +}
> +
> +/* Check obj doesn't overlap a stack frame record */
> +int arch_within_stack_frames(const void *stack,
> + const void *stack_end,
> + const void *obj, unsigned long obj_len)
> +{
> + struct stackframe frame;
> + struct check_frame_arg arg;
> +
> + if (!IS_ENABLED(CONFIG_FRAME_POINTER))
> + return NOT_STACK;
> +
> + arg.err = GOOD_FRAME;
> + arg.obj_start = (unsigned long)obj;
> + arg.obj_end = arg.obj_start + obj_len;
> +
> + FAKE_FRAME(frame, arch_within_stack_frames);
> + arg.frame_start = frame.fp;
> +
> + /*
> + * Skip 4 non-inlined frames: <fake frame>,
> + * arch_within_stack_frames(), check_stack_object() and
> + * __check_object_size().
> + */
> + arg.discard_frames = 4;
> +
> + walk_stackframe(current, &frame, check_frame, &arg);
> +
> + return arg.err;
> +}
> +
> #ifdef CONFIG_STACKTRACE
> struct stack_trace_data {
> struct stack_trace *trace;
> --
> 2.7.4
>
--
Kees Cook
Pixel Security
next prev parent reply other threads:[~2018-04-04 22:55 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1519899591-29761-1-git-send-email-kpark3469@gmail.com>
[not found] ` <1519899591-29761-2-git-send-email-kpark3469@gmail.com>
2018-04-04 22:52 ` [PATCH 1/4] stacktrace: move arch_within_stack_frames from thread_info.h Kees Cook
[not found] ` <1519899591-29761-3-git-send-email-kpark3469@gmail.com>
2018-04-04 22:55 ` Kees Cook [this message]
2018-04-08 6:39 ` [PATCH 2/4] arm64: usercopy: implement arch_within_stack_frames Keun-O Park
[not found] ` <1519899591-29761-4-git-send-email-kpark3469@gmail.com>
[not found] ` <1519899591-29761-5-git-send-email-kpark3469@gmail.com>
[not found] ` <CAGXu5jJbPGifqTkKkqP3OJTCY=YO9+d_fRdvY-Fj_41HH4HFCw@mail.gmail.com>
2018-04-04 23:11 ` [PATCH 4/4] x86: usercopy: reimplement arch_within_stack_frames with unwinder Kees Cook
2018-04-09 5:40 ` Keun-O Park
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGXu5jKidijcESX2vguQ=m4Fx1S7eKOKXrLTEb3Of1FN9jQbbg@mail.gmail.com' \
--to=keescook@chromium.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=jpoimboe@redhat.com \
--cc=kernel-hardening@lists.openwall.com \
--cc=keun-o.park@darkmatter.ae \
--cc=kpark3469@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=psodagud@codeaurora.org \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).