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

On Mon, Feb 08, 2021 at 05:40:29PM +0000, Dave P Martin wrote:

On Mon, Feb 08, 2021 at 04:47:45PM +0000, Szabolcs Nagy via Libc-alpha wrote:

quoted

The 02/08/2021 14:13, Szabolcs Nagy via Libc-alpha wrote:

quoted

The 02/08/2021 12:44, Will Deacon wrote:

quoted

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?

Thanks Szabolcs for testing.

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.

Ah, I haven't thought of these cases either.

In theory, if the dynamic loader hasn't been compiled with
-mbranch-protection=bti, it does not mean it won't be able to use
PROT_BTI on the mapped executable/libraries. The same goes for
BTI-compiled loader with a non-BTI executable, it should be allowed.

So I think decoupling the two makes sense but the patch is no longer as
simple as the ELF loader now needs to parse the main executable. If
that's doable and the code looks sane, I'd go with it. Otherwise, we
scrap the idea of the kernel setting PROT_BTI on the main executable.

-- 
Catalin

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