Thread (11 messages) flat view 11 messages, 5 authors, 2021-02-08

Re: [PATCH] arm64: bti: Set PROT_BTI on all BTI executables mapped by the kernel

From: Dave Martin <Dave.Martin@arm.com>
Date: 2021-02-08 14:54:53

On Fri, Feb 05, 2021 at 05:38:37PM +0000, Mark Brown via Libc-alpha wrote:
quoted hunk ↗ jump to hunk
Currently for dynamically linked executables the kernel only enables
PROT_BTI for the interpreter, the interpreter is responsible for
enabling it for everything else including the main executable.
Unfortunately this interacts poorly with systemd's
MemoryDenyWriteExecute feature which uses a seccomp filter to prevent
setting PROT_EXEC on already mapped memory via mprotect(), it lacks the
context to detect that PROT_EXEC is already set and so refuses to allow
the mprotect() on the main executable which the kernel has already
mapped.

Since we don't want to force users to choose between having MDWX and BTI
as these are othogonal features have the kernel enable PROT_BTI for all
the ELF objects it loads, not just the dynamic linker.  This means that
if there is a problem with BTI it will be harder to disable at the
executable level but we currently have no conditional support for this
in any libc anyway so that would be new development.  Ideally we would
have interfaces that allowed us to more clearly specify what is enabled
and disabled by a given syscall but this would be a far more difficult
change to deploy.

Reported-by: Jeremy Linton <redacted>
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Szabolcs Nagy <redacted>
Cc: Dave Martin <dave.martin@arm.com>
Cc: Kees Cook <redacted>
Cc: libc-alpha@sourceware.org
---

This solution was proposed by Catalin, I'm just writing it up into a
patch since it looks to be what we've converged on as the most practical
solution and but things seemed to have stalled out.

 arch/arm64/kernel/process.c | 8 --------
 1 file changed, 8 deletions(-)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 71c8265b9139..0967f9e1f9fd 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -717,14 +717,6 @@ asmlinkage void __sched arm64_preempt_schedule_irq(void)
 int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
 			 bool has_interp, bool is_interp)
 {
-	/*
-	 * For dynamically linked executables the interpreter is
-	 * responsible for setting PROT_BTI on everything except
-	 * itself.
-	 */
-	if (is_interp != has_interp)
-		return prot;
-
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
 	if (!(state->flags & ARM64_ELF_BTI))
 		return prot;
The original idea was to interfere with userspace as little as possible,
and leave this until/unless there was a clear need for it and a clear
understanding that it wouldn't break anything.

Looks like we have both of those now -- I'll leave it to Szabolcs to
confirm the userspace view of this.

Question: will this change prevent BTI executables from working under a
non-BTI-aware ld.so?  And do we care?  (I think "probably not" for both,
but I'd be interested in others' views.)

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help