[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFftDdoK1tc9foTMLhn6NXXDaR9am3qNVF0jfUP7XNT9TzRpeg@mail.gmail.com>
Date: Tue, 13 Feb 2024 21:19:02 -0600
From: William Roberts <bill.c.roberts@...il.com>
To: musl@...ts.openwall.com
Cc: Markus Wichmann <nullplan@....net>, enh <enh@...gle.com>
Subject: Re: PAC/BTI Support on aarch64
On Tue, Feb 13, 2024 at 8:19 PM Rich Felker <dalias@...c.org> wrote:
>
> On Tue, Feb 13, 2024 at 06:51:47PM +0100, Markus Wichmann wrote:
> > Am Tue, Feb 13, 2024 at 08:47:42AM -0600 schrieb William Roberts:
> > > It should. Is there a known minimal tool chain requirement and I can test?
> >
> > Typically the first C99 compiler or the first aarch64 compiler,
> > whichever is younger.
>
> I think binutils is the relevant component, and that'd be whichever
> version of binutils added aarch64.
AFAICT 2.24 tagged Dec 2013
>
> > > No, anywhere branches are allowed, a BTI instruction must be the first
> > > instruction. BTI is just a way for software to say, hey this is a
> > > valid jump/branch
> > > target, allow it. This reduces the amount of gadgets available to an
> > > attacker, which
> > > is why libc is such a juicy target, as it's in everything. A lot of
> > > things static link it,
> > > which effectively turns it off for the whole process.
> > >
> >
> > So this means there must be a BTI instruction following every single BL
> > instruction.
> >
I don't think so, you wouldn't want call sites to effectively become gadget
locations. You want entry points marked, returns are handled with PAC,
which goes hand in hand with BTI. As the PAC instruction can also be
a landing pad. Looking at some generated ASM, I don't see BL's being marked.
Here's a decent doc BTW:
- https://www.google.com/search?q=arm+introduction+to+bti&rlz=1C5GCEM_enUS1088US1089&oq=arm+introduction+to+bti&gs_lcrp=EgZjaHJvbWUyBggAEEUYOTIHCAEQIRigATIHCAIQIRigATIHCAMQIRifBTIHCAQQIRifBTIHCAUQIRifBdIBCDM2NTZqMGo3qAIAsAIA&sourceid=chrome&ie=UTF-8#:~:text=Arm%20Instruction%20Set,Arm%20Compiler%206
Essentially call points need a valid pac/bti instruction, if using
pac, then there must
be a validate before ret.
landing pad with pointer auth (with the A key): pacisasp or hint #25
validate with autiasp or int #29
landing pad with pointer auth (with the B key): pacisasp or hint #hint
#27 validate with autibsp or hint #31
landing pad BTI only: bti c or hint #34
The compilers set some defines so you know which key to use, but some
projects just support the A key.
To support other keys, you would need to go the route of conditional
asm, but almost
everyone just uses the A key, it's what's turned on by
-mbranch-protection=standard (I think),
easy to catch in the arm header and balk if someone sets the B key.
> > But in the end this isn't that much different from endbr64 on the PC.
> > Whatever happened to those patches, BTW?
>
> What is the situation on x86? Does it use the same kind of per-page
> enforcement mode, or is it only global, requiring disabling it if any
> DSO lacks support? Is the endbr64 opcode a guaranteed-safe nop on
> older ISA levels, or does it need to be conditional?
>
> > > Yes, so the kernel will manage the EL1 register flag for this, and then
> > > mprotect sets the PROT_BTI flag during dlopen().
> >
> > Well, this is a novelty. This is the first time there will be an
> > arch-specific flag in mmap()/mprotect() for the musl dynlinker. So far
> > that code has been entirely portable.
>
> Can the flag be used at mmap time, or only in mprotect? It would be a
> lot more efficient to do it as part of the mmap, but getting
> visibility to the note to know you need it at mmap time seems
> difficult and more costly than doing the mprotect later...
>
> I assume we would either add the code conditional on the existence of
> a PROT_BTI macro (#ifdef) and define that to the corresponding thing
> on other archs in the future, or abstract it with a new name in
> arch/$ARCH/reloc.h defined in terms of whatever the arch provides so
> as to be a little bit more naming-agnostic.
>
> It should not be #ifdef __aarch64__ or similar.
>
> > > It's important to note, that even when enabling the assembly code files, if the
> > > C level source is not built with -mbranch-protection=standard, the feature will
> > > remain off for the library.
> > >
> >
> > Arch-specific compiler flags are not a problem; configure.sh can add
> > those as needed.
>
> Yep, that's fine. Possibly a question of whether it should be on by
> default or configurable, but if there's essentially no cost,
> on-by-default seems fine.
>
> > > I can't think of anything like this offhand, but aarches may want to add prot
> > > flags to mprotect calls.
> >
> > That hasn't happened yet. Of course, this may be as simple as adding a
> > static inline function. The fact that the important information is in a
> > note section is yet another novelty, of course. So far, the important
> > information (even arch-specific) has been contained in the dynamic
> > section.
>
> Yes, that's gratuitously annoying. Ideally it would have been
> somewhere easily accessible from the Ehdr so it's available at initial
> mmap time... :/
>
> > > it usually
> > > #ifdef aarch64
> > > if (gnu_notes_bti_set && (prot & PROT_EXEC)) {
> > > prot |= PROT_BTI;
> > > else {
> > > prot &= ~PROT_BTI;
> > > }
> > > #endif
> > >
> > > mprotect(..., prot);
> > >
> >
> > So far we have managed to steer clear of conditional inclusion, and I
> > think we should try to keep it that way.
>
> Yes. I think reloc.h should define a predicate macro (which may call a
> static inline function if the predicate is complex) to check if a DSO
> needs branch protection on its PROT_EXEC segments.
> src/internal/dynlink.h could provide a default always-false one if
> it's not defined. Then dynlink.c can just, when that predicate
> evaluates true, loop thru the segments and mprotect any PROT_EXEC ones
> to also have PROT_BTI or whatever.
I was tinkering today, arch/generic has a bunch of empty files, just
add an empty file
and let arches add to it. This code is far from useful, but just
clarifying that approach.
diff --git a/arch/aarch64/mprot_arch.h b/arch/aarch64/mprot_arch.h
new file mode 100644
index 00000000..32f7afc6
--- /dev/null
+++ b/arch/aarch64/mprot_arch.h
@@ -0,0 +1,13 @@
+#ifndef _MPROT_ARCH_H
+#define _MPROT_ARCH_H
+
+static inline int do_mprot(int prot) {
+ if (prot & PROT_EXEC)
+
+#define MPROT_ARCH(prot) \
+do {
+ if (prot & PROT_EXEC) { \
+ prot |= PROT_BTI; \
+ } \
+} while(0)
+#endif
diff --git a/arch/generic/mprot_arch.h b/arch/generic/mprot_arch.h
new file mode 100644
index 00000000..e69de29b
diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index 324aa859..a9b2278a 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -22,6 +22,7 @@
#include "pthread_impl.h"
#include "fork_impl.h"
#include "dynlink.h"
+#include "mprot_arch.h"
static size_t ldso_page_size;
#ifndef PAGE_SIZE
@@ -851,7 +852,9 @@ static void *map_library(int fd, struct dso *dso)
}
for (i=0; ((size_t *)(base+dyn))[i]; i+=2)
if (((size_t *)(base+dyn))[i]==DT_TEXTREL) {
- if (mprotect(map, map_len, PROT_READ|PROT_WRITE|PROT_EXEC)
+ int prot = PROT_READ|PROT_WRITE|PROT_EXEC;
+ MPROT_ARCH(prot);
+ if (mprotect(map, map_len, prot)
&& errno != ENOSYS)
goto error;
break;
> This remains very arch-agnostic and the code should be either directly
> usable on other archs, or admit easy generalization if needed.
>
> Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
