[PATCH 1/1] aarch64: support PAC and BTI

[Bill Roberts]

On 10/23/24 10:18 AM, Niels Möller wrote:
> Bill Roberts <bill.roberts at arm.com> writes:
>
>> Enable Pointer Authentication Codes (PAC) and Branch Target
>> Identification (BTI) support for ARM 64 targets.
> I only have a rough idea of how these security features work, but I have

I have a talk here on it: https://vimeo.com/1019370992
I'll have more public information available later.

> a few suggestions after having a new look at the patch. (These are my
> opinions as a GMP contributor, I'm not speaking for the rest of the GMP
> team).
>
> 1. Must PAC and BTI be used together, or can support be added as two
>     separate contributions?

I can split up the patch series, but they work better together. 
Essentially if you have
pac and bti enabled, the pac instructions work as a bti landing pad. 
This means with
both enabled the first instruction can be something like paciasp ... 
rest of code instead of 2
instructions like bti c paciasp .... rest of code, if that makes sense?

> 2. It's not so nice to need a BTI_C line in every(?) .asm file. Please
>     fold the needed magic inside the PROLOGUE macro. A 900 line patch
>     looks rather intimidating.

I have this fixed locally. I initially went that way and then abandoned 
it to a combination
of bad m4 foo and to get that one instruction speed up, but I think I 
understand the PROLOGUE_cpu
better, and my m4 error, and now was able to leverage that to simplify 
the patchset and get that
optimization.

>
> 3. Tests would help providing some confidence that this works as
>     intended, and keeps working as intended as GMP evolves. Maybe you can

When building this with CFLAGS='-mbranch-protection=standard', as like 
the Fedora package does, the
current test suite exercises this. It would fail with a pile of sigill's 
on systems that support BTI and sigsegv
for PAC if something in the assembly is missing.

>     get some inspiration from this test in Nettle:
>     https://git.lysator.liu.se/nettle/nettle/-/blob/master/testsuite/x86-ibt-test.c?ref_type=heads

That test does not ensure that Nettle works, it ensures that the 
architectural feature works and doesn't
ensure that Nettle has it implemented.You could take that test and drop 
it into anyone's project, so I have no idea why this would be in GMP's 
tree. This should be in a vendor's test suite or something. What you 
really want is a test that ensures it works for the actual gmp code base.

>     A test could verify (i) that an executable linked with GMP gets the
>     properly marks (ELF flags or whatever it is) to have the protections
>     enabled by the kernel,

That should be easy enough and tests that at least gmp has the proper 
annotations.

> and (ii) that if the executable runs some code
>     violating the PAC/BTI conventions, than it gets the expected kill
>     signal.

Offhand, I think an easy test would be to get a function pointer to 
something in the assembly,
add an instruction to the pointer value and then call it. Pure C and it 
will not land on a landing
pad, but rather the old entry point, that would be a test that tests 
nettle, not the processor.

> 4. "Dynamically generated m4" doesn't sound good to me. I would hope
>     this support can be added without adding one more level of
>     preprocessing. I'd prefer either configure tests based on, e.g.,
>     predefines set by the compiler.
That's what it's doing, it's invoking the compiler to get the 
environmental defines and slapping it into the m4.
If the problem is the mechanism of how, and there is a way to move this 
functionality somewhere else, like during
configure I am all ears. I guess it would be adding it to DEFS via -D, 
but how to plumb it over there isn't clear to
me at the moment, I can look into it more, is that the right direction 
you're thinking?
> If for some reason it's very
>     difficult to configure automatically in a robust way, then it's
>     probably better to start with only a configure argument for enabling
>     this, and improve on automatic configuration later.
> Regards,
> /Niels

Thanks, I appreciate your comments.

Bill