All of lore.kernel.org
 help / color / mirror / Atom feed
From: kpark3469@gmail.com
To: kernel-hardening@lists.openwall.com
Cc: keescook@chromium.org, james.morse@arm.com,
	catalin.marinas@arm.com, will.deacon@arm.com,
	mark.rutland@arm.com, keun-o.park@darkmatter.ae
Subject: [PATCH 4/4] x86: usercopy: reimplement arch_within_stack_frames with unwinder
Date: Thu,  1 Mar 2018 14:19:51 +0400	[thread overview]
Message-ID: <1519899591-29761-5-git-send-email-kpark3469@gmail.com> (raw)
In-Reply-To: <1519899591-29761-4-git-send-email-kpark3469@gmail.com>

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

The old arch_within_stack_frames which used the frame pointer is
now reimplemented to use frame pointer unwinder apis. So the main
functionality is same as before.

Signed-off-by: Sahara <keun-o.park@darkmatter.ae>
---
 arch/x86/include/asm/unwind.h  |  5 +++
 arch/x86/kernel/stacktrace.c   | 77 +++++++++++++++++++++++++++++-------------
 arch/x86/kernel/unwind_frame.c |  4 +--
 3 files changed, 60 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 1f86e1b..6f04906f 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -87,6 +87,11 @@ void unwind_init(void);
 void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size,
 			void *orc, size_t orc_size);
 #else
+#ifdef CONFIG_UNWINDER_FRAME_POINTER
+#define FRAME_HEADER_SIZE (sizeof(long) * 2)
+size_t regs_size(struct pt_regs *regs);
+#endif
+
 static inline void unwind_init(void) {}
 static inline
 void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size,
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index f433a33..c26eb55 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -12,6 +12,37 @@
 #include <asm/unwind.h>
 
 
+static inline void *get_cur_frame(struct unwind_state *state)
+{
+	void *frame = NULL;
+
+#if defined(CONFIG_UNWINDER_ORC)
+#elif defined(CONFIG_UNWINDER_FRAME_POINTER)
+	if (state->regs)
+		frame = (void *)state->regs;
+	else
+		frame = (void *)state->bp;
+#else
+#endif
+	return frame;
+}
+
+static inline void *get_frame_end(struct unwind_state *state)
+{
+	void *frame_end = NULL;
+
+#if defined(CONFIG_UNWINDER_ORC)
+#elif defined(CONFIG_UNWINDER_FRAME_POINTER)
+	if (state->regs) {
+		frame_end = (void *)state->regs + regs_size(state->regs);
+	} else {
+		frame_end = (void *)state->bp + FRAME_HEADER_SIZE;
+	}
+#else
+#endif
+	return frame_end;
+}
+
 /*
  * Walks up the stack frames to make sure that the specified object is
  * entirely contained by a single stack frame.
@@ -25,31 +56,31 @@ int arch_within_stack_frames(const void * const stack,
 			     const void * const stackend,
 			     const void *obj, unsigned long len)
 {
-#if defined(CONFIG_FRAME_POINTER)
-	const void *frame = NULL;
-	const void *oldframe;
-
-	oldframe = __builtin_frame_address(2);
-	if (oldframe)
-		frame = __builtin_frame_address(3);
+#if defined(CONFIG_UNWINDER_FRAME_POINTER)
+	struct unwind_state state;
+	void *prev_frame_end = NULL;
 	/*
-	 * low ----------------------------------------------> high
-	 * [saved bp][saved ip][args][local vars][saved bp][saved ip]
-	 *                     ^----------------^
-	 *               allow copies only within here
+	 * Skip 3 non-inlined frames: arch_within_stack_frames(),
+	 * check_stack_object() and __check_object_size().
+	 *
 	 */
-	while (stack <= frame && frame < stackend) {
-		/*
-		 * If obj + len extends past the last 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 (obj + len <= frame)
-			return obj >= oldframe + 2 * sizeof(void *) ?
-				GOOD_FRAME : BAD_STACK;
-		oldframe = frame;
-		frame = *(const void * const *)frame;
+	unsigned int discard_frames = 3;
+
+	for (unwind_start(&state, current, NULL, NULL); !unwind_done(&state);
+	     unwind_next_frame(&state)) {
+		if (discard_frames) {
+			discard_frames--;
+		} else {
+			void *frame = get_cur_frame(&state);
+
+			if (!frame || !prev_frame_end)
+				return NOT_STACK;
+			if (obj + len <= frame)
+				return obj >= prev_frame_end ?
+						GOOD_FRAME : BAD_STACK;
+		}
+		/* save current frame end before move to next frame */
+		prev_frame_end = get_frame_end(&state);
 	}
 	return BAD_STACK;
 #else
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 3dc26f9..c8bfa5c 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -8,8 +8,6 @@
 #include <asm/stacktrace.h>
 #include <asm/unwind.h>
 
-#define FRAME_HEADER_SIZE (sizeof(long) * 2)
-
 unsigned long unwind_get_return_address(struct unwind_state *state)
 {
 	if (unwind_done(state))
@@ -69,7 +67,7 @@ static void unwind_dump(struct unwind_state *state)
 	}
 }
 
-static size_t regs_size(struct pt_regs *regs)
+size_t regs_size(struct pt_regs *regs)
 {
 	/* x86_32 regs from kernel mode are two words shorter: */
 	if (IS_ENABLED(CONFIG_X86_32) && !user_mode(regs))
-- 
2.7.4

  reply	other threads:[~2018-03-01 10:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-01 10:19 [PATCH 0/4] usercopy: reimplement arch_within_stack_frames kpark3469
2018-03-01 10:19 ` [PATCH 1/4] stacktrace: move arch_within_stack_frames from thread_info.h kpark3469
2018-03-01 10:19   ` [PATCH 2/4] arm64: usercopy: implement arch_within_stack_frames kpark3469
2018-03-01 10:19     ` [PATCH 3/4] arm64: usercopy: consider dynamic array stack variable kpark3469
2018-03-01 10:19       ` kpark3469 [this message]
2018-04-04 23:10         ` [PATCH 4/4] x86: usercopy: reimplement arch_within_stack_frames with unwinder Kees Cook
2018-04-04 23:11           ` Kees Cook
2018-04-09  5:40             ` Keun-O Park
2018-04-04 22:57       ` [PATCH 3/4] arm64: usercopy: consider dynamic array stack variable Kees Cook
2018-04-08  6:45         ` Keun-O Park
2018-04-09  5:54           ` Keun-O Park
2018-04-04 22:55     ` [PATCH 2/4] arm64: usercopy: implement arch_within_stack_frames Kees Cook
2018-04-08  6:39       ` Keun-O Park
2018-04-04 22:52   ` [PATCH 1/4] stacktrace: move arch_within_stack_frames from thread_info.h Kees Cook
2018-03-26 14:35 ` [PATCH 0/4] usercopy: reimplement arch_within_stack_frames Will Deacon
2018-03-28 10:16   ` Keun-O Park
2018-04-04 22:46     ` 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=1519899591-29761-5-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=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.