kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs
@ 2020-02-25  5:13 Kees Cook
  2020-02-25  5:13 ` [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC Kees Cook
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Kees Cook @ 2020-02-25  5:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Kees Cook, Hector Marco-Gisbert, Jason Gunthorpe,
	Catalin Marinas, Russell King, Will Deacon, Jann Horn, x86,
	linux-arm-kernel, kernel-hardening, linux-kernel

Hi,

This is a refresh of my earlier attempt to fix READ_IMPLIES_EXEC. I
think it incorporates the feedback from v2 (there are no code changes
between v3 and v4; I've just added Reviews/Acks and directed at Boris,
as it seems Ingo is busy). The selftest from v3 has been remove from v4,
as I will land it separately via Shuah's selftest tree.

This series is for x86, arm, and arm64; I'd like it to go via -tip,
though, just to keep this change together with the selftest. To
that end, I'd like to collect Acks from the respective architecture
maintainers. (Note that most other architectures don't suffer from this
problem. e.g. powerpc's behavior appears to already be correct. MIPS may
need adjusting but the history of CPU features and toolchain behavior
is very unclear to me.)

Repeating the commit log from later in the series:


The READ_IMPLIES_EXEC work-around was designed for old toolchains that
lacked the ELF PT_GNU_STACK marking under the assumption that toolchains
that couldn't specify executable permission flags for the stack may not
know how to do it correctly for any memory region.

This logic is sensible for having ancient binaries coexist in a system
with possibly NX memory, but was implemented in a way that equated having
a PT_GNU_STACK marked executable as being as "broken" as lacking the
PT_GNU_STACK marking entirely. Things like unmarked assembly and stack
trampolines may cause PT_GNU_STACK to need an executable bit, but they
do not imply all mappings must be executable.

This confusion has led to situations where modern programs with explicitly
marked executable stack are forced into the READ_IMPLIES_EXEC state when
no such thing is needed. (And leads to unexpected failures when mmap()ing
regions of device driver memory that wish to disallow VM_EXEC[1].)

In looking for other reasons for the READ_IMPLIES_EXEC behavior, Jann
Horn noted that glibc thread stacks have always been marked RWX (until
2003 when they started tracking the PT_GNU_STACK flag instead[2]). And
musl doesn't support executable stacks at all[3]. As such, no breakage
for multithreaded applications is expected from this change.

[1] https://lkml.kernel.org/r/20190418055759.GA3155@mellanox.com
[2] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=54ee14b3882
[3] https://lkml.kernel.org/r/20190423192534.GN23599@brightrain.aerifal.cx


-Kees


v4:
 - split selftest into separate series to go via Shuah's tree
 - add Reviews/Acks
v3: https://lore.kernel.org/lkml/20200210193049.64362-1-keescook@chromium.org
v2: https://lore.kernel.org/lkml/20190424203408.GA11386@beast/
v1: https://lore.kernel.org/lkml/20190423181210.GA2443@beast/

Kees Cook (6):
  x86/elf: Add table to document READ_IMPLIES_EXEC
  x86/elf: Split READ_IMPLIES_EXEC from executable GNU_STACK
  x86/elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces
  arm32/64, elf: Add tables to document READ_IMPLIES_EXEC
  arm32/64, elf: Split READ_IMPLIES_EXEC from executable GNU_STACK
  arm64, elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address
    spaces

 arch/arm/kernel/elf.c        | 27 +++++++++++++++++++++++----
 arch/arm64/include/asm/elf.h | 23 ++++++++++++++++++++++-
 arch/x86/include/asm/elf.h   | 22 +++++++++++++++++++++-
 fs/compat_binfmt_elf.c       |  5 +++++
 4 files changed, 71 insertions(+), 6 deletions(-)

-- 
2.20.1


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

* [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC
  2020-02-25  5:13 [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Kees Cook
@ 2020-02-25  5:13 ` Kees Cook
  2020-03-11 19:44   ` Borislav Petkov
  2020-02-25  5:13 ` [PATCH v4 2/6] x86/elf: Split READ_IMPLIES_EXEC from executable GNU_STACK Kees Cook
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2020-02-25  5:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Kees Cook, Jason Gunthorpe, Hector Marco-Gisbert,
	Jason Gunthorpe, Catalin Marinas, Russell King, Will Deacon,
	Jann Horn, x86, linux-arm-kernel, kernel-hardening, linux-kernel

Add a table to document the current behavior of READ_IMPLIES_EXEC in
preparation for changing the behavior.

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
---
 arch/x86/include/asm/elf.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 69c0f892e310..733f69c2b053 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -281,6 +281,25 @@ extern u32 elf_hwcap2;
 /*
  * An executable for which elf_read_implies_exec() returns TRUE will
  * have the READ_IMPLIES_EXEC personality flag set automatically.
+ *
+ * The decision process for determining the results are:
+ *
+ *              CPU: | lacks NX*  | has NX, ia32     | has NX, x86_64 |
+ * ELF:              |            |                  |                |
+ * -------------------------------|------------------|----------------|
+ * missing GNU_STACK | exec-all   | exec-all         | exec-all       |
+ * GNU_STACK == RWX  | exec-all   | exec-all         | exec-all       |
+ * GNU_STACK == RW   | exec-none  | exec-none        | exec-none      |
+ *
+ *  exec-all  : all PROT_READ user mappings are executable, except when
+ *              backed by files on a noexec-filesystem.
+ *  exec-none : only PROT_EXEC user mappings are executable.
+ *
+ *  *this column has no architectural effect: NX markings are ignored by
+ *   hardware, but may have behavioral effects when "wants X" collides with
+ *   "cannot be X" constraints in memory permission flags, as in
+ *   https://lkml.kernel.org/r/20190418055759.GA3155@mellanox.com
+ *
  */
 #define elf_read_implies_exec(ex, executable_stack)	\
 	(executable_stack != EXSTACK_DISABLE_X)
-- 
2.20.1


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

* [PATCH v4 2/6] x86/elf: Split READ_IMPLIES_EXEC from executable GNU_STACK
  2020-02-25  5:13 [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Kees Cook
  2020-02-25  5:13 ` [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC Kees Cook
@ 2020-02-25  5:13 ` Kees Cook
  2020-02-25  5:13 ` [PATCH v4 3/6] x86/elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces Kees Cook
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-02-25  5:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Kees Cook, Hector Marco-Gisbert, Jason Gunthorpe,
	Jason Gunthorpe, Catalin Marinas, Russell King, Will Deacon,
	Jann Horn, x86, linux-arm-kernel, kernel-hardening, linux-kernel

The READ_IMPLIES_EXEC work-around was designed for old toolchains that
lacked the ELF PT_GNU_STACK marking under the assumption that toolchains
that couldn't specify executable permission flags for the stack may not
know how to do it correctly for any memory region.

This logic is sensible for having ancient binaries coexist in a system
with possibly NX memory, but was implemented in a way that equated having
a PT_GNU_STACK marked executable as being as "broken" as lacking the
PT_GNU_STACK marking entirely. Things like unmarked assembly and stack
trampolines may cause PT_GNU_STACK to need an executable bit, but they
do not imply all mappings must be executable.

This confusion has led to situations where modern programs with explicitly
marked executable stack are forced into the READ_IMPLIES_EXEC state when
no such thing is needed. (And leads to unexpected failures when mmap()ing
regions of device driver memory that wish to disallow VM_EXEC[1].)

In looking for other reasons for the READ_IMPLIES_EXEC behavior, Jann
Horn noted that glibc thread stacks have always been marked RWX (until
2003 when they started tracking the PT_GNU_STACK flag instead[2]). And
musl doesn't support executable stacks at all[3]. As such, no breakage
for multithreaded applications is expected from this change.

[1] https://lkml.kernel.org/r/20190418055759.GA3155@mellanox.com
[2] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=54ee14b3882
[3] https://lkml.kernel.org/r/20190423192534.GN23599@brightrain.aerifal.cx

Suggested-by: Hector Marco-Gisbert <hecmargi@upv.es>
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
---
 arch/x86/include/asm/elf.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 733f69c2b053..a7035065377c 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -288,12 +288,13 @@ extern u32 elf_hwcap2;
  * ELF:              |            |                  |                |
  * -------------------------------|------------------|----------------|
  * missing GNU_STACK | exec-all   | exec-all         | exec-all       |
- * GNU_STACK == RWX  | exec-all   | exec-all         | exec-all       |
+ * GNU_STACK == RWX  | exec-stack | exec-stack       | exec-stack     |
  * GNU_STACK == RW   | exec-none  | exec-none        | exec-none      |
  *
  *  exec-all  : all PROT_READ user mappings are executable, except when
  *              backed by files on a noexec-filesystem.
  *  exec-none : only PROT_EXEC user mappings are executable.
+ *  exec-stack: only the stack and PROT_EXEC user mappings are executable.
  *
  *  *this column has no architectural effect: NX markings are ignored by
  *   hardware, but may have behavioral effects when "wants X" collides with
@@ -302,7 +303,7 @@ extern u32 elf_hwcap2;
  *
  */
 #define elf_read_implies_exec(ex, executable_stack)	\
-	(executable_stack != EXSTACK_DISABLE_X)
+	(executable_stack == EXSTACK_DEFAULT)
 
 struct task_struct;
 
-- 
2.20.1


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

* [PATCH v4 3/6] x86/elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces
  2020-02-25  5:13 [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Kees Cook
  2020-02-25  5:13 ` [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC Kees Cook
  2020-02-25  5:13 ` [PATCH v4 2/6] x86/elf: Split READ_IMPLIES_EXEC from executable GNU_STACK Kees Cook
@ 2020-02-25  5:13 ` Kees Cook
  2020-02-25  5:13 ` [PATCH v4 4/6] arm32/64, elf: Add tables to document READ_IMPLIES_EXEC Kees Cook
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-02-25  5:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Kees Cook, Hector Marco-Gisbert, Jason Gunthorpe,
	Jason Gunthorpe, Catalin Marinas, Russell King, Will Deacon,
	Jann Horn, x86, linux-arm-kernel, kernel-hardening, linux-kernel

With modern x86 64-bit environments, there should never be a need for
automatic READ_IMPLIES_EXEC, as the architecture is intended to always
be execute-bit aware (as in, the default memory protection should be NX
unless a region explicitly requests to be executable).

There were very old x86_64 systems that lacked the NX bit, but for those,
the NX bit is, obviously, unenforceable, so these changes should have
no impact on them.

Suggested-by: Hector Marco-Gisbert <hecmargi@upv.es>
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
---
 arch/x86/include/asm/elf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index a7035065377c..c9b7be0bcad3 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -287,7 +287,7 @@ extern u32 elf_hwcap2;
  *              CPU: | lacks NX*  | has NX, ia32     | has NX, x86_64 |
  * ELF:              |            |                  |                |
  * -------------------------------|------------------|----------------|
- * missing GNU_STACK | exec-all   | exec-all         | exec-all       |
+ * missing GNU_STACK | exec-all   | exec-all         | exec-none      |
  * GNU_STACK == RWX  | exec-stack | exec-stack       | exec-stack     |
  * GNU_STACK == RW   | exec-none  | exec-none        | exec-none      |
  *
@@ -303,7 +303,7 @@ extern u32 elf_hwcap2;
  *
  */
 #define elf_read_implies_exec(ex, executable_stack)	\
-	(executable_stack == EXSTACK_DEFAULT)
+	(mmap_is_ia32() && executable_stack == EXSTACK_DEFAULT)
 
 struct task_struct;
 
-- 
2.20.1


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

* [PATCH v4 4/6] arm32/64, elf: Add tables to document READ_IMPLIES_EXEC
  2020-02-25  5:13 [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Kees Cook
                   ` (2 preceding siblings ...)
  2020-02-25  5:13 ` [PATCH v4 3/6] x86/elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces Kees Cook
@ 2020-02-25  5:13 ` Kees Cook
  2020-02-25  5:13 ` [PATCH v4 5/6] arm32/64, elf: Split READ_IMPLIES_EXEC from executable GNU_STACK Kees Cook
  2020-02-25  5:13 ` [PATCH v4 6/6] arm64, elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces Kees Cook
  5 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-02-25  5:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Kees Cook, Jason Gunthorpe, Catalin Marinas,
	Hector Marco-Gisbert, Jason Gunthorpe, Russell King, Will Deacon,
	Jann Horn, x86, linux-arm-kernel, kernel-hardening, linux-kernel

Add tables to document the current behavior of READ_IMPLIES_EXEC in
preparation for changing the behavior for both arm64 and arm.

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/elf.c        | 24 +++++++++++++++++++++---
 arch/arm64/include/asm/elf.h | 20 ++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/elf.c b/arch/arm/kernel/elf.c
index 182422981386..2f69cf978fe3 100644
--- a/arch/arm/kernel/elf.c
+++ b/arch/arm/kernel/elf.c
@@ -78,9 +78,27 @@ void elf_set_personality(const struct elf32_hdr *x)
 EXPORT_SYMBOL(elf_set_personality);
 
 /*
- * Set READ_IMPLIES_EXEC if:
- *  - the binary requires an executable stack
- *  - we're running on a CPU which doesn't support NX.
+ * An executable for which elf_read_implies_exec() returns TRUE will
+ * have the READ_IMPLIES_EXEC personality flag set automatically.
+ *
+ * The decision process for determining the results are:
+ *
+ *              CPU: | lacks NX*  | has NX     |
+ * ELF:              |            |            |
+ * -------------------------------|------------|
+ * missing GNU_STACK | exec-all   | exec-all   |
+ * GNU_STACK == RWX  | exec-all   | exec-all   |
+ * GNU_STACK == RW   | exec-all   | exec-none  |
+ *
+ *  exec-all  : all PROT_READ user mappings are executable, except when
+ *              backed by files on a noexec-filesystem.
+ *  exec-none : only PROT_EXEC user mappings are executable.
+ *
+ *  *this column has no architectural effect: NX markings are ignored by
+ *   hardware, but may have behavioral effects when "wants X" collides with
+ *   "cannot be X" constraints in memory permission flags, as in
+ *   https://lkml.kernel.org/r/20190418055759.GA3155@mellanox.com
+ *
  */
 int arm_elf_read_implies_exec(int executable_stack)
 {
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index b618017205a3..7fc779e3f1ec 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -96,6 +96,26 @@
  */
 #define elf_check_arch(x)		((x)->e_machine == EM_AARCH64)
 
+/*
+ * An executable for which elf_read_implies_exec() returns TRUE will
+ * have the READ_IMPLIES_EXEC personality flag set automatically.
+ *
+ * The decision process for determining the results are:
+ *
+ *             CPU*: | arm32      | arm64      |
+ * ELF:              |            |            |
+ * -------------------------------|------------|
+ * missing GNU_STACK | exec-all   | exec-all   |
+ * GNU_STACK == RWX  | exec-all   | exec-all   |
+ * GNU_STACK == RW   | exec-none  | exec-none  |
+ *
+ *  exec-all  : all PROT_READ user mappings are executable, except when
+ *              backed by files on a noexec-filesystem.
+ *  exec-none : only PROT_EXEC user mappings are executable.
+ *
+ *  *all arm64 CPUs support NX, so there is no "lacks NX" column.
+ *
+ */
 #define elf_read_implies_exec(ex,stk)	(stk != EXSTACK_DISABLE_X)
 
 #define CORE_DUMP_USE_REGSET
-- 
2.20.1


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

* [PATCH v4 5/6] arm32/64, elf: Split READ_IMPLIES_EXEC from executable GNU_STACK
  2020-02-25  5:13 [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Kees Cook
                   ` (3 preceding siblings ...)
  2020-02-25  5:13 ` [PATCH v4 4/6] arm32/64, elf: Add tables to document READ_IMPLIES_EXEC Kees Cook
@ 2020-02-25  5:13 ` Kees Cook
  2020-02-25  5:13 ` [PATCH v4 6/6] arm64, elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces Kees Cook
  5 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-02-25  5:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Kees Cook, Hector Marco-Gisbert, Jason Gunthorpe,
	Catalin Marinas, Jason Gunthorpe, Russell King, Will Deacon,
	Jann Horn, x86, linux-arm-kernel, kernel-hardening, linux-kernel

The READ_IMPLIES_EXEC work-around was designed for old toolchains that
lacked the ELF PT_GNU_STACK marking under the assumption that toolchains
that couldn't specify executable permission flags for the stack may not
know how to do it correctly for any memory region.

This logic is sensible for having ancient binaries coexist in a system
with possibly NX memory, but was implemented in a way that equated having
a PT_GNU_STACK marked executable as being as "broken" as lacking the
PT_GNU_STACK marking entirely. Things like unmarked assembly and stack
trampolines may cause PT_GNU_STACK to need an executable bit, but they
do not imply all mappings must be executable.

This confusion has led to situations where modern programs with explicitly
marked executable stack are forced into the READ_IMPLIES_EXEC state when
no such thing is needed. (And leads to unexpected failures when mmap()ing
regions of device driver memory that wish to disallow VM_EXEC[1].)

In looking for other reasons for the READ_IMPLIES_EXEC behavior, Jann
Horn noted that glibc thread stacks have always been marked RWX (until
2003 when they started tracking the PT_GNU_STACK flag instead[2]). And
musl doesn't support executable stacks at all[3]. As such, no breakage
for multithreaded applications is expected from this change.

This changes arm32 and arm64 compat together, to keep behavior the same.

[1] https://lkml.kernel.org/r/20190418055759.GA3155@mellanox.com
[2] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=54ee14b3882
[3] https://lkml.kernel.org/r/20190423192534.GN23599@brightrain.aerifal.cx

Suggested-by: Hector Marco-Gisbert <hecmargi@upv.es>
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/elf.c        | 5 +++--
 arch/arm64/include/asm/elf.h | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/elf.c b/arch/arm/kernel/elf.c
index 2f69cf978fe3..6965a673a141 100644
--- a/arch/arm/kernel/elf.c
+++ b/arch/arm/kernel/elf.c
@@ -87,12 +87,13 @@ EXPORT_SYMBOL(elf_set_personality);
  * ELF:              |            |            |
  * -------------------------------|------------|
  * missing GNU_STACK | exec-all   | exec-all   |
- * GNU_STACK == RWX  | exec-all   | exec-all   |
+ * GNU_STACK == RWX  | exec-all   | exec-stack |
  * GNU_STACK == RW   | exec-all   | exec-none  |
  *
  *  exec-all  : all PROT_READ user mappings are executable, except when
  *              backed by files on a noexec-filesystem.
  *  exec-none : only PROT_EXEC user mappings are executable.
+ *  exec-stack: only the stack and PROT_EXEC user mappings are executable.
  *
  *  *this column has no architectural effect: NX markings are ignored by
  *   hardware, but may have behavioral effects when "wants X" collides with
@@ -102,7 +103,7 @@ EXPORT_SYMBOL(elf_set_personality);
  */
 int arm_elf_read_implies_exec(int executable_stack)
 {
-	if (executable_stack != EXSTACK_DISABLE_X)
+	if (executable_stack == EXSTACK_DEFAULT)
 		return 1;
 	if (cpu_architecture() < CPU_ARCH_ARMv6)
 		return 1;
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 7fc779e3f1ec..03ada29984a7 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -106,17 +106,18 @@
  * ELF:              |            |            |
  * -------------------------------|------------|
  * missing GNU_STACK | exec-all   | exec-all   |
- * GNU_STACK == RWX  | exec-all   | exec-all   |
+ * GNU_STACK == RWX  | exec-stack | exec-stack |
  * GNU_STACK == RW   | exec-none  | exec-none  |
  *
  *  exec-all  : all PROT_READ user mappings are executable, except when
  *              backed by files on a noexec-filesystem.
  *  exec-none : only PROT_EXEC user mappings are executable.
+ *  exec-stack: only the stack and PROT_EXEC user mappings are executable.
  *
  *  *all arm64 CPUs support NX, so there is no "lacks NX" column.
  *
  */
-#define elf_read_implies_exec(ex,stk)	(stk != EXSTACK_DISABLE_X)
+#define elf_read_implies_exec(ex,stk)	(stk == EXSTACK_DEFAULT)
 
 #define CORE_DUMP_USE_REGSET
 #define ELF_EXEC_PAGESIZE	PAGE_SIZE
-- 
2.20.1


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

* [PATCH v4 6/6] arm64, elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces
  2020-02-25  5:13 [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Kees Cook
                   ` (4 preceding siblings ...)
  2020-02-25  5:13 ` [PATCH v4 5/6] arm32/64, elf: Split READ_IMPLIES_EXEC from executable GNU_STACK Kees Cook
@ 2020-02-25  5:13 ` Kees Cook
  5 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-02-25  5:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Kees Cook, Hector Marco-Gisbert, Jason Gunthorpe,
	Catalin Marinas, Jason Gunthorpe, Russell King, Will Deacon,
	Jann Horn, x86, linux-arm-kernel, kernel-hardening, linux-kernel

With arm64 64-bit environments, there should never be a need for automatic
READ_IMPLIES_EXEC, as the architecture has always been execute-bit aware
(as in, the default memory protection should be NX unless a region
explicitly requests to be executable).

Suggested-by: Hector Marco-Gisbert <hecmargi@upv.es>
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/elf.h | 4 ++--
 fs/compat_binfmt_elf.c       | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 03ada29984a7..ea9221ed68a1 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -105,7 +105,7 @@
  *             CPU*: | arm32      | arm64      |
  * ELF:              |            |            |
  * -------------------------------|------------|
- * missing GNU_STACK | exec-all   | exec-all   |
+ * missing GNU_STACK | exec-all   | exec-none  |
  * GNU_STACK == RWX  | exec-stack | exec-stack |
  * GNU_STACK == RW   | exec-none  | exec-none  |
  *
@@ -117,7 +117,7 @@
  *  *all arm64 CPUs support NX, so there is no "lacks NX" column.
  *
  */
-#define elf_read_implies_exec(ex,stk)	(stk == EXSTACK_DEFAULT)
+#define compat_elf_read_implies_exec(ex, stk)	(stk == EXSTACK_DEFAULT)
 
 #define CORE_DUMP_USE_REGSET
 #define ELF_EXEC_PAGESIZE	PAGE_SIZE
diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
index aaad4ca1217e..3068d57436b3 100644
--- a/fs/compat_binfmt_elf.c
+++ b/fs/compat_binfmt_elf.c
@@ -113,6 +113,11 @@
 #define	arch_setup_additional_pages compat_arch_setup_additional_pages
 #endif
 
+#ifdef	compat_elf_read_implies_exec
+#undef	elf_read_implies_exec
+#define	elf_read_implies_exec compat_elf_read_implies_exec
+#endif
+
 /*
  * Rename a few of the symbols that binfmt_elf.c will define.
  * These are all local so the names don't really matter, but it
-- 
2.20.1


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

* Re: [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC
  2020-02-25  5:13 ` [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC Kees Cook
@ 2020-03-11 19:44   ` Borislav Petkov
  2020-03-13  0:06     ` Kees Cook
  0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2020-03-11 19:44 UTC (permalink / raw)
  To: Kees Cook
  Cc: Jason Gunthorpe, Hector Marco-Gisbert, Jason Gunthorpe,
	Catalin Marinas, Russell King, Will Deacon, Jann Horn, x86,
	linux-arm-kernel, kernel-hardening, linux-kernel

Ozenn Mon, Feb 24, 2020 at 09:13:02PM -0800, Kees Cook wrote:
> Add a table to document the current behavior of READ_IMPLIES_EXEC in
> preparation for changing the behavior.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
>  arch/x86/include/asm/elf.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> index 69c0f892e310..733f69c2b053 100644
> --- a/arch/x86/include/asm/elf.h
> +++ b/arch/x86/include/asm/elf.h
> @@ -281,6 +281,25 @@ extern u32 elf_hwcap2;
>  /*
>   * An executable for which elf_read_implies_exec() returns TRUE will
>   * have the READ_IMPLIES_EXEC personality flag set automatically.
> + *
> + * The decision process for determining the results are:
> + *
> + *              CPU: | lacks NX*  | has NX, ia32     | has NX, x86_64 |
> + * ELF:              |            |                  |                |
> + * -------------------------------|------------------|----------------|
> + * missing GNU_STACK | exec-all   | exec-all         | exec-all       |
> + * GNU_STACK == RWX  | exec-all   | exec-all         | exec-all       |
> + * GNU_STACK == RW   | exec-none  | exec-none        | exec-none      |

In all those tables, you wanna do:

s/GNU_STACK/PT_GNU_STACK/g

so that it is clear what this define is.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC
  2020-03-11 19:44   ` Borislav Petkov
@ 2020-03-13  0:06     ` Kees Cook
  0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-03-13  0:06 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Jason Gunthorpe, Hector Marco-Gisbert, Jason Gunthorpe,
	Catalin Marinas, Russell King, Will Deacon, Jann Horn, x86,
	linux-arm-kernel, kernel-hardening, linux-kernel

On Wed, Mar 11, 2020 at 08:44:46PM +0100, Borislav Petkov wrote:
> Ozenn Mon, Feb 24, 2020 at 09:13:02PM -0800, Kees Cook wrote:
> > Add a table to document the current behavior of READ_IMPLIES_EXEC in
> > preparation for changing the behavior.
> > 
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
> > ---
> >  arch/x86/include/asm/elf.h | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> > index 69c0f892e310..733f69c2b053 100644
> > --- a/arch/x86/include/asm/elf.h
> > +++ b/arch/x86/include/asm/elf.h
> > @@ -281,6 +281,25 @@ extern u32 elf_hwcap2;
> >  /*
> >   * An executable for which elf_read_implies_exec() returns TRUE will
> >   * have the READ_IMPLIES_EXEC personality flag set automatically.
> > + *
> > + * The decision process for determining the results are:
> > + *
> > + *              CPU: | lacks NX*  | has NX, ia32     | has NX, x86_64 |
> > + * ELF:              |            |                  |                |
> > + * -------------------------------|------------------|----------------|
> > + * missing GNU_STACK | exec-all   | exec-all         | exec-all       |
> > + * GNU_STACK == RWX  | exec-all   | exec-all         | exec-all       |
> > + * GNU_STACK == RW   | exec-none  | exec-none        | exec-none      |
> 
> In all those tables, you wanna do:
> 
> s/GNU_STACK/PT_GNU_STACK/g
> 
> so that it is clear what this define is.

Fair enough. :) I think I was trying to save 3 characters from earlier
tables that were wider. I'll send a v5.

Thanks!

-Kees

-- 
Kees Cook

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

end of thread, other threads:[~2020-03-13  0:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25  5:13 [PATCH v4 0/6] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Kees Cook
2020-02-25  5:13 ` [PATCH v4 1/6] x86/elf: Add table to document READ_IMPLIES_EXEC Kees Cook
2020-03-11 19:44   ` Borislav Petkov
2020-03-13  0:06     ` Kees Cook
2020-02-25  5:13 ` [PATCH v4 2/6] x86/elf: Split READ_IMPLIES_EXEC from executable GNU_STACK Kees Cook
2020-02-25  5:13 ` [PATCH v4 3/6] x86/elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces Kees Cook
2020-02-25  5:13 ` [PATCH v4 4/6] arm32/64, elf: Add tables to document READ_IMPLIES_EXEC Kees Cook
2020-02-25  5:13 ` [PATCH v4 5/6] arm32/64, elf: Split READ_IMPLIES_EXEC from executable GNU_STACK Kees Cook
2020-02-25  5:13 ` [PATCH v4 6/6] arm64, elf: Disable automatic READ_IMPLIES_EXEC for 64-bit address spaces Kees Cook

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