Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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.