|
|
Message-ID: <d37e4b8b-6cc9-4c3b-b402-112b3529d9fe@foss.arm.com>
Date: Fri, 14 Nov 2025 00:35:11 -0600
From: Bill Roberts <bill.roberts@...s.arm.com>
To: Bill Roberts <bill.roberts@....com>, musl@...ts.openwall.com
Subject: Re: [PATCH 1/3] dso/aarch64: add PROT_BTI support for mem maps
On 11/14/25 11:37 AM, Szabolcs Nagy wrote:
> * Bill Roberts <bill.roberts@....com> [2025-11-13 13:44:26 -0600]:
>
>> This change adds support for setting PROT_BTI on executable mappings
>> when the corresponding ELF object includes the GNU_PROPERTY_AARCH64_FEATURE_1_BTI
>> note. This ensures correct Branch Target Identification (BTI) enforcement
>> as defined in the Arm Architecture Reference Manual (Arm ARM) and the
>> ELF for the Arm 64-bit Architecture (AArch64) ABI supplement.
>>
>> When loading shared objects or executables, the dynamic loader now checks
>> for BTI-related GNU notes and applies the appropriate protection flags
>> via `mprotect` and `mmap` during mapping. This aligns musl’s behavior with
>> other modern loaders, improving security and correctness on systems with BTI
>> enabled hardware.
>>
>> References:
>> - Arm Architecture Reference Manual for A-profile architecture (Arm ARM):
>> https://developer.arm.com/documentation/ddi0487/latest
>> - ELF for the Arm® 64-bit Architecture (AArch64) ABI supplement:
>> https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst
>> - Arm Community Blog – *Enabling Pointer Authentication and Branch Target Identification*:
>> 1. https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/enabling-pac-and-bti-part1
>> 2. https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/enabling-pac-and-bti-part2
>> 3. https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/enabling-pac-and-bti-part3
>>
>> Signed-off-by: Bill Roberts <bill.roberts@....com>
>> ---
> ...
>> +++ b/configure
>> @@ -671,6 +671,7 @@ fi
>>
>> if test "$ARCH" = "aarch64" ; then
>> trycppif __AARCH64EB__ "$t" && SUBARCH=${SUBARCH}_be
>> +CFLAGS_AUTO="${CFLAGS_AUTO} -DARCH_SUPPORTS_DL_ADD_PROTECTIONS"
>
> don't pass this around everywhere, add it to reloc.h
>
>> +++ b/ldso/dynlink.c
>> @@ -682,6 +682,21 @@ static void unmap_library(struct dso *dso)
>> }
>> }
>>
>> +#ifdef ARCH_SUPPORTS_DL_ADD_PROTECTIONS
>> +#include <arch_ldso_hook.h>
>> +#else
>> +static inline unsigned dl_add_protections(const Elf64_Phdr *ph,
>> + const Elf64_Ehdr *eh, int fd, unsigned *prot, unsigned *mask)
>> +{
>> + (void)ph;
>> + (void)eh;
>> + (void)fd;
>> + (void)prot;
>> + (void)mask;
>> + return 0;
>> +}
>> +#endif
>> +
>
> you can place the hook+macro in arch/aarch64/reloc.h
> then no new file is needed.
>
>> static void *map_library(int fd, struct dso *dso)
>> {
>> Ehdr buf[(896+sizeof(Ehdr))/sizeof(Ehdr)];
>> @@ -693,7 +708,7 @@ static void *map_library(int fd, struct dso *dso)
>> off_t off_start;
>> Ehdr *eh;
>> Phdr *ph, *ph0;
>> - unsigned prot;
>> + unsigned prot, arch_prot, arch_mask;
>> unsigned char *map=MAP_FAILED, *base;
>> size_t dyn=0;
>> size_t tls_image=0;
>> @@ -720,6 +735,16 @@ static void *map_library(int fd, struct dso *dso)
>> } else {
>> ph = ph0 = (void *)((char *)buf + eh->e_phoff);
>> }
>> +
>> + /*
>> + * some architectures may have additional protection flags embedded in the
>> + * ELF section somewhere. Start with those and OR in the extras. Do this
>> + * before we need to map any sections.
>> + */
>> + if(dl_add_protections(ph, eh, fd, &arch_prot, &arch_mask))
>> + goto error;
>> +
>> +
>> for (i=eh->e_phnum; i; i--, ph=(void *)((char *)ph+eh->e_phentsize)) {
>> if (ph->p_type == PT_DYNAMIC) {
>> dyn = ph->p_vaddr;
>> @@ -746,6 +771,7 @@ static void *map_library(int fd, struct dso *dso)
>> prot = (((ph->p_flags&PF_R) ? PROT_READ : 0) |
>> ((ph->p_flags&PF_W) ? PROT_WRITE: 0) |
>> ((ph->p_flags&PF_X) ? PROT_EXEC : 0));
>> + prot |= prot&arch_mask ? arch_prot : 0;
>> }
>> if (ph->p_vaddr+ph->p_memsz > addr_max) {
>> addr_max = ph->p_vaddr+ph->p_memsz;
>> @@ -762,6 +788,7 @@ static void *map_library(int fd, struct dso *dso)
>> prot = (((ph->p_flags&PF_R) ? PROT_READ : 0) |
>> ((ph->p_flags&PF_W) ? PROT_WRITE: 0) |
>> ((ph->p_flags&PF_X) ? PROT_EXEC : 0));
>> + prot |= prot&arch_mask ? arch_prot : 0;
>> map = mmap(0, ph->p_memsz + (ph->p_vaddr & PAGE_SIZE-1),
>> prot, MAP_PRIVATE,
>> fd, ph->p_offset & -PAGE_SIZE);
>> @@ -780,6 +807,7 @@ static void *map_library(int fd, struct dso *dso)
>> size_t pgbrk = brk + PAGE_SIZE-1 & -PAGE_SIZE;
>> size_t pgend = brk + ph->p_memsz - ph->p_filesz
>> + PAGE_SIZE-1 & -PAGE_SIZE;
>> + /* arch_prot has already been added */
>> if (pgend > pgbrk && mmap_fixed(map+pgbrk,
>> pgend-pgbrk, prot,
>> MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS,
>> @@ -801,11 +829,16 @@ static void *map_library(int fd, struct dso *dso)
>> * the length of the file. This is okay because we will not
>> * use the invalid part; we just need to reserve the right
>> * amount of virtual address space to map over later. */
>> - map = DL_NOMMU_SUPPORT
>> - ? mmap((void *)addr_min, map_len, PROT_READ|PROT_WRITE|PROT_EXEC,
>> - MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
>> - : mmap((void *)addr_min, map_len, prot,
>> - MAP_PRIVATE, fd, off_start);
>> + if (map == DL_NOMMU_SUPPORT) {
>> + prot = PROT_READ|PROT_WRITE|PROT_EXEC;
>> + prot |= prot&arch_mask ? arch_prot : 0;
>> + map = mmap((void *)addr_min, map_len, prot,
>> + MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
>> + } else {
>> + prot = prot & arch_mask ? prot | arch_prot : prot;
>> + map = mmap((void *)addr_min, map_len, prot,
>> + MAP_PRIVATE, fd, off_start);
>> + }
>
> i'd leave this code alone, prot should be
> set up already with arch_prot.
> the nommu case does not need bti support.
>
>> if (map==MAP_FAILED) goto error;
>> dso->map = map;
>> dso->map_len = map_len;
>> @@ -835,6 +868,7 @@ static void *map_library(int fd, struct dso *dso)
>> prot = (((ph->p_flags&PF_R) ? PROT_READ : 0) |
>> ((ph->p_flags&PF_W) ? PROT_WRITE: 0) |
>> ((ph->p_flags&PF_X) ? PROT_EXEC : 0));
>> + prot |= prot&arch_mask ? arch_prot : 0;
>> /* Reuse the existing mapping for the lowest-address LOAD */
>> if ((ph->p_vaddr & -PAGE_SIZE) != addr_min || DL_NOMMU_SUPPORT)
>> if (mmap_fixed(base+this_min, this_max-this_min, prot, MAP_PRIVATE|MAP_FIXED, fd, off_start) == MAP_FAILED)
>> @@ -849,7 +883,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)
>> + prot = PROT_READ|PROT_WRITE|PROT_EXEC;
>> + prot |= prot&arch_mask ? arch_prot : 0;
>> + if (mprotect(map, map_len, prot)
>> && errno != ENOSYS)
>> goto error;
>
> i'd leave this alone too: DT_TEXTREL is a
> much bigger security hole than what bti
> is trying to cover here.
>
> i don't think PROT_BTI generalizes to other targets
>
>> + for (i=eh->e_phnum; i; i--, ph=(void *)((char *)ph+eh->e_phentsize)) {
>> +
>> + /* skip non note sections */
>> + if (ph->p_type != PT_NOTE)
>> + continue;
>> +
>> + buf = malloc(ph->p_filesz);
>> + if (!buf)
>> + return rc;
>> +
>> + if (pread(fd, buf, ph->p_filesz, ph->p_offset) != ph->p_filesz)
>> + goto error;
> ...
>
> yeah gnu properties are a bit ugly..
>
> the kernel handles the ld.so, the vdso and the
> static-linked exe cases.
>
> however i don't think the kernel adds PROT_BTI
> to the main exe it loads, and map_library does
> not work for that (there is no fd to be mapped).
> please check what the kernel does (might behave
> differently now).
>
> this caused trouble with systemd that seccomp
> blocked mprotect(PROT_EXEC|PROT_BTI), which was
> eventually solved on the kernel side
> https://lkml.org/lkml/2023/1/19/753
>
> please test that exec pages have bti:
>
> ./bti-cat /proc/self/smaps |grep 'VmFlags.*ex'
> ld.so ./bti-cat /proc/self/smaps |grep 'VmFlags.*ex'
> ./static-bti-cat /proc/self/smaps |grep 'VmFlags.*ex'
> ./cat-with-dlopen /proc/self/smaps |grep 'VmFlags.*ex'
>
> all lines should contain a 'bt' flag.
Thanks for that history, will check. AFAICT, it looks to
be the same, so you need to mprotect itself. There is a check
were if the interpreter in the ELF file is not itself to not
set PROT_BTI. Kernel code snippet below:
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;
if (!(state->flags & ARM64_ELF_BTI))
return prot;
if (prot & PROT_EXEC)
prot |= PROT_BTI;
return prot;
}
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.