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

From: Dave Martin <Dave.Martin@arm.com>
To: Szabolcs Nagy <szabolcs.nagy@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	libc-alpha@sourceware.org, Kees Cook <keescook@chromium.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jeremy Linton <jeremy.linton@arm.com>,
	Mark Brown <broonie@kernel.org>, Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: bti: Set PROT_BTI on all BTI executables mapped by the kernel
Date: Mon, 8 Feb 2021 17:40:29 +0000	[thread overview]
Message-ID: <20210208174028.GG21837@arm.com> (raw)
In-Reply-To: <20210208164744.GA16506@arm.com>

On Mon, Feb 08, 2021 at 04:47:45PM +0000, Szabolcs Nagy via Libc-alpha wrote:
> The 02/08/2021 14:13, Szabolcs Nagy via Libc-alpha wrote:
> > The 02/08/2021 12:44, Will Deacon wrote:
> > > I'd like an Ack from Szabolcs before we queue this.
> > 
> > i'm ok with this in principle, but i will rerun
> > the glibc tests over night to be sure.
> 
> 
> the patch applied cleanly on top of arm64 for-next/core
> 
> but it does not work as i expected:
> 
> executables that do not have the bti property note
> set seems to get bti guarded by the kernel.
> 
> at least i see crash in _start when the dynamic
> linker (which has bti marking) transfers control
> to it and the start code has no bti c.
> 
> (according to strace the dynamic linker did not
> remap/mprotect the main exe with bti so i assume
> this is the kernel's doing)
> 
> can somebody verify that the notes are checked
> on the executable too and not just on ld.so?

Reviewed-by bites the dust...

Aha, looking at the ELF code in the kernel, it looks like some extra
refactoring is needed.

We do the heavy lifting only for the image containing the userspace
entry point -- i.e., ld.so in the dynamically linked case.  This
includes the ELF property handling.  When ld.so is present, the main
executable is just data so we map it in but don't do a whole lot else
with it:

static int load_elf_binary(struct linux_binprm *bprm)
{
	/* ... */

	retval = parse_elf_properties(interpreter ?: bprm->file,
				      elf_property_phdata, &arch_state);

The way I originally integrated this therefore just tracks the BTI-ness
(and/or equivalently how to mutate PROT_EXEC) once.  Looks like we need
to do that independently for ld.so and for the executable instead.

We could simplify things by treating it as an error if the executable
and ld.so have different BTI properties, but that seems a bit of an own
goal, since it breaks foreseeable backwards compatibility / hybrid use
cases.

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel