All of lore.kernel.org
 help / color / mirror / Atom feed
From: kpark3469@gmail.com
To: kernel-hardening@lists.openwall.com
Cc: catalin.marinas@arm.com, keescook@chromium.org,
	will.deacon@arm.com, mark.rutland@arm.com, james.morse@arm.com,
	panand@redhat.com, keun-o.park@darkmatter.ae
Subject: [kernel-hardening] [PATCH v2 2/3] arm64: usercopy: Implement stack frame object validation
Date: Thu,  2 Feb 2017 16:50:23 +0400	[thread overview]
Message-ID: <1486039824-8470-2-git-send-email-kpark3469@gmail.com> (raw)
In-Reply-To: <1486039824-8470-1-git-send-email-kpark3469@gmail.com>

From: Sahara <keun-o.park@darkmatter.ae>

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: Sahara <keun-o.park@darkmatter.ae>
Reviewed-by: James Morse <james.morse@arm.com>
---
 arch/arm64/Kconfig                   |  1 +
 arch/arm64/include/asm/thread_info.h | 65 ++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1117421..8bf70b4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -97,6 +97,7 @@ config ARM64
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES if HAVE_KPROBES
+	select HAVE_ARCH_WITHIN_STACK_FRAMES
 	select IOMMU_DMA if IOMMU_SUPPORT
 	select IRQ_DOMAIN
 	select IRQ_FORCED_THREADING
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 46c3b93..4ca7a0b 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -36,6 +36,7 @@
 
 struct task_struct;
 
+#include <linux/usercopy.h>
 #include <asm/stack_pointer.h>
 #include <asm/types.h>
 
@@ -68,7 +69,71 @@ struct thread_info {
 #define thread_saved_fp(tsk)	\
 	((unsigned long)(tsk->thread.cpu_context.fp))
 
+#define get_stack_start(fp) (fp + 2 * sizeof(void *))
+
+/*
+ * Walks up the stack frames to make sure that the specified object is
+ * entirely contained by a single stack frame.
+ *
+ * Returns:
+ *	GOOD_FRAME	if within a frame
+ *	BAD_STACK	if placed across a frame boundary (or outside stack)
+ *	NOT_STACK	unable to determine (no frame pointers, etc)
+ */
+
+static inline enum stack_type arch_within_stack_frames(const void * const stack,
+					const void * const stackend,
+					const void *obj, unsigned long len)
+{
+#ifdef CONFIG_FRAME_POINTER
+	const void *callee_fp = NULL;
+	const void *caller_fp = NULL;
+
+	callee_fp = __builtin_frame_address(1);
+	if (callee_fp)
+		caller_fp = *(const void * const *)callee_fp;
+	/*
+	 * Case #1:
+	 * low ----------------------------------------------> high
+	 * [callee_fp][lr][args][local vars][caller_fp'][lr']
+	 *                ^----------------^
+	 *               allow copies only within here
+	 *
+	 * Case #2:
+	 * low ----------------------------------------------> high
+	 * [check_object_size_fp][lr][args][local vars][callee_fp][lr]
+	 *                           ^----------------^
+	 *                     dynamically allocated stack variable of
+	 *                     callee frame copies are allowed within here
+	 *
+	 * < example code snippet for Case#2 >
+	 * array_size = get_random_int() & 0x0f;
+	 * if (to_user) {
+	 *         unsigned char array[array_size];
+	 *         if (copy_to_user((void __user *)user_addr, array,
+	 *                          unconst + sizeof(array))) {
+	 */
+	while (stack <= callee_fp && callee_fp < stackend &&
+			!((unsigned long)caller_fp & 0xf)) {
+		/*
+		 * If obj + len extends past the caller frame, this
+		 * check won't pass and the next frame will be 0,
+		 * causing us to bail out and correctly report
+		 * the copy as invalid.
+		 */
+		if (!caller_fp || (obj + len <= caller_fp))
+			return (obj >= get_stack_start(callee_fp)) ?
+				GOOD_FRAME : BAD_STACK;
+		callee_fp = caller_fp;
+		caller_fp = *(const void * const *)caller_fp;
+	}
+	return BAD_STACK;
+#else
+	return NOT_STACK;
 #endif
+}
+
+#endif /* !__ASSEMBLY__ */
 
 /*
  * thread information flags:
-- 
2.7.4

  reply	other threads:[~2017-02-02 12:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-02 12:50 [kernel-hardening] [PATCH v2 1/3] usercopy: create usercopy.h and move enum stack_type kpark3469
2017-02-02 12:50 ` kpark3469 [this message]
2017-02-02 12:50   ` [kernel-hardening] [PATCH v2 3/3] lkdtm: add tests for dynamic array in local stack kpark3469
2017-02-02 16:23 ` [kernel-hardening] Re: [PATCH v2 1/3] usercopy: create usercopy.h and move enum stack_type Kees Cook

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=1486039824-8470-2-git-send-email-kpark3469@gmail.com \
    --to=kpark3469@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=keun-o.park@darkmatter.ae \
    --cc=mark.rutland@arm.com \
    --cc=panand@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.