linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall
@ 2021-03-30 20:57 Kees Cook
  2021-03-30 20:57 ` [PATCH v8 1/6] jump_label: Provide CONFIG-driven build state defaults Kees Cook
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Kees Cook @ 2021-03-30 20:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Kees Cook, Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel


v8:
- switch to __this_cpu_*() (tglx)
- improve commit log details, comments, and masking (ingo, tglx)
v7: https://lore.kernel.org/lkml/20210319212835.3928492-1-keescook@chromium.org/
v6: https://lore.kernel.org/lkml/20210315180229.1224655-1-keescook@chromium.org/
v5: https://lore.kernel.org/lkml/20210309214301.678739-1-keescook@chromium.org/
v4: https://lore.kernel.org/lkml/20200622193146.2985288-1-keescook@chromium.org/
v3: https://lore.kernel.org/lkml/20200406231606.37619-1-keescook@chromium.org/
v2: https://lore.kernel.org/lkml/20200324203231.64324-1-keescook@chromium.org/
rfc: https://lore.kernel.org/kernel-hardening/20190329081358.30497-1-elena.reshetova@intel.com/

Hi,

This is a continuation and refactoring of Elena's earlier effort to add
kernel stack base offset randomization. In the time since the earlier
discussions, two attacks[1][2] were made public that depended on stack
determinism, so we're no longer in the position of "this is a good idea
but we have no examples of attacks". :)

Earlier discussions also devolved into debates on entropy sources, which
is mostly a red herring, given the already low entropy available due
to stack size. Regardless, entropy can be changed/improved separately
from this series as needed.

Earlier discussions also got stuck debating how much syscall overhead
was too much, but this is also a red herring since the feature itself
needs to be selectable at boot with no cost for those that don't want it:
this is solved here with static branches.

So, here is the latest improved version, made as arch-agnostic as
possible, with usage added for x86 and arm64. It also includes some small
static branch clean ups, and addresses some surprise performance issues
due to the stack canary[3].

At the very least, the first two patches can land separately (already
Acked and Reviewed), since they're kind of "separate", but introduce
macros that are used in the core stack changes.

If I can get an Ack from an arm64 maintainer, I think this could all
land via -tip to make merging easiest.

Thanks!

-Kees

[1] https://a13xp0p0v.github.io/2020/02/15/CVE-2019-18683.html
[2] https://repositorio-aberto.up.pt/bitstream/10216/125357/2/374717.pdf
[3] https://lore.kernel.org/lkml/202003281520.A9BFF461@keescook/


Kees Cook (6):
  jump_label: Provide CONFIG-driven build state defaults
  init_on_alloc: Optimize static branches
  stack: Optionally randomize kernel stack offset each syscall
  x86/entry: Enable random_kstack_offset support
  arm64: entry: Enable random_kstack_offset support
  lkdtm: Add REPORT_STACK for checking stack offsets

 .../admin-guide/kernel-parameters.txt         | 11 ++++
 Makefile                                      |  4 ++
 arch/Kconfig                                  | 23 ++++++++
 arch/arm64/Kconfig                            |  1 +
 arch/arm64/kernel/Makefile                    |  5 ++
 arch/arm64/kernel/syscall.c                   | 16 ++++++
 arch/x86/Kconfig                              |  1 +
 arch/x86/entry/common.c                       |  3 +
 arch/x86/include/asm/entry-common.h           | 16 ++++++
 drivers/misc/lkdtm/bugs.c                     | 17 ++++++
 drivers/misc/lkdtm/core.c                     |  1 +
 drivers/misc/lkdtm/lkdtm.h                    |  1 +
 include/linux/jump_label.h                    | 19 +++++++
 include/linux/mm.h                            | 10 ++--
 include/linux/randomize_kstack.h              | 55 +++++++++++++++++++
 init/main.c                                   | 23 ++++++++
 mm/page_alloc.c                               |  4 +-
 mm/slab.h                                     |  6 +-
 18 files changed, 208 insertions(+), 8 deletions(-)
 create mode 100644 include/linux/randomize_kstack.h

-- 
2.25.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v8 1/6] jump_label: Provide CONFIG-driven build state defaults
  2021-03-30 20:57 [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
@ 2021-03-30 20:57 ` Kees Cook
  2021-03-30 20:57 ` [PATCH v8 2/6] init_on_alloc: Optimize static branches Kees Cook
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2021-03-30 20:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Kees Cook, Peter Zijlstra, Elena Reshetova, x86, Andy Lutomirski,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

As shown in jump_label.h[1], choosing the initial state of static
branches changes the assembly layout. If the condition is expected to
be likely it's inline, and if unlikely it is out of line via a jump. A
few places in the kernel use (or could be using) a CONFIG to choose the
default state, which would give a small performance benefit to their
compile-time declared default. Provide the infrastructure to do this.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/jump_label.h?h=v5.11#n398

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/lkml/20200324220641.GT2452@worktop.programming.kicks-ass.net/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/jump_label.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index d92691262f51..05f5554d860f 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -382,6 +382,21 @@ struct static_key_false {
 		[0 ... (count) - 1] = STATIC_KEY_FALSE_INIT,	\
 	}
 
+#define _DEFINE_STATIC_KEY_1(name)	DEFINE_STATIC_KEY_TRUE(name)
+#define _DEFINE_STATIC_KEY_0(name)	DEFINE_STATIC_KEY_FALSE(name)
+#define DEFINE_STATIC_KEY_MAYBE(cfg, name)			\
+	__PASTE(_DEFINE_STATIC_KEY_, IS_ENABLED(cfg))(name)
+
+#define _DEFINE_STATIC_KEY_RO_1(name)	DEFINE_STATIC_KEY_TRUE_RO(name)
+#define _DEFINE_STATIC_KEY_RO_0(name)	DEFINE_STATIC_KEY_FALSE_RO(name)
+#define DEFINE_STATIC_KEY_MAYBE_RO(cfg, name)			\
+	__PASTE(_DEFINE_STATIC_KEY_RO_, IS_ENABLED(cfg))(name)
+
+#define _DECLARE_STATIC_KEY_1(name)	DECLARE_STATIC_KEY_TRUE(name)
+#define _DECLARE_STATIC_KEY_0(name)	DECLARE_STATIC_KEY_FALSE(name)
+#define DECLARE_STATIC_KEY_MAYBE(cfg, name)			\
+	__PASTE(_DECLARE_STATIC_KEY_, IS_ENABLED(cfg))(name)
+
 extern bool ____wrong_branch_error(void);
 
 #define static_key_enabled(x)							\
@@ -482,6 +497,10 @@ extern bool ____wrong_branch_error(void);
 
 #endif /* CONFIG_JUMP_LABEL */
 
+#define static_branch_maybe(config, x)					\
+	(IS_ENABLED(config) ? static_branch_likely(x)			\
+			    : static_branch_unlikely(x))
+
 /*
  * Advanced usage; refcount, branch is enabled when: count != 0
  */
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v8 2/6] init_on_alloc: Optimize static branches
  2021-03-30 20:57 [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
  2021-03-30 20:57 ` [PATCH v8 1/6] jump_label: Provide CONFIG-driven build state defaults Kees Cook
@ 2021-03-30 20:57 ` Kees Cook
  2021-03-30 20:57 ` [PATCH v8 4/6] x86/entry: Enable random_kstack_offset support Kees Cook
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2021-03-30 20:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Kees Cook, Alexander Potapenko, Vlastimil Babka, Elena Reshetova,
	x86, Andy Lutomirski, Peter Zijlstra, Catalin Marinas,
	Will Deacon, Mark Rutland, Alexander Popov, Ard Biesheuvel,
	Jann Horn, David Hildenbrand, Mike Rapoport, Andrew Morton,
	Jonathan Corbet, Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

The state of CONFIG_INIT_ON_ALLOC_DEFAULT_ON (and ...ON_FREE...) did not
change the assembly ordering of the static branches: they were always out
of line. Use the new jump_label macros to check the CONFIG settings to
default to the "expected" state, which slightly optimizes the resulting
assembly code.

Reviewed-by: Alexander Potapenko <glider@google.com>
Link: https://lore.kernel.org/lkml/CAG_fn=X0DVwqLaHJTO6Jw7TGcMSm77GKHinrd0m_6y0SzWOrFA@mail.gmail.com/
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Link: https://lore.kernel.org/lkml/5d626b9b-5355-be94-e8e2-1be47f880f30@suse.cz
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/mm.h | 10 ++++++----
 mm/page_alloc.c    |  4 ++--
 mm/slab.h          |  6 ++++--
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 77e64e3eac80..2ccd856ac0d1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2871,18 +2871,20 @@ static inline void kernel_poison_pages(struct page *page, int numpages) { }
 static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
 #endif
 
-DECLARE_STATIC_KEY_FALSE(init_on_alloc);
+DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_ALLOC_DEFAULT_ON, init_on_alloc);
 static inline bool want_init_on_alloc(gfp_t flags)
 {
-	if (static_branch_unlikely(&init_on_alloc))
+	if (static_branch_maybe(CONFIG_INIT_ON_ALLOC_DEFAULT_ON,
+				&init_on_alloc))
 		return true;
 	return flags & __GFP_ZERO;
 }
 
-DECLARE_STATIC_KEY_FALSE(init_on_free);
+DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free);
 static inline bool want_init_on_free(void)
 {
-	return static_branch_unlikely(&init_on_free);
+	return static_branch_maybe(CONFIG_INIT_ON_FREE_DEFAULT_ON,
+				   &init_on_free);
 }
 
 extern bool _debug_pagealloc_enabled_early;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3e4b29ee2b1e..267c04b8911d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -167,10 +167,10 @@ unsigned long totalcma_pages __read_mostly;
 
 int percpu_pagelist_fraction;
 gfp_t gfp_allowed_mask __read_mostly = GFP_BOOT_MASK;
-DEFINE_STATIC_KEY_FALSE(init_on_alloc);
+DEFINE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_ALLOC_DEFAULT_ON, init_on_alloc);
 EXPORT_SYMBOL(init_on_alloc);
 
-DEFINE_STATIC_KEY_FALSE(init_on_free);
+DEFINE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free);
 EXPORT_SYMBOL(init_on_free);
 
 static bool _init_on_alloc_enabled_early __read_mostly
diff --git a/mm/slab.h b/mm/slab.h
index 076582f58f68..774c7221efdc 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -601,7 +601,8 @@ static inline void cache_random_seq_destroy(struct kmem_cache *cachep) { }
 
 static inline bool slab_want_init_on_alloc(gfp_t flags, struct kmem_cache *c)
 {
-	if (static_branch_unlikely(&init_on_alloc)) {
+	if (static_branch_maybe(CONFIG_INIT_ON_ALLOC_DEFAULT_ON,
+				&init_on_alloc)) {
 		if (c->ctor)
 			return false;
 		if (c->flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON))
@@ -613,7 +614,8 @@ static inline bool slab_want_init_on_alloc(gfp_t flags, struct kmem_cache *c)
 
 static inline bool slab_want_init_on_free(struct kmem_cache *c)
 {
-	if (static_branch_unlikely(&init_on_free))
+	if (static_branch_maybe(CONFIG_INIT_ON_FREE_DEFAULT_ON,
+				&init_on_free))
 		return !(c->ctor ||
 			 (c->flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON)));
 	return false;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v8 4/6] x86/entry: Enable random_kstack_offset support
  2021-03-30 20:57 [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
  2021-03-30 20:57 ` [PATCH v8 1/6] jump_label: Provide CONFIG-driven build state defaults Kees Cook
  2021-03-30 20:57 ` [PATCH v8 2/6] init_on_alloc: Optimize static branches Kees Cook
@ 2021-03-30 20:57 ` Kees Cook
  2021-03-31  7:50   ` Thomas Gleixner
  2021-03-30 20:57 ` [PATCH v8 5/6] arm64: entry: " Kees Cook
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Kees Cook @ 2021-03-30 20:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Kees Cook, Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

Allow for a randomized stack offset on a per-syscall basis, with roughly
5-6 bits of entropy, depending on compiler and word size. Since the
method of offsetting uses macros, this cannot live in the common entry
code (the stack offset needs to be retained for the life of the syscall,
which means it needs to happen at the actual entry point).

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/Kconfig                    |  1 +
 arch/x86/entry/common.c             |  3 +++
 arch/x86/include/asm/entry-common.h | 16 ++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2792879d398e..4b4ad8ec10d2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -165,6 +165,7 @@ config X86
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD if X86_64
 	select HAVE_ARCH_USERFAULTFD_WP         if X86_64 && USERFAULTFD
 	select HAVE_ARCH_VMAP_STACK		if X86_64
+	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
 	select HAVE_ARCH_WITHIN_STACK_FRAMES
 	select HAVE_ASM_MODVERSIONS
 	select HAVE_CMPXCHG_DOUBLE
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index a2433ae8a65e..810983d7c26f 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -38,6 +38,7 @@
 #ifdef CONFIG_X86_64
 __visible noinstr void do_syscall_64(unsigned long nr, struct pt_regs *regs)
 {
+	add_random_kstack_offset();
 	nr = syscall_enter_from_user_mode(regs, nr);
 
 	instrumentation_begin();
@@ -83,6 +84,7 @@ __visible noinstr void do_int80_syscall_32(struct pt_regs *regs)
 {
 	unsigned int nr = syscall_32_enter(regs);
 
+	add_random_kstack_offset();
 	/*
 	 * Subtlety here: if ptrace pokes something larger than 2^32-1 into
 	 * orig_ax, the unsigned int return value truncates it.  This may
@@ -102,6 +104,7 @@ static noinstr bool __do_fast_syscall_32(struct pt_regs *regs)
 	unsigned int nr = syscall_32_enter(regs);
 	int res;
 
+	add_random_kstack_offset();
 	/*
 	 * This cannot use syscall_enter_from_user_mode() as it has to
 	 * fetch EBP before invoking any of the syscall entry work
diff --git a/arch/x86/include/asm/entry-common.h b/arch/x86/include/asm/entry-common.h
index 2b87b191b3b8..14ebd2196569 100644
--- a/arch/x86/include/asm/entry-common.h
+++ b/arch/x86/include/asm/entry-common.h
@@ -2,6 +2,7 @@
 #ifndef _ASM_X86_ENTRY_COMMON_H
 #define _ASM_X86_ENTRY_COMMON_H
 
+#include <linux/randomize_kstack.h>
 #include <linux/user-return-notifier.h>
 
 #include <asm/nospec-branch.h>
@@ -70,6 +71,21 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
 	 */
 	current_thread_info()->status &= ~(TS_COMPAT | TS_I386_REGS_POKED);
 #endif
+
+	/*
+	 * Ultimately, this value will get limited by KSTACK_OFFSET_MAX(),
+	 * but not enough for x86 stack utilization comfort. To keep
+	 * reasonable stack head room, reduce the maximum offset to 8 bits.
+	 *
+	 * The actual entropy will be further reduced by the compiler when
+	 * applying stack alignment constraints (see cc_stack_align4/8 in
+	 * arch/x86/Makefile), which will remove the 3 (x86_64) or 2 (ia32)
+	 * low bits from any entropy chosen here.
+	 *
+	 * Therefore, final stack offset entropy will be 5 (x86_64) or
+	 * 6 (ia32) bits.
+	 */
+	choose_random_kstack_offset(rdtsc() & 0xFF);
 }
 #define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v8 5/6] arm64: entry: Enable random_kstack_offset support
  2021-03-30 20:57 [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
                   ` (2 preceding siblings ...)
  2021-03-30 20:57 ` [PATCH v8 4/6] x86/entry: Enable random_kstack_offset support Kees Cook
@ 2021-03-30 20:57 ` Kees Cook
  2021-03-30 20:57 ` [PATCH v8 6/6] lkdtm: Add REPORT_STACK for checking stack offsets Kees Cook
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2021-03-30 20:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Kees Cook, Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

Allow for a randomized stack offset on a per-syscall basis, with roughly
5 bits of entropy. (And include AAPCS rationale AAPCS thanks to Mark
Rutland.)

In order to avoid unconditional stack canaries on syscall entry (due to
the use of alloca()), also disable stack protector to avoid triggering
needless checks and slowing down the entry path. As there is no general
way to control stack protector coverage with a function attribute[1],
this must be disabled at the compilation unit level. This isn't a problem
here, though, since stack protector was not triggered before: examining
the resulting syscall.o, there are no changes in canary coverage (none
before, none now).

[1] a working __attribute__((no_stack_protector)) has been added to GCC
and Clang but has not been released in any version yet:
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=346b302d09c1e6db56d9fe69048acb32fbb97845
https://reviews.llvm.org/rG4fbf84c1732fca596ad1d6e96015e19760eb8a9b

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm64/Kconfig          |  1 +
 arch/arm64/kernel/Makefile  |  5 +++++
 arch/arm64/kernel/syscall.c | 16 ++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1f212b47a48a..2d0e5f544429 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -146,6 +146,7 @@ config ARM64
 	select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
 	select HAVE_ARCH_PFN_VALID
 	select HAVE_ARCH_PREL32_RELOCATIONS
+	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_STACKLEAK
 	select HAVE_ARCH_THREAD_STRUCT_WHITELIST
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index ed65576ce710..6cc97730790e 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -9,6 +9,11 @@ CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
 
+# Remove stack protector to avoid triggering unneeded stack canary
+# checks due to randomize_kstack_offset.
+CFLAGS_REMOVE_syscall.o	 = -fstack-protector -fstack-protector-strong
+CFLAGS_syscall.o	+= -fno-stack-protector
+
 # Object file lists.
 obj-y			:= debug-monitors.o entry.o irq.o fpsimd.o		\
 			   entry-common.o entry-fpsimd.o process.o ptrace.o	\
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index b9cf12b271d7..263d6c1a525f 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -5,6 +5,7 @@
 #include <linux/errno.h>
 #include <linux/nospec.h>
 #include <linux/ptrace.h>
+#include <linux/randomize_kstack.h>
 #include <linux/syscalls.h>
 
 #include <asm/daifflags.h>
@@ -43,6 +44,8 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno,
 {
 	long ret;
 
+	add_random_kstack_offset();
+
 	if (scno < sc_nr) {
 		syscall_fn_t syscall_fn;
 		syscall_fn = syscall_table[array_index_nospec(scno, sc_nr)];
@@ -55,6 +58,19 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno,
 		ret = lower_32_bits(ret);
 
 	regs->regs[0] = ret;
+
+	/*
+	 * Ultimately, this value will get limited by KSTACK_OFFSET_MAX(),
+	 * but not enough for arm64 stack utilization comfort. To keep
+	 * reasonable stack head room, reduce the maximum offset to 9 bits.
+	 *
+	 * The actual entropy will be further reduced by the compiler when
+	 * applying stack alignment constraints: the AAPCS mandates a
+	 * 16-byte (i.e. 4-bit) aligned SP at function boundaries.
+	 *
+	 * The resulting 5 bits of entropy is seen in SP[8:4].
+	 */
+	choose_random_kstack_offset(get_random_int() & 0x1FF);
 }
 
 static inline bool has_syscall_work(unsigned long flags)
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v8 6/6] lkdtm: Add REPORT_STACK for checking stack offsets
  2021-03-30 20:57 [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
                   ` (3 preceding siblings ...)
  2021-03-30 20:57 ` [PATCH v8 5/6] arm64: entry: " Kees Cook
@ 2021-03-30 20:57 ` Kees Cook
       [not found] ` <20210330205750.428816-4-keescook@chromium.org>
  2021-04-01 19:17 ` [PATCH] Where we are for this patch? Roy Yang
  6 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2021-03-30 20:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Kees Cook, Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

For validating the stack offset behavior, report the offset from a given
process's first seen stack address. A quick way to measure the entropy:

for i in $(seq 1 1000); do
	echo "REPORT_STACK" >/sys/kernel/debug/provoke-crash/DIRECT
done
offsets=$(dmesg | grep 'Stack offset' | cut -d: -f3 | sort | uniq -c | sort -n | wc -l)
echo "$(uname -m) bits of stack entropy: $(echo "obase=2; $offsets" | bc | wc -L)"

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/misc/lkdtm/bugs.c  | 17 +++++++++++++++++
 drivers/misc/lkdtm/core.c  |  1 +
 drivers/misc/lkdtm/lkdtm.h |  1 +
 3 files changed, 19 insertions(+)

diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 110f5a8538e9..0e8254d0cf0b 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -134,6 +134,23 @@ noinline void lkdtm_CORRUPT_STACK_STRONG(void)
 	__lkdtm_CORRUPT_STACK((void *)&data);
 }
 
+static pid_t stack_pid;
+static unsigned long stack_addr;
+
+void lkdtm_REPORT_STACK(void)
+{
+	volatile uintptr_t magic;
+	pid_t pid = task_pid_nr(current);
+
+	if (pid != stack_pid) {
+		pr_info("Starting stack offset tracking for pid %d\n", pid);
+		stack_pid = pid;
+		stack_addr = (uintptr_t)&magic;
+	}
+
+	pr_info("Stack offset: %d\n", (int)(stack_addr - (uintptr_t)&magic));
+}
+
 void lkdtm_UNALIGNED_LOAD_STORE_WRITE(void)
 {
 	static u8 data[5] __attribute__((aligned(4))) = {1, 2, 3, 4, 5};
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index b2aff4d87c01..8024b6a5cc7f 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -110,6 +110,7 @@ static const struct crashtype crashtypes[] = {
 	CRASHTYPE(EXHAUST_STACK),
 	CRASHTYPE(CORRUPT_STACK),
 	CRASHTYPE(CORRUPT_STACK_STRONG),
+	CRASHTYPE(REPORT_STACK),
 	CRASHTYPE(CORRUPT_LIST_ADD),
 	CRASHTYPE(CORRUPT_LIST_DEL),
 	CRASHTYPE(STACK_GUARD_PAGE_LEADING),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 5ae48c64df24..99f90d3e5e9c 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -17,6 +17,7 @@ void lkdtm_LOOP(void);
 void lkdtm_EXHAUST_STACK(void);
 void lkdtm_CORRUPT_STACK(void);
 void lkdtm_CORRUPT_STACK_STRONG(void);
+void lkdtm_REPORT_STACK(void);
 void lkdtm_UNALIGNED_LOAD_STORE_WRITE(void);
 void lkdtm_SOFTLOCKUP(void);
 void lkdtm_HARDLOCKUP(void);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 4/6] x86/entry: Enable random_kstack_offset support
  2021-03-30 20:57 ` [PATCH v8 4/6] x86/entry: Enable random_kstack_offset support Kees Cook
@ 2021-03-31  7:50   ` Thomas Gleixner
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Gleixner @ 2021-03-31  7:50 UTC (permalink / raw)
  To: Kees Cook
  Cc: Kees Cook, Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

On Tue, Mar 30 2021 at 13:57, Kees Cook wrote:

> Allow for a randomized stack offset on a per-syscall basis, with roughly
> 5-6 bits of entropy, depending on compiler and word size. Since the
> method of offsetting uses macros, this cannot live in the common entry
> code (the stack offset needs to be retained for the life of the syscall,
> which means it needs to happen at the actual entry point).
>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall
       [not found] ` <20210330205750.428816-4-keescook@chromium.org>
@ 2021-03-31  7:53   ` Thomas Gleixner
  2021-03-31 21:54     ` Kees Cook
  2021-04-01  8:30   ` Will Deacon
  1 sibling, 1 reply; 20+ messages in thread
From: Thomas Gleixner @ 2021-03-31  7:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Kees Cook, Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

On Tue, Mar 30 2021 at 13:57, Kees Cook wrote:
> +/*
> + * Do not use this anywhere else in the kernel. This is used here because
> + * it provides an arch-agnostic way to grow the stack with correct
> + * alignment. Also, since this use is being explicitly masked to a max of
> + * 10 bits, stack-clash style attacks are unlikely. For more details see
> + * "VLAs" in Documentation/process/deprecated.rst
> + * The asm statement is designed to convince the compiler to keep the
> + * allocation around even after "ptr" goes out of scope.

Nit. That explanation of "ptr" might be better placed right at the
add_random...() macro.

> + */
> +void *__builtin_alloca(size_t size);
> +/*
> + * Use, at most, 10 bits of entropy. We explicitly cap this to keep the
> + * "VLA" from being unbounded (see above). 10 bits leaves enough room for
> + * per-arch offset masks to reduce entropy (by removing higher bits, since
> + * high entropy may overly constrain usable stack space), and for
> + * compiler/arch-specific stack alignment to remove the lower bits.
> + */
> +#define KSTACK_OFFSET_MAX(x)	((x) & 0x3FF)
> +
> +/*
> + * These macros must be used during syscall entry when interrupts and
> + * preempt are disabled, and after user registers have been stored to
> + * the stack.
> + */
> +#define add_random_kstack_offset() do {					\
> +	if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,	\
> +				&randomize_kstack_offset)) {		\
> +		u32 offset = __this_cpu_read(kstack_offset);		\
> +		u8 *ptr = __builtin_alloca(KSTACK_OFFSET_MAX(offset));	\
> +		asm volatile("" : "=m"(*ptr) :: "memory");		\
> +	}								\
> +} while (0)

Other than that.

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall
  2021-03-31  7:53   ` [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall Thomas Gleixner
@ 2021-03-31 21:54     ` Kees Cook
  2021-03-31 22:38       ` Thomas Gleixner
  0 siblings, 1 reply; 20+ messages in thread
From: Kees Cook @ 2021-03-31 21:54 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

On Wed, Mar 31, 2021 at 09:53:26AM +0200, Thomas Gleixner wrote:
> On Tue, Mar 30 2021 at 13:57, Kees Cook wrote:
> > +/*
> > + * Do not use this anywhere else in the kernel. This is used here because
> > + * it provides an arch-agnostic way to grow the stack with correct
> > + * alignment. Also, since this use is being explicitly masked to a max of
> > + * 10 bits, stack-clash style attacks are unlikely. For more details see
> > + * "VLAs" in Documentation/process/deprecated.rst
> > + * The asm statement is designed to convince the compiler to keep the
> > + * allocation around even after "ptr" goes out of scope.
> 
> Nit. That explanation of "ptr" might be better placed right at the
> add_random...() macro.

Ah, yes! Fixed in v9.

> Other than that.
> 
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Thank you for the reviews!

Do you want to take this via -tip (and leave off the arm64 patch until
it is acked), or would you rather it go via arm64? (I've sent v9 now...)

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall
  2021-03-31 21:54     ` Kees Cook
@ 2021-03-31 22:38       ` Thomas Gleixner
  2021-04-01  6:31         ` Kees Cook
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Gleixner @ 2021-03-31 22:38 UTC (permalink / raw)
  To: Kees Cook
  Cc: Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

On Wed, Mar 31 2021 at 14:54, Kees Cook wrote:
> On Wed, Mar 31, 2021 at 09:53:26AM +0200, Thomas Gleixner wrote:
>> On Tue, Mar 30 2021 at 13:57, Kees Cook wrote:
>> > +/*
>> > + * Do not use this anywhere else in the kernel. This is used here because
>> > + * it provides an arch-agnostic way to grow the stack with correct
>> > + * alignment. Also, since this use is being explicitly masked to a max of
>> > + * 10 bits, stack-clash style attacks are unlikely. For more details see
>> > + * "VLAs" in Documentation/process/deprecated.rst
>> > + * The asm statement is designed to convince the compiler to keep the
>> > + * allocation around even after "ptr" goes out of scope.
>> 
>> Nit. That explanation of "ptr" might be better placed right at the
>> add_random...() macro.
>
> Ah, yes! Fixed in v9.

Hmm, looking at V9 the "ptr" thing got lost ....

> +/*
> + * Do not use this anywhere else in the kernel. This is used here because
> + * it provides an arch-agnostic way to grow the stack with correct
> + * alignment. Also, since this use is being explicitly masked to a max of
> + * 10 bits, stack-clash style attacks are unlikely. For more details see
> + * "VLAs" in Documentation/process/deprecated.rst
> + */
> +void *__builtin_alloca(size_t size);
> +/*
> + * Use, at most, 10 bits of entropy. We explicitly cap this to keep the
> + * "VLA" from being unbounded (see above). 10 bits leaves enough room for
> + * per-arch offset masks to reduce entropy (by removing higher bits, since
> + * high entropy may overly constrain usable stack space), and for
> + * compiler/arch-specific stack alignment to remove the lower bits.
> + */
> +#define KSTACK_OFFSET_MAX(x)	((x) & 0x3FF)
> +
> +/*
> + * These macros must be used during syscall entry when interrupts and
> + * preempt are disabled, and after user registers have been stored to
> + * the stack.
> + */
> +#define add_random_kstack_offset() do {					\

> Do you want to take this via -tip (and leave off the arm64 patch until
> it is acked), or would you rather it go via arm64? (I've sent v9 now...)

Either way is fine.

Thanks,

        tglx


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall
  2021-03-31 22:38       ` Thomas Gleixner
@ 2021-04-01  6:31         ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2021-04-01  6:31 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Elena Reshetova, x86, Andy Lutomirski, Peter Zijlstra,
	Catalin Marinas, Will Deacon, Mark Rutland, Alexander Potapenko,
	Alexander Popov, Ard Biesheuvel, Jann Horn, Vlastimil Babka,
	David Hildenbrand, Mike Rapoport, Andrew Morton, Jonathan Corbet,
	Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

On Thu, Apr 01, 2021 at 12:38:31AM +0200, Thomas Gleixner wrote:
> On Wed, Mar 31 2021 at 14:54, Kees Cook wrote:
> > On Wed, Mar 31, 2021 at 09:53:26AM +0200, Thomas Gleixner wrote:
> >> On Tue, Mar 30 2021 at 13:57, Kees Cook wrote:
> >> > +/*
> >> > + * Do not use this anywhere else in the kernel. This is used here because
> >> > + * it provides an arch-agnostic way to grow the stack with correct
> >> > + * alignment. Also, since this use is being explicitly masked to a max of
> >> > + * 10 bits, stack-clash style attacks are unlikely. For more details see
> >> > + * "VLAs" in Documentation/process/deprecated.rst
> >> > + * The asm statement is designed to convince the compiler to keep the
> >> > + * allocation around even after "ptr" goes out of scope.
> >> 
> >> Nit. That explanation of "ptr" might be better placed right at the
> >> add_random...() macro.
> >
> > Ah, yes! Fixed in v9.
> 
> Hmm, looking at V9 the "ptr" thing got lost ....

I put the comment inline in the macro directly above the asm().

> > Do you want to take this via -tip (and leave off the arm64 patch until
> > it is acked), or would you rather it go via arm64? (I've sent v9 now...)
> 
> Either way is fine.

Since the arm64 folks have been a bit busy, can you just put this in
-tip and leave off the arm64 patch for now?

Thanks!

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall
       [not found] ` <20210330205750.428816-4-keescook@chromium.org>
  2021-03-31  7:53   ` [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall Thomas Gleixner
@ 2021-04-01  8:30   ` Will Deacon
  2021-04-01 11:15     ` David Laight
  1 sibling, 1 reply; 20+ messages in thread
From: Will Deacon @ 2021-04-01  8:30 UTC (permalink / raw)
  To: Kees Cook
  Cc: Thomas Gleixner, Elena Reshetova, x86, Andy Lutomirski,
	Peter Zijlstra, Catalin Marinas, Mark Rutland,
	Alexander Potapenko, Alexander Popov, Ard Biesheuvel, Jann Horn,
	Vlastimil Babka, David Hildenbrand, Mike Rapoport, Andrew Morton,
	Jonathan Corbet, Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

On Tue, Mar 30, 2021 at 01:57:47PM -0700, Kees Cook wrote:
> diff --git a/include/linux/randomize_kstack.h b/include/linux/randomize_kstack.h
> new file mode 100644
> index 000000000000..351520803006
> --- /dev/null
> +++ b/include/linux/randomize_kstack.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _LINUX_RANDOMIZE_KSTACK_H
> +#define _LINUX_RANDOMIZE_KSTACK_H
> +
> +#include <linux/kernel.h>
> +#include <linux/jump_label.h>
> +#include <linux/percpu-defs.h>
> +
> +DECLARE_STATIC_KEY_MAYBE(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,
> +			 randomize_kstack_offset);
> +DECLARE_PER_CPU(u32, kstack_offset);
> +
> +/*
> + * Do not use this anywhere else in the kernel. This is used here because
> + * it provides an arch-agnostic way to grow the stack with correct
> + * alignment. Also, since this use is being explicitly masked to a max of
> + * 10 bits, stack-clash style attacks are unlikely. For more details see
> + * "VLAs" in Documentation/process/deprecated.rst
> + * The asm statement is designed to convince the compiler to keep the
> + * allocation around even after "ptr" goes out of scope.
> + */
> +void *__builtin_alloca(size_t size);
> +/*
> + * Use, at most, 10 bits of entropy. We explicitly cap this to keep the
> + * "VLA" from being unbounded (see above). 10 bits leaves enough room for
> + * per-arch offset masks to reduce entropy (by removing higher bits, since
> + * high entropy may overly constrain usable stack space), and for
> + * compiler/arch-specific stack alignment to remove the lower bits.
> + */
> +#define KSTACK_OFFSET_MAX(x)	((x) & 0x3FF)
> +
> +/*
> + * These macros must be used during syscall entry when interrupts and
> + * preempt are disabled, and after user registers have been stored to
> + * the stack.
> + */
> +#define add_random_kstack_offset() do {					\
> +	if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,	\
> +				&randomize_kstack_offset)) {		\
> +		u32 offset = __this_cpu_read(kstack_offset);		\
> +		u8 *ptr = __builtin_alloca(KSTACK_OFFSET_MAX(offset));	\
> +		asm volatile("" : "=m"(*ptr) :: "memory");		\

Using the "m" constraint here is dangerous if you don't actually evaluate it
inside the asm. For example, if the compiler decides to generate an
addressing mode relative to the stack but with writeback (autodecrement), then
the stack pointer will be off by 8 bytes. Can you use "o" instead?

Will


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall
  2021-04-01  8:30   ` Will Deacon
@ 2021-04-01 11:15     ` David Laight
  2021-04-01 22:42       ` Kees Cook
  0 siblings, 1 reply; 20+ messages in thread
From: David Laight @ 2021-04-01 11:15 UTC (permalink / raw)
  To: 'Will Deacon', Kees Cook
  Cc: Thomas Gleixner, Elena Reshetova, x86, Andy Lutomirski,
	Peter Zijlstra, Catalin Marinas, Mark Rutland,
	Alexander Potapenko, Alexander Popov, Ard Biesheuvel, Jann Horn,
	Vlastimil Babka, David Hildenbrand, Mike Rapoport, Andrew Morton,
	Jonathan Corbet, Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

From: Will Deacon
> Sent: 01 April 2021 09:31
...
> > +/*
> > + * These macros must be used during syscall entry when interrupts and
> > + * preempt are disabled, and after user registers have been stored to
> > + * the stack.
> > + */
> > +#define add_random_kstack_offset() do {					\
> > +	if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,	\
> > +				&randomize_kstack_offset)) {		\
> > +		u32 offset = __this_cpu_read(kstack_offset);		\
> > +		u8 *ptr = __builtin_alloca(KSTACK_OFFSET_MAX(offset));	\
> > +		asm volatile("" : "=m"(*ptr) :: "memory");		\
> 
> Using the "m" constraint here is dangerous if you don't actually evaluate it
> inside the asm. For example, if the compiler decides to generate an
> addressing mode relative to the stack but with writeback (autodecrement), then
> the stack pointer will be off by 8 bytes. Can you use "o" instead?

Is it allowed to use such a mode?
It would have to know that the "m" was substituted exactly once.
I think there are quite a few examples with 'strange' uses of memory
asm arguments.

However, in this case, isn't it enough to ensure the address is 'saved'?
So:
	asm volatile("" : "=r"(ptr) );
should be enough.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] Where we are for this patch?
  2021-03-30 20:57 [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
                   ` (5 preceding siblings ...)
       [not found] ` <20210330205750.428816-4-keescook@chromium.org>
@ 2021-04-01 19:17 ` Roy Yang
  2021-04-01 19:48   ` Al Viro
  2021-04-01 21:46   ` [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
  6 siblings, 2 replies; 20+ messages in thread
From: Roy Yang @ 2021-04-01 19:17 UTC (permalink / raw)
  To: keescook
  Cc: akpm, alex.popov, ard.biesheuvel, catalin.marinas, corbet, david,
	elena.reshetova, glider, jannh, kernel-hardening,
	linux-arm-kernel, linux-hardening, linux-kernel, linux-mm, luto,
	mark.rutland, peterz, rdunlap, rppt, tglx, vbabka, will, x86,
	Roy Yang

Both Android and Chrome OS really want this feature; For Container-Optimized OS, we have customers
interested in the defense too.

Thank you very much.

Change-Id: I1eb1b726007aa8f9c374b934cc1c690fb4924aa3
-- 
2.31.0.208.g409f899ff0-goog



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Where we are for this patch?
  2021-04-01 19:17 ` [PATCH] Where we are for this patch? Roy Yang
@ 2021-04-01 19:48   ` Al Viro
  2021-04-01 20:13     ` Theodore Ts'o
  2021-04-01 21:46   ` [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
  1 sibling, 1 reply; 20+ messages in thread
From: Al Viro @ 2021-04-01 19:48 UTC (permalink / raw)
  To: Roy Yang
  Cc: keescook, akpm, alex.popov, ard.biesheuvel, catalin.marinas,
	corbet, david, elena.reshetova, glider, jannh, kernel-hardening,
	linux-arm-kernel, linux-hardening, linux-kernel, linux-mm, luto,
	mark.rutland, peterz, rdunlap, rppt, tglx, vbabka, will, x86

On Thu, Apr 01, 2021 at 12:17:44PM -0700, Roy Yang wrote:
> Both Android and Chrome OS really want this feature; For Container-Optimized OS, we have customers
> interested in the defense too.
> 
> Thank you very much.
> 
> Change-Id: I1eb1b726007aa8f9c374b934cc1c690fb4924aa3

	You forgot to tell what patch you are refering to.  Your
Change-Id (whatever the hell that is) doesn't help at all.  Don't
assume that keys in your internal database make sense for the
rest of the world, especially when they appear to contain a hash
of something...


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Where we are for this patch?
  2021-04-01 19:48   ` Al Viro
@ 2021-04-01 20:13     ` Theodore Ts'o
  0 siblings, 0 replies; 20+ messages in thread
From: Theodore Ts'o @ 2021-04-01 20:13 UTC (permalink / raw)
  To: Al Viro
  Cc: Roy Yang, keescook, akpm, alex.popov, ard.biesheuvel,
	catalin.marinas, corbet, david, elena.reshetova, glider, jannh,
	kernel-hardening, linux-arm-kernel, linux-hardening,
	linux-kernel, linux-mm, luto, mark.rutland, peterz, rdunlap,
	rppt, tglx, vbabka, will, x86

On Thu, Apr 01, 2021 at 07:48:30PM +0000, Al Viro wrote:
> On Thu, Apr 01, 2021 at 12:17:44PM -0700, Roy Yang wrote:
> > Both Android and Chrome OS really want this feature; For Container-Optimized OS, we have customers
> > interested in the defense too.
> > 
> > Thank you very much.
> > 
> > Change-Id: I1eb1b726007aa8f9c374b934cc1c690fb4924aa3
> 
> 	You forgot to tell what patch you are refering to.  Your
> Change-Id (whatever the hell that is) doesn't help at all.  Don't
> assume that keys in your internal database make sense for the
> rest of the world, especially when they appear to contain a hash
> of something...

The Change-Id fails to have any direct search hits at lore.kernel.org.
However, it turn up Roy's original patch, and clicking on the
message-Id in the "In-Reply-Field", it apperas Roy was replying to
this message:

https://lore.kernel.org/lkml/20210330205750.428816-1-keescook@chromium.org/

which is the head of this patch series:

Subject: [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall

That being said, it would have been better if the original subject
line had been preserved, and it's yet another example of how the
lore.kernel.org URL is infinitely better than the Change-Id.  :-)

		       		  	      - Ted


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall
  2021-04-01 19:17 ` [PATCH] Where we are for this patch? Roy Yang
  2021-04-01 19:48   ` Al Viro
@ 2021-04-01 21:46   ` Kees Cook
  1 sibling, 0 replies; 20+ messages in thread
From: Kees Cook @ 2021-04-01 21:46 UTC (permalink / raw)
  To: Roy Yang
  Cc: akpm, alex.popov, ard.biesheuvel, catalin.marinas, corbet, david,
	elena.reshetova, glider, jannh, kernel-hardening,
	linux-arm-kernel, linux-hardening, linux-kernel, linux-mm, luto,
	mark.rutland, peterz, rdunlap, rppt, tglx, vbabka, will, x86

On Thu, Apr 01, 2021 at 12:17:44PM -0700, Roy Yang wrote:
> Both Android and Chrome OS really want this feature; For Container-Optimized OS, we have customers
> interested in the defense too.

It's pretty close! There are a couple recent comments that need to be
addressed, but hopefully it can land if x86 and arm64 maintainers are
happy v10.

> Change-Id: I1eb1b726007aa8f9c374b934cc1c690fb4924aa3
> -- 
> 2.31.0.208.g409f899ff0-goog

And to let other folks know, I'm guessing this email got sent with git
send-email to try to get a valid In-Reply-To header, but I guess git
trashed the Subject and ran hooks to generate a Change-Id UUID.

I assume it's from following the "Reply instructions" at the bottom of:
https://lore.kernel.org/lkml/20210330205750.428816-1-keescook@chromium.org/
(It seems those need clarification about Subject handling.)

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall
  2021-04-01 11:15     ` David Laight
@ 2021-04-01 22:42       ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2021-04-01 22:42 UTC (permalink / raw)
  To: David Laight
  Cc: 'Will Deacon',
	Thomas Gleixner, Elena Reshetova, x86, Andy Lutomirski,
	Peter Zijlstra, Catalin Marinas, Mark Rutland,
	Alexander Potapenko, Alexander Popov, Ard Biesheuvel, Jann Horn,
	Vlastimil Babka, David Hildenbrand, Mike Rapoport, Andrew Morton,
	Jonathan Corbet, Randy Dunlap, kernel-hardening, linux-hardening,
	linux-arm-kernel, linux-mm, linux-kernel

On Thu, Apr 01, 2021 at 11:15:43AM +0000, David Laight wrote:
> From: Will Deacon
> > Sent: 01 April 2021 09:31
> ...
> > > +/*
> > > + * These macros must be used during syscall entry when interrupts and
> > > + * preempt are disabled, and after user registers have been stored to
> > > + * the stack.
> > > + */
> > > +#define add_random_kstack_offset() do {					\
> > > +	if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,	\
> > > +				&randomize_kstack_offset)) {		\
> > > +		u32 offset = __this_cpu_read(kstack_offset);		\
> > > +		u8 *ptr = __builtin_alloca(KSTACK_OFFSET_MAX(offset));	\
> > > +		asm volatile("" : "=m"(*ptr) :: "memory");		\
> > 
> > Using the "m" constraint here is dangerous if you don't actually evaluate it
> > inside the asm. For example, if the compiler decides to generate an
> > addressing mode relative to the stack but with writeback (autodecrement), then
> > the stack pointer will be off by 8 bytes. Can you use "o" instead?

I see other examples of empty asm, but it's true, none are using "=m" read
constraints. But, yes, using "o" appears to work happily.

> Is it allowed to use such a mode?
> It would have to know that the "m" was substituted exactly once.
> I think there are quite a few examples with 'strange' uses of memory
> asm arguments.
> 
> However, in this case, isn't it enough to ensure the address is 'saved'?
> So:
> 	asm volatile("" : "=r"(ptr) );
> should be enough.

It isn't, it seems.

Here's a comparison:

https://godbolt.org/z/xYGn9GfGY

So, I'll resend with "o", and with raw_cpu_*().

Thanks!

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall
  2021-04-01 20:49 Roy Yang
@ 2021-04-01 20:59 ` Theodore Ts'o
  0 siblings, 0 replies; 20+ messages in thread
From: Theodore Ts'o @ 2021-04-01 20:59 UTC (permalink / raw)
  To: Roy Yang
  Cc: Al Viro, keescook, akpm, alex.popov, ard.biesheuvel,
	catalin.marinas, corbet, david, elena.reshetova,
	Alexander Potapenko, Jann Horn, kernel-hardening,
	linux-arm-kernel, linux-hardening, linux-kernel, linux-mm, luto,
	mark.rutland, peterz, rdunlap, rppt, tglx, vbabka, will, x86

On Thu, Apr 01, 2021 at 01:49:17PM -0700, Roy Yang wrote:
> Thanks Ted, Casey and Al Viro. I am sorry for the inconvenience.
> 
> I tried to follow the instructions listed under
> https://lore.kernel.org/lkml/20210330205750.428816-1-keescook@chromium.org/
> using git-send-email
> 
> Thought that will reply to the original thread with the original
> subject . Let me know what I can do to correct this to avoid
> confusion.

It did chain to the original thread; that's how I was able to figure
out the context.  However, it looks like in your reply, you set the
subject to be:

[PATCH] Where we are for this patch?

And the problem is if someone had deleted the original e-mail chain
--- for example, optimizing the kernel stack is not one of the
subjects that I normally closely track, I had already deleted those
e-mail chains.  So all I saw (and I suspect all Al saw) was a message
with the above subject line, and no context.

If you had kept the original subject line, then those of us who mostly
focus on file system stuff would have known that it's an area which is
covered by other maintainers, and we wouldn't have deleted your query
and let someone else respond.

Cheers,

					- Ted

P.S.  I personally find the use "git-send-email" to reply to be so
much of a pain that I just use a non-google.com address to which I can
use a traditional threaded mail-reader (such as mutt) and I can send
e-mail without having to worry about the DMARC nonsense.  :-)



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall
@ 2021-04-01 20:49 Roy Yang
  2021-04-01 20:59 ` Theodore Ts'o
  0 siblings, 1 reply; 20+ messages in thread
From: Roy Yang @ 2021-04-01 20:49 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Al Viro, keescook, akpm, alex.popov, ard.biesheuvel,
	catalin.marinas, corbet, david, elena.reshetova,
	Alexander Potapenko, Jann Horn, kernel-hardening,
	linux-arm-kernel, linux-hardening, linux-kernel, linux-mm, luto,
	mark.rutland, peterz, rdunlap, rppt, tglx, vbabka, will, x86

Thanks Ted, Casey and Al Viro. I am sorry for the inconvenience.

I tried to follow the instructions listed under
https://lore.kernel.org/lkml/20210330205750.428816-1-keescook@chromium.org/
using git-send-email

Thought that will reply to the original thread with the original
subject . Let me know what I can do to correct this to avoid
confusion.


- Roy


On Thu, Apr 1, 2021 at 1:13 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Thu, Apr 01, 2021 at 07:48:30PM +0000, Al Viro wrote:
> > On Thu, Apr 01, 2021 at 12:17:44PM -0700, Roy Yang wrote:
> > > Both Android and Chrome OS really want this feature; For Container-Optimized OS, we have customers
> > > interested in the defense too.
> > >
> > > Thank you very much.
> > >
> > > Change-Id: I1eb1b726007aa8f9c374b934cc1c690fb4924aa3
> >
> >       You forgot to tell what patch you are refering to.  Your
> > Change-Id (whatever the hell that is) doesn't help at all.  Don't
> > assume that keys in your internal database make sense for the
> > rest of the world, especially when they appear to contain a hash
> > of something...
>
> The Change-Id fails to have any direct search hits at lore.kernel.org.
> However, it turn up Roy's original patch, and clicking on the
> message-Id in the "In-Reply-Field", it apperas Roy was replying to
> this message:
>
> https://lore.kernel.org/lkml/20210330205750.428816-1-keescook@chromium.org/
>
> which is the head of this patch series:
>
> Subject: [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall
>
> That being said, it would have been better if the original subject
> line had been preserved, and it's yet another example of how the
> lore.kernel.org URL is infinitely better than the Change-Id.  :-)
>
>                                               - Ted


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-04-01 22:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 20:57 [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
2021-03-30 20:57 ` [PATCH v8 1/6] jump_label: Provide CONFIG-driven build state defaults Kees Cook
2021-03-30 20:57 ` [PATCH v8 2/6] init_on_alloc: Optimize static branches Kees Cook
2021-03-30 20:57 ` [PATCH v8 4/6] x86/entry: Enable random_kstack_offset support Kees Cook
2021-03-31  7:50   ` Thomas Gleixner
2021-03-30 20:57 ` [PATCH v8 5/6] arm64: entry: " Kees Cook
2021-03-30 20:57 ` [PATCH v8 6/6] lkdtm: Add REPORT_STACK for checking stack offsets Kees Cook
     [not found] ` <20210330205750.428816-4-keescook@chromium.org>
2021-03-31  7:53   ` [PATCH v8 3/6] stack: Optionally randomize kernel stack offset each syscall Thomas Gleixner
2021-03-31 21:54     ` Kees Cook
2021-03-31 22:38       ` Thomas Gleixner
2021-04-01  6:31         ` Kees Cook
2021-04-01  8:30   ` Will Deacon
2021-04-01 11:15     ` David Laight
2021-04-01 22:42       ` Kees Cook
2021-04-01 19:17 ` [PATCH] Where we are for this patch? Roy Yang
2021-04-01 19:48   ` Al Viro
2021-04-01 20:13     ` Theodore Ts'o
2021-04-01 21:46   ` [PATCH v8 0/6] Optionally randomize kernel stack offset each syscall Kees Cook
2021-04-01 20:49 Roy Yang
2021-04-01 20:59 ` Theodore Ts'o

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).