From: Mark Rutland <mark.rutland@arm.com> To: linux-kernel@vger.kernel.org Cc: akpm@linux-foundation.org, alex.popov@linux.com, catalin.marinas@arm.com, keescook@chromium.org, linux-arm-kernel@lists.infradead.org, luto@kernel.org, mark.rutland@arm.com, will@kernel.org Subject: [PATCH 3/8] stackleak: rework stack low bound handling Date: Mon, 25 Apr 2022 12:55:58 +0100 [thread overview] Message-ID: <20220425115603.781311-4-mark.rutland@arm.com> (raw) In-Reply-To: <20220425115603.781311-1-mark.rutland@arm.com> In stackleak_task_init(), stackleak_track_stack(), and __stackleak_erase(), we open-code skipping the STACK_END_MAGIC at the bottom of the stack. Each case is implemented slightly differently, and only the __stackleak_erase() case is commented. In stackleak_task_init() and stackleak_track_stack() we unconditionally add sizeof(unsigned long) to the lowest stack address. In stackleak_task_init() we use end_of_stack() for this, and in stackleak_track_stack() we use task_stack_page(). In __stackleak_erase() we handle this by detecting if `kstack_ptr` has hit the stack end boundary, and if so, conditionally moving it above the magic. This patch adds a new stackleak_task_low_bound() helper which is used in all three cases, which unconditionally adds sizeof(unsigned long) to the lowest address on the task stack, with commentary as to why. This uses end_of_stack() as stackleak_task_init() did prior to this patch, as this is consistent with the code in kernel/fork.c which initializes the STACK_END_MAGIC value. In __stackleak_erase() we no longer need to check whether we've spilled into the STACK_END_MAGIC value, as stackleak_track_stack() ensures that `current->lowest_stack` stops immediately above this, and similarly the poison scan will stop immediately above this. For stackleak_task_init() and stackleak_track_stack() this results in no change to code generation. For __stackleak_erase() the generated assembly is slightly simpler and shorter. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Alexander Popov <alex.popov@linux.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Kees Cook <keescook@chromium.org> --- include/linux/stackleak.h | 15 ++++++++++++++- kernel/stackleak.c | 14 ++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/include/linux/stackleak.h b/include/linux/stackleak.h index ccaab2043fcd5..67430faa5c518 100644 --- a/include/linux/stackleak.h +++ b/include/linux/stackleak.h @@ -15,9 +15,22 @@ #ifdef CONFIG_GCC_PLUGIN_STACKLEAK #include <asm/stacktrace.h> +/* + * The lowest address on tsk's stack which we can plausibly erase. + */ +static __always_inline unsigned long +stackleak_task_low_bound(const struct task_struct *tsk) +{ + /* + * The lowest unsigned long on the task stack contains STACK_END_MAGIC, + * which we must not corrupt. + */ + return (unsigned long)end_of_stack(tsk) + sizeof(unsigned long); +} + static inline void stackleak_task_init(struct task_struct *t) { - t->lowest_stack = (unsigned long)end_of_stack(t) + sizeof(unsigned long); + t->lowest_stack = stackleak_task_low_bound(t); # ifdef CONFIG_STACKLEAK_METRICS t->prev_lowest_stack = t->lowest_stack; # endif diff --git a/kernel/stackleak.c b/kernel/stackleak.c index 753eab797a04d..0472956d9a2ce 100644 --- a/kernel/stackleak.c +++ b/kernel/stackleak.c @@ -72,9 +72,11 @@ late_initcall(stackleak_sysctls_init); static __always_inline void __stackleak_erase(void) { + const unsigned long task_stack_low = stackleak_task_low_bound(current); + /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */ unsigned long kstack_ptr = current->lowest_stack; - unsigned long boundary = (unsigned long)end_of_stack(current); + unsigned long boundary = task_stack_low; unsigned int poison_count = 0; const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long); @@ -92,13 +94,6 @@ static __always_inline void __stackleak_erase(void) kstack_ptr -= sizeof(unsigned long); } - /* - * One 'long int' at the bottom of the thread stack is reserved and - * should not be poisoned (see CONFIG_SCHED_STACK_END_CHECK=y). - */ - if (kstack_ptr == boundary) - kstack_ptr += sizeof(unsigned long); - #ifdef CONFIG_STACKLEAK_METRICS current->prev_lowest_stack = kstack_ptr; #endif @@ -144,8 +139,7 @@ void __used __no_caller_saved_registers noinstr stackleak_track_stack(void) /* 'lowest_stack' should be aligned on the register width boundary */ sp = ALIGN(sp, sizeof(unsigned long)); if (sp < current->lowest_stack && - sp >= (unsigned long)task_stack_page(current) + - sizeof(unsigned long)) { + sp >= stackleak_task_low_bound(current)) { current->lowest_stack = sp; } } -- 2.30.2
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com> To: linux-kernel@vger.kernel.org Cc: akpm@linux-foundation.org, alex.popov@linux.com, catalin.marinas@arm.com, keescook@chromium.org, linux-arm-kernel@lists.infradead.org, luto@kernel.org, mark.rutland@arm.com, will@kernel.org Subject: [PATCH 3/8] stackleak: rework stack low bound handling Date: Mon, 25 Apr 2022 12:55:58 +0100 [thread overview] Message-ID: <20220425115603.781311-4-mark.rutland@arm.com> (raw) In-Reply-To: <20220425115603.781311-1-mark.rutland@arm.com> In stackleak_task_init(), stackleak_track_stack(), and __stackleak_erase(), we open-code skipping the STACK_END_MAGIC at the bottom of the stack. Each case is implemented slightly differently, and only the __stackleak_erase() case is commented. In stackleak_task_init() and stackleak_track_stack() we unconditionally add sizeof(unsigned long) to the lowest stack address. In stackleak_task_init() we use end_of_stack() for this, and in stackleak_track_stack() we use task_stack_page(). In __stackleak_erase() we handle this by detecting if `kstack_ptr` has hit the stack end boundary, and if so, conditionally moving it above the magic. This patch adds a new stackleak_task_low_bound() helper which is used in all three cases, which unconditionally adds sizeof(unsigned long) to the lowest address on the task stack, with commentary as to why. This uses end_of_stack() as stackleak_task_init() did prior to this patch, as this is consistent with the code in kernel/fork.c which initializes the STACK_END_MAGIC value. In __stackleak_erase() we no longer need to check whether we've spilled into the STACK_END_MAGIC value, as stackleak_track_stack() ensures that `current->lowest_stack` stops immediately above this, and similarly the poison scan will stop immediately above this. For stackleak_task_init() and stackleak_track_stack() this results in no change to code generation. For __stackleak_erase() the generated assembly is slightly simpler and shorter. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Alexander Popov <alex.popov@linux.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Kees Cook <keescook@chromium.org> --- include/linux/stackleak.h | 15 ++++++++++++++- kernel/stackleak.c | 14 ++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/include/linux/stackleak.h b/include/linux/stackleak.h index ccaab2043fcd5..67430faa5c518 100644 --- a/include/linux/stackleak.h +++ b/include/linux/stackleak.h @@ -15,9 +15,22 @@ #ifdef CONFIG_GCC_PLUGIN_STACKLEAK #include <asm/stacktrace.h> +/* + * The lowest address on tsk's stack which we can plausibly erase. + */ +static __always_inline unsigned long +stackleak_task_low_bound(const struct task_struct *tsk) +{ + /* + * The lowest unsigned long on the task stack contains STACK_END_MAGIC, + * which we must not corrupt. + */ + return (unsigned long)end_of_stack(tsk) + sizeof(unsigned long); +} + static inline void stackleak_task_init(struct task_struct *t) { - t->lowest_stack = (unsigned long)end_of_stack(t) + sizeof(unsigned long); + t->lowest_stack = stackleak_task_low_bound(t); # ifdef CONFIG_STACKLEAK_METRICS t->prev_lowest_stack = t->lowest_stack; # endif diff --git a/kernel/stackleak.c b/kernel/stackleak.c index 753eab797a04d..0472956d9a2ce 100644 --- a/kernel/stackleak.c +++ b/kernel/stackleak.c @@ -72,9 +72,11 @@ late_initcall(stackleak_sysctls_init); static __always_inline void __stackleak_erase(void) { + const unsigned long task_stack_low = stackleak_task_low_bound(current); + /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */ unsigned long kstack_ptr = current->lowest_stack; - unsigned long boundary = (unsigned long)end_of_stack(current); + unsigned long boundary = task_stack_low; unsigned int poison_count = 0; const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long); @@ -92,13 +94,6 @@ static __always_inline void __stackleak_erase(void) kstack_ptr -= sizeof(unsigned long); } - /* - * One 'long int' at the bottom of the thread stack is reserved and - * should not be poisoned (see CONFIG_SCHED_STACK_END_CHECK=y). - */ - if (kstack_ptr == boundary) - kstack_ptr += sizeof(unsigned long); - #ifdef CONFIG_STACKLEAK_METRICS current->prev_lowest_stack = kstack_ptr; #endif @@ -144,8 +139,7 @@ void __used __no_caller_saved_registers noinstr stackleak_track_stack(void) /* 'lowest_stack' should be aligned on the register width boundary */ sp = ALIGN(sp, sizeof(unsigned long)); if (sp < current->lowest_stack && - sp >= (unsigned long)task_stack_page(current) + - sizeof(unsigned long)) { + sp >= stackleak_task_low_bound(current)) { current->lowest_stack = sp; } } -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-04-25 11:56 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-25 11:55 [PATCH 0/8] stackleak: fixes and rework Mark Rutland 2022-04-25 11:55 ` Mark Rutland 2022-04-25 11:55 ` [PATCH 1/8] arm64: stackleak: fix current_top_of_stack() Mark Rutland 2022-04-25 11:55 ` Mark Rutland 2022-04-25 11:55 ` [PATCH 2/8] stackleak: move skip_erasing() check earlier Mark Rutland 2022-04-25 11:55 ` Mark Rutland 2022-04-25 11:55 ` Mark Rutland [this message] 2022-04-25 11:55 ` [PATCH 3/8] stackleak: rework stack low bound handling Mark Rutland 2022-04-25 11:55 ` [PATCH 4/8] stackleak: clarify variable names Mark Rutland 2022-04-25 11:55 ` Mark Rutland 2022-04-25 11:56 ` [PATCH 5/8] stackleak: rework stack high bound handling Mark Rutland 2022-04-25 11:56 ` Mark Rutland 2022-04-25 11:56 ` [PATCH 6/8] stackleak: remove redundant check Mark Rutland 2022-04-25 11:56 ` Mark Rutland 2022-04-25 11:56 ` [PATCH 7/8] stackleak: add on/off stack variants Mark Rutland 2022-04-25 11:56 ` Mark Rutland 2022-04-25 11:56 ` [PATCH 8/8] arm64: entry: use stackleak_erase_on_task_stack() Mark Rutland 2022-04-25 11:56 ` Mark Rutland 2022-04-25 22:54 ` [PATCH 0/8] stackleak: fixes and rework Kees Cook 2022-04-25 22:54 ` Kees Cook 2022-04-26 10:10 ` Mark Rutland 2022-04-26 10:10 ` Mark Rutland 2022-04-26 10:37 ` Mark Rutland 2022-04-26 10:37 ` Mark Rutland 2022-04-26 11:15 ` Mark Rutland 2022-04-26 11:15 ` Mark Rutland 2022-04-26 15:51 ` Alexander Popov 2022-04-26 15:51 ` Alexander Popov 2022-04-26 16:07 ` Mark Rutland 2022-04-26 16:07 ` Mark Rutland 2022-04-26 16:01 ` Mark Rutland 2022-04-26 16:01 ` Mark Rutland 2022-04-26 18:01 ` Kees Cook 2022-04-26 18:01 ` Kees Cook 2022-04-26 17:51 ` Kees Cook 2022-04-26 17:51 ` 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=20220425115603.781311-4-mark.rutland@arm.com \ --to=mark.rutland@arm.com \ --cc=akpm@linux-foundation.org \ --cc=alex.popov@linux.com \ --cc=catalin.marinas@arm.com \ --cc=keescook@chromium.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@kernel.org \ --cc=will@kernel.org \ /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: linkBe 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.