Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aarch64: enable PAC/BTI #15

Open
wants to merge 1 commit into
base: master-with-github-ci
Choose a base branch
from

Conversation

billatarm
Copy link

Enable Pointer Authentication Codes (PAC) and Branch Target Identification (BTI) support for ARM 64 targets.

PAC works by signing the LR with either an A key or B key and verifying the return address. Since the assembly code does not push and pop the link register to the stack, and it remains in the register file, their is no need to sign the LR, so PAC is essentially just adding it to the GNU notes section for auditing purposes.

BTI works by marking all call and jump positions with bti c and bti j instructions. If execution control transfers via an indirect branch or call to an instruction other than a BTI instruction, the execution is killed via SIGILL.

For BTI to work, all object files linked for a unit of execution, whether an executable or a library must have the GNU Notes section of the ELF file marked to indicate BTI support. This is so loader/linkers can apply the proper permission bits (PROT_BRI) on the memory region.

PAC can also be annotated in the GNU ELF notes section, but it's not required for enablement, as interleaved PAC and non-pac code works as expected since it's the callee that performs all the checking.

Becuase the aarch64 assembly code does not make use of pushing the LR to the stack, only BTI targets were needed to be instrumented and the GNU notes section indicating support for BTU. Thus for PAC the only requirement was to mark the GNU notes section as supporting PAC.

Testing was done under the following CFLAGS:

  1. -mbranch-protection=none
  2. -mbranch-protection=standard
  3. -mbranch-protection=pac-ret
  4. -mbranch-protection=pac-ret+b-key
  5. -mbranch-protection=bti

Copy link

github-actions bot commented Sep 6, 2024

The mpg123-devel mailing list has been notified of the existence of this pr.

@sobukus
Copy link

sobukus commented Sep 6, 2024

Can you explain to me why this is necessary on ARM and not on x86? I got around the BTI stuff for x86 by introducing wrapper C functions, avoiding any indirect branches to the assembly code (meaning: calls via function pointer).

So I didn't need to add BTI landing pads to the other assembly functions. Is ARM different? Or would just marking the code as safe make the compiler happy here, too? Is the compiler actually happy for the x86 code?

What toolchain do I need to verify if this works or not? I did some testing with clang on Android using termux, but that wasn't conclusive.

Enable Pointer Authentication Codes (PAC) and Branch Target
Identification (BTI) support for ARM 64 targets.

PAC works by signing the LR with either an A key or B key and verifying
the return address. Since the assembly code does not push and pop the
link register to the stack, and it remains in the register file, their
is no need to sign the LR, so PAC is essentially just adding it to the
GNU notes section for auditing purposes.

BTI works by marking all call and jump positions with bti c and bti
j instructions. If execution control transfers via an indirect branch
or call to an instruction other than a BTI instruction, the execution
is killed via SIGILL.

For BTI to work, all object files linked for a unit of execution,
whether an executable or a library must have the GNU Notes section of
the ELF file marked to indicate BTI support. This is so loader/linkers
can apply the proper permission bits (PROT_BRI) on the memory region.

PAC can also be annotated in the GNU ELF notes section, but it's not
required for enablement, as interleaved PAC and non-pac code works as
expected since it's the callee that performs all the checking.

Becuase the aarch64 assembly code does not make use of pushing the LR to
the stack, only BTI targets were needed to be instrumented and the GNU
notes section indicating support for BTU. Thus for PAC the only
requirement was to mark the GNU notes section as supporting PAC.

Testing was done under the following CFLAGS and CXXFLAGS for all
combinations:
1. -mbranch-protection=none
2. -mbranch-protection=standard
3. -mbranch-protection=pac-ret
4. -mbranch-protection=pac-ret+b-key
5. -mbranch-protection=bti

Signed-off-by: Bill Roberts <[email protected]>
@billatarm
Copy link
Author

Can you explain to me why this is necessary on ARM and not on x86?

I don't know how x86 did their implementation so no comment.

I got around the BTI stuff for x86 by introducing wrapper C functions, avoiding any indirect branches to the assembly code (meaning: calls via function pointer).

Yeah, good point. I went back through and ran the test suite as well as looked all the function calls and none are done indirectly. I don't know why my initial thought was that I needed the BTI_C landing pad. I cleaned it, removed the landing pads and tested again and its fine and updated the PR. However, you still need the gnu notes bits to instruct the loader to mmap it with PROT_BTI.

So I didn't need to add BTI landing pads to the other assembly functions. Is ARM different?

I don't know about x86, but for Arm it's only indirect branches.

Or would just marking the code as safe make the compiler happy here, too?

Yeah that appears to be all that we need.

Is the compiler actually happy for the x86 code?

No idea.

What toolchain do I need to verify if this works or not?

Any contemporary gcc or clang version that supports -mbranch-protection=standard. The GNU Notes for the shared object should indicate that it's enabled:

readelf -n ./src/libmpg123/.libs/libmpg123.so | grep -i bti
      Properties: AArch64 feature: BTI, PAC

I did some testing with clang on Android using termux, but that wasn't conclusive.

Unless you have a pac/bti enabled machine, in my case a M2, you will only get the bti/pac instructions emitted in the compiled code but they will nop. You should see the instructions in a disassembly (usually as things like hint 34 for bti c) as well as you should see the GNU Notes bits set that PAC and BTI are enabled.

How to check if your system supports it:

# BTI (bti)
#PAC (paca and pacg)
cat /proc/cpuinfo
Features	: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 asimddp sha512 asimdfhm dit uscat ilrcpc flagm ssbs sb **paca pacg** dcpodp flagm2 frint i8mm bf16 **bti** ecv

@sobukus
Copy link

sobukus commented Sep 7, 2024

Hm, the aarch64 cross toolchain availabl in debian (gcc 12) seems too old? Or is it readelf? even with generic CPU code, I don't see a line with Properties/Aarch64 features. Only a build ID. In termux, it's some opaque flags.

Also: mbrach-potection is only a think on aarch64, anyway, as it seems.

@williamcroberts
Copy link

Hm, the aarch64 cross toolchain availabl in debian (gcc 12) seems too old? Or is it readelf? even with generic CPU code, I don't see a line with Properties/Aarch64 features. Only a build ID. In termux, it's some opaque flags.

What OS are you on, I'll try and replicate? I would imagine gcc12 supports it so it might be a lack of support in readelf, but I doubt it as -n predates this work IIUC. I can tell you it works on Fedora and the feature is enabled in the Debian builders for packages.

Also: mbrach-potection is only a think on aarch64, anyway, as it seems.

Yes its aarch64 specific that's why it's behind a -m option.

@sobukus
Copy link

sobukus commented Sep 10, 2024

There are two aarch64 toolchains available in Ubuntu 22.04.4 LTS.

gcc-aarch64-linux-gnu/jammy 4:11.2.0-1ubuntu1 amd64
  GNU C compiler for the arm64 architecture
gcc-12-aarch64-linux-gnu-base/jammy-updates,jammy-security,now 12.3.0-1ubuntu1~22.04cross1 amd64 [instalado, automático]
  GCC, the GNU Compiler Collection (base package)

I tested a build with ./configure --host=aarch64-linux-gnu CFFLAGS=-mbranch-protection=standard (and CC= for the latter), even with --with-cpu=generic to disable assembly codes. That, as baseline, should produce a branch-protected binary also without this PR … Debian 12 seems to have the same versions.

Of course, I could get a fresher toolchain and/or OS container to test, but I used what I had at hand while being busy.

@williamcroberts
Copy link

CFFLAGS

Was this is s typo?

Ill reproduce with those versions, thanks.

@sobukus
Copy link

sobukus commented Sep 10, 2024

Good point. To make sure:

$ ./configure --host=aarch64-linux-gnu --with-cpu=generic --with-audio=dummy CC=aarch64-linux-gnu-gcc-12 CFLAGS=-mbranch-protection=standard
$ make -j5

$ LANG=C readelf -n src/libmpg123/.libs/libmpg123.so

Displaying notes found in: .note.gnu.build-id
  Owner                Data size 	Description
  GNU                  0x00000014	NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: 6db06fa3b97f53251235af30de2d893b99dc1f08

(That's on Ubuntu 22.04.)

@billatarm
Copy link
Author

Ok, so while gcc13 supports it on Fedora, it does not on Ubuntu, my guess is some build time flag not getting set. However gcc-14 on Ubuntu does support it either. I need to check with the debian folks as to why. I can also confirm that current version of readelf will show the gnu notes bits. I took a confirmed shared object from Fedora and used readelf -n from Ubuntu and got the expected output.

Ok it does add the instructions. I compiled with and without -mbranch-protection=standard and saved each objfile's disassembly (objdump -d) and then diff'd them looking for things like paciasp:

diff ~/d.with.txt ~/d.without.txt | grep paciasp
<     6520:	d503233f 	paciasp

@sobukus
Copy link

sobukus commented Sep 14, 2024

I also see those paciasp instructions. In fact

$ grep -w paciasp libmpg123-generic-branch-protect.asm | wc -l
238
$ grep -w paciasp libmpg123-aarch64-branch-protect.asm | wc -l
253
$ grep -w paciasp libmpg123-generic.asm | wc -l
0
$ grep -w paciasp libmpg123-aarch64.asm | wc -l
0

(.asm obtained via aarch64-linux-gnu-objdump -d on the final libmpg123.so, with and without mbranch-protection=standard)

There are more instances of this instruction with the assembly code, but of couse not inside the assembly objects.

So, which toolchain actually does add the relevant notes section itself? Would it be confusing if our assembly files just have them and other objects not?

$ LANG=C readelf -S src/libmpg123/.libs/libmpg123.so.0.48.2 
There are 28 section headers, starting at offset 0x52978:

Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0]                   NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
  [ 1] .note.gnu.bu[...] NOTE             00000000000001c8  000001c8
       0000000000000024  0000000000000000   A       0     0     4
  [ 2] .gnu.hash         GNU_HASH         00000000000001f0  000001f0
       0000000000000514  0000000000000000   A       3     0     8
  [ 3] .dynsym           DYNSYM           0000000000000708  00000708
       0000000000001350  0000000000000018   A       4     3     8
  [ 4] .dynstr           STRTAB           0000000000001a58  00001a58
       0000000000000c99  0000000000000000   A       0     0     1
  [ 5] .gnu.version      VERSYM           00000000000026f2  000026f2
       000000000000019c  0000000000000002   A       3     0     2
  [ 6] .gnu.version_r    VERNEED          0000000000002890  00002890
       0000000000000070  0000000000000000   A       4     3     8
  [ 7] .rela.dyn         RELA             0000000000002900  00002900
       0000000000002808  0000000000000018   A       3     0     8
  [ 8] .rela.plt         RELA             0000000000005108  00005108
       00000000000009d8  0000000000000018  AI       3    21     8
  [ 9] .init             PROGBITS         0000000000005ae0  00005ae0
       0000000000000018  0000000000000000  AX       0     0     4
  [10] .plt              PROGBITS         0000000000005b00  00005b00
       00000000000006b0  0000000000000000  AX       0     0     16
  [11] .text             PROGBITS         00000000000061b0  000061b0
       0000000000028e64  0000000000000000  AX       0     0     16
  [12] .fini             PROGBITS         000000000002f014  0002f014
       0000000000000014  0000000000000000  AX       0     0     4
  [13] .rodata           PROGBITS         000000000002f030  0002f030
       0000000000015f4b  0000000000000000   A       0     0     16
  [14] .eh_frame_hdr     PROGBITS         0000000000044f7c  00044f7c
       0000000000000c7c  0000000000000000   A       0     0     4
  [15] .eh_frame         PROGBITS         0000000000045bf8  00045bf8
       00000000000043bc  0000000000000000   A       0     0     8
  [16] .init_array       INIT_ARRAY       000000000005a4b0  0004a4b0
       0000000000000008  0000000000000008  WA       0     0     8
  [17] .fini_array       FINI_ARRAY       000000000005a4b8  0004a4b8
       0000000000000008  0000000000000008  WA       0     0     8
  [18] .data.rel.ro      PROGBITS         000000000005a4c0  0004a4c0
       0000000000000820  0000000000000000  WA       0     0     16
  [19] .dynamic          DYNAMIC          000000000005ace0  0004ace0
       00000000000001f0  0000000000000010  WA       4     0     8
  [20] .got              PROGBITS         000000000005aed0  0004aed0
       0000000000000110  0000000000000008  WA       0     0     8
  [21] .got.plt          PROGBITS         000000000005afe8  0004afe8
       0000000000000360  0000000000000008  WA       0     0     8
  [22] .data             PROGBITS         000000000005b350  0004b350
       00000000000005e0  0000000000000000  WA       0     0     16
  [23] .bss              NOBITS           000000000005b930  0004b930
       0000000000000008  0000000000000000  WA       0     0     1
  [24] .comment          PROGBITS         0000000000000000  0004b930
       000000000000002b  0000000000000001  MS       0     0     1
  [25] .symtab           SYMTAB           0000000000000000  0004b960
       0000000000004950  0000000000000018          26   579     8
  [26] .strtab           STRTAB           0000000000000000  000502b0
       00000000000025cb  0000000000000000           0     0     1
  [27] .shstrtab         STRTAB           0000000000000000  0005287b
       00000000000000fa  0000000000000000           0     0     1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  D (mbind), p (processor specific)

No gnu.notes.property section. I got binutils 2.38 here … shouldn't it support that section?

I even found an old question from someone toying with assembly on how to prevent adding that section:

https://stackoverflow.com/questions/52084184/how-do-i-prevent-ld-from-adding-note-gnu-property

Did Ubuntu/Debian do something specific to disable that? I'd like to understand this a bit better before unconditionally adding that section ot our assembly files.

@billatarm
Copy link
Author

There are more instances of this instruction with the assembly code, but of couse not inside the assembly objects.

So, which toolchain actually does add the relevant notes section itself?
The version you're using does support it, It has to do with Ubuntu and Debian specific issues:

Essentially not every library dependency in Debian has it enabled, so the linker is turning it off. If you just grab something like Fedora 40 it will show up.

Would it be confusing if our assembly files just have them and other objects not?

No it was designed to work in this case. For PAC, it's not needed, just a nice to have for auditing. You can mix PAC and non-pac code together. Same for BTI, but when the BTI enabled code is statically linked the linker discards the note, and when the pages are mapped, the note is consulted to add PROT_BTI to the mapping. For dynamically loaded things, the note is checked and the mapping protections applied based on the note. So a process may have some pages marked PROT_BTI and some pages not marked, and it all works.

You can see this behavior looking at the individually built objects:

readelf -n ./src/libmpg123/.libs/check_neon.o

Displaying notes found in: .note.gnu.property
  Owner                Data size 	Description
  GNU                  0x00000010	NT_GNU_PROPERTY_TYPE_0
      Properties: AArch64 feature: BTI, PAC

No gnu.notes.property section. I got binutils 2.38 here … shouldn't it support that section?

Not all sections are mandatory, gnu notes will only display if a gnu notes section is added. Its not added if its empty.
The linked result because of some dependency strips it. Specifically these deps:

configure:4739: gcc -mbranch-protection=standard  -Wl,-zforce-bti,--fatal-warnings conftest.c  >&5
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/Scrt1.o: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crti.o: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/crtbeginS.o: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/crtendS.o: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
/usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crtn.o: warning: BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
collect2: error: ld returned 1 exit status

You can recreate this by running:

./configure CFLAGS='-mbranch-protection=standard' LDFLAGS='-Wl,-zforce-bti,--fatal-warnings'

I even found an old question from someone toying with assembly on how to prevent adding that section:

https://stackoverflow.com/questions/52084184/how-do-i-prevent-ld-from-adding-note-gnu-property

Did Ubuntu/Debian do something specific to disable that? I'd like to understand this a bit better before unconditionally adding that section ot our assembly files.

Its the debian runtime AFAICT not built with support yet, but that's ok, as it's all interoperable. Here are some more projects that have picked these changes up:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants