[PATCH v1 1/1] aarch64: make RA signing method extendable for upcoming PAuth_LR
Matthieu Longo
matthieu.longo@arm.com
Mon Nov 25 14:21:04 GMT 2024
On 2024-11-14 04:36, Indu Bhagat wrote:
> On 11/13/24 6:30 AM, Matthieu Longo wrote:
>> On 2024-11-12 22:42, Indu Bhagat wrote:
>>> On 11/8/24 7:25 AM, Matthieu Longo wrote:
>>>> The Pointer Authentication (PAuth) architecture provides instructions
>>>> that enable software to sign an address using SP as a diversifier.
>>>> This form of PAC instruction is typically used for signing return
>>>> addresses that are stored on the stack.
>>>>
>>>> The new Armv9.5-A architecture introduces an enhancement of the
>>>> previous PAuth feature, called PAuth_LR. It aims at hardening the PAC
>>>> in a signed return address. When signing the return address in LR, the
>>>> PC is used as a diversifier, in addition to the SP to generate PAC
>>>> code.
>>>>
>>>> PAuth introduced the DWARF register RA_SIGN_STATE which stores the
>>>> return address signed state (currently no signing, or signed with SP).
>>>> PAuth_LR adds a new value to this state: signed with SP and PC.
>>>>
>>>> This DWARF register can now be set via 2 directives:
>>>> - DW_CFA_AARCH64_negate_ra_state: toggle the RA state in a PAuth
>>>> context.
>>>> - DW_CFA_AARCH64_negate_ra_state_with_pc: toggle the RA state in a
>>>> PAuth_LR context.
>>>>
>>>> The existing implementation relies on a binary approach that cannot
>>>> accomodate with the new state value.
>>>> This patch is a refactoring to make the RA signing method extendable
>>>> for
>>>> the upcoming PAuth_LR patch series.
>>>
>>>
>>> Hi Matthieu,
>>>
>>> Thanks for the patch. And apologies for the massive delay. I somehow
>>> missed this patch in the original series.
>>
>> No worry. Thanks for having a look.
>>
>>> It looks like, in theory, an application is allowed to have mixed
>>> usage of DW_CFA_AARCH64_negate_ra_state_with_pc and
>>> DW_CFA_AARCH64_negate_ra_state across functions. Can you confirm my
>>> understanding ?
>>
>> Yes, in the same compilation unit, a given function is allowed to:
>> - not use return pointer authentication, and so none of the DWARF
>> directives mentioned above are emitted.
>> - use return pointer authentication with SP. The place of signing and
>> authentication are marked with DW_CFA_AARCH64_negate_ra_state.
>> - use return pointer authentication with SP + PC. The place of signing
>> and authentication are marked with
>> DW_CFA_AARCH64_negate_ra_state_with_pc.
>>
>> The 2 directives cannot be mixed together inside a function body.
>>
>>> If yes, then technically the signing method information is a per-
>>> SFrame- FDE information.
>>
>> Yes.
>>
>
> Thanks for confirming.
>
>>> I will add supporting Pauth_LR in the SFrame V3 ToDo
>>
>> What is this "SFrame V3 ToDo" ? I need a bit more context here.
>>
>
> SFrame (Simple Frame stack trace) format is currently at version 2.
> There are some planned enhancements to be targeted for Version 3. I
> have been keeping a rough ToDO list, I still need to clean it up and
> publish it somehwere on the Binutils wiki.
>
> IOW, it means SFrame V3 is being planned.
>
>>> list. For V3 then, for an SFrame FDE, we would add additional bits to
>>> keep this information. The SFrame FREs will continue to carry the
>>> boolean information of ra_mangled_p.
>>
>> In the binary form, the RA state register is composed of 2 bits, so
>> can store in theory 4 states but only 3 are valid according to the
>> specs [1].
>> The boolean ra_mangled_p does not allow to store 3 states, that is why
>> I had to change it. Adding an additional boolean to know if we are
>> dealing with SP or SP+PC signing does not seem the right approach from
>> my perspective as it would make the code more confusing. Please could
>> you explain me more what you meant ?
>>
>
> RA state register is a DWARF pseudo register. In SFrame, we can chose
> to keep information differently:
> - A function level bit to indicate whether pauth or pauth_lr is in
> effect. Something similar to the current SFRAME_V1_FUNC_PAUTH_KEY.
> - Continue to use 1-bit in SFrame FRE to indicate whether RA is mangled.
>
> Regarding it being confusing, I think this can be addressed by providing
> access macros/APIs.
>
> In any case, the above suggestion cannot be done unless we bump the
> format version to version 3.
>
> For the immediate problem at hand around handling
> DW_CFA_AARCH64_negate_ra_state_with_pc in SFrame, I suggest we error out
> with a warning. See below.
Firstly, I apologize for the confusion on my side. I thought SFrame was
the same thing as DWARF.
I had a look at the spec v2, everything makes sense now.
Great presentation by the way :) =>
https://www.youtube.com/watch?v=4XrFYpjyodo
I am going to publish a different patch series to add the support for
the new CFI directive in both DWARF and SFrame (warning + no FDE
generatation until SFrame v3 is released).
Regarding SFrame v3, do you have a draft version somewhere ?
https://sourceware.org/binutils/wiki/sframe does not list any.
Let me rephrase to check if I understood well your proposition for v3.
Since PAuth_LR is per-SFrame SDE, you would like to store whether the
address is signed with PAuth or PAuth_LR inside the 2 unused bits of
struct sframe_func_desc_entry. Or is it somewhere else ?
PAuth_LR also requires to know where the signing occured (PC of the
signing instruction) to demangle the return address.
Is it the responsibility of stacktrace generator to figure out where
this signing happens by scanning the instructions from the start of the
function looking for either "pacm + paci<k>sp" or "paci<k>sppc" ?
In the DWARF unwinder in libunwind (the patch series has not been
published yet, it is still in-progress, sorry for that), the demangling
of the address relies on the address value matching the DWARF
instruction DW_CFA_AARCH64_negate_ra_state_with_pc and substracts 4
(size of an instruction on AArch64) from it.
In my understanding, this search for the signing instruction would go
against the original goal of SFrame consisting in providing a quick and
simple stacktracing mechanism only relying on the minimal amount of
informations stored in an FRE.
Would it make sense to store an additional information (the address of
signing) in the FRE for this use-case ?
>>> I am a bit curious now to know how will the subsequent patches use
>>> the refactoring. :)
>>
>> If we forget about the boiler plates related to the new directive, the
>> interesting pieces of the future patch interacting with the enum are:
>>
>> --- a/gas/gen-sframe.h
>> +++ b/gas/gen-sframe.h
>> @@ -33,8 +33,9 @@
>>
>> typedef enum ATTRIBUTE_PACKED
>> {
>> - ra_no_signing = 0x0,
>> - ra_signing_sp = 0x1,
>> + ra_no_signing = 0x0,
>> + ra_signing_sp = 0x1,
>> + ra_signing_sp_pc = 0x3,
>> } ra_signing_method_t;
>>
>> --- a/gas/gen-sframe.c
>> +++ b/gas/gen-sframe.c
>> @@ -1307,6 +1307,26 @@ sframe_xlate_do_gnu_window_save (struct
>> sframe_xlate_ctx *xlate_ctx,
>> return SFRAME_XLATE_ERR_NOTREPRESENTED; /* Not represented. */
>> }
>>
>> +/* Translate DW_CFA_AARCH64_negate_ra_state_with_pc into SFrame context.
>> + Return SFRAME_XLATE_OK if success. */
>> +
>> +static int
>> +sframe_xlate_do_aarch64_negate_ra_state_with_pc (struct
>> sframe_xlate_ctx *xlate_ctx,
>> + struct cfi_insn_data
>> *cfi_insn ATTRIBUTE_UNUSED)
>> +{
>> + struct sframe_row_entry *cur_fre = xlate_ctx->cur_fre;
>> +
>> + gas_assert (cur_fre);
>> + /* Toggle the mangled RA status bit. */
>> + cur_fre->ra_signing_info
>> + = ((cur_fre->ra_signing_info == ra_no_signing)
>> + ? ra_signing_sp_pc
>> + : ra_no_signing);
>> + cur_fre->merge_candidate = false;
>> +
>
> Till we are emitting SFrame V2, this function will need to just return
> SFRAME_XLATE_ERR_NOTREPRESENTED together after emitting a warning.
> Perhaps something like:
>
> as_warn (_("skipping SFrame FDE; .cfi_negate_ra_state_with_pc"));
> return SFRAME_XLATE_ERR_NOTREPRESENTED; /* Not represented. */
>
>> + return SFRAME_XLATE_OK;
>> +}
>> +
>> /* Returns the DWARF call frame instruction name or fake CFI name
>> for the
>> specified CFI opcode, or NULL if the value is not recognized. */
>>
>> --- a/include/sframe.h
>> +++ b/include/sframe.h
>> @@ -265,7 +265,7 @@ typedef struct sframe_fre_info
>>
>> /* Set the mangled_ra_p bit as indicated. */
>> #define SFRAME_V1_FRE_INFO_UPDATE_RA_SIGNING_INFO(ra_signing_info,
>> fre_info) \
>> - ((((ra_signing_info) & 0x1) << 7) | ((fre_info) & 0x7f))
>> + ((((ra_signing_info) & 0x3) << 7) | ((fre_info) & 0x7f))
>>
>
> fre_info is uint8_t. We do not have space to keep more than 1 + 7 bits
> of information here.
>
>> #define SFRAME_V1_FRE_CFA_BASE_REG_ID(data) ((data) & 0x1)
>>
>>
>> Please let me know if you have more questions.
>>
>> Regards,
>> Matthieu
>>
>>
>> [1]: https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/
>> aadwarf64.rst#dwarf-register-names
>>
>>>> ---
>>>> gas/gen-sframe.c | 22 ++++++++++++++--------
>>>> gas/gen-sframe.h | 17 ++++++++++++++---
>>>> include/sframe.h | 4 ++--
>>>> 3 files changed, 30 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>>>> index 626dc33b71d..565148d2842 100644
>>>> --- a/gas/gen-sframe.c
>>>> +++ b/gas/gen-sframe.c
>>>> @@ -254,11 +254,13 @@ static struct sframe_version_ops sframe_ver_ops;
>>>> static unsigned char
>>>> sframe_v1_set_fre_info (unsigned int base_reg, unsigned int
>>>> num_offsets,
>>>> - unsigned int offset_size, bool mangled_ra_p)
>>>> + unsigned int offset_size,
>>>> + ra_signing_method_t ra_signing_info)
>>>> {
>>>> unsigned char fre_info;
>>>> fre_info = SFRAME_V1_FRE_INFO (base_reg, num_offsets, offset_size);
>>>> - fre_info = SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P (mangled_ra_p,
>>>> fre_info);
>>>> + fre_info = SFRAME_V1_FRE_INFO_UPDATE_RA_SIGNING_INFO
>>>> (ra_signing_info,
>>>> + fre_info);
>>>> return fre_info;
>>>> }
>>>> @@ -292,10 +294,11 @@ sframe_set_version (uint32_t sframe_version
>>>> ATTRIBUTE_UNUSED)
>>>> static unsigned char
>>>> sframe_set_fre_info (unsigned int base_reg, unsigned int num_offsets,
>>>> - unsigned int offset_size, bool mangled_ra_p)
>>>> + unsigned int offset_size,
>>>> + ra_signing_method_t ra_signing_info)
>>>> {
>>>> return sframe_ver_ops.set_fre_info (base_reg, num_offsets,
>>>> - offset_size, mangled_ra_p);
>>>> + offset_size, ra_signing_info);
>>>> }
>>>> /* SFrame set func info. */
>>>> @@ -526,7 +529,7 @@ output_sframe_row_entry (symbolS *fde_start_addr,
>>>> fre_num_offsets = get_fre_num_offsets (sframe_fre);
>>>> fre_offset_size = sframe_get_fre_offset_size (sframe_fre);
>>>> fre_info = sframe_set_fre_info (fre_base_reg, fre_num_offsets,
>>>> - fre_offset_size, sframe_fre->mangled_ra_p);
>>>> + fre_offset_size, sframe_fre->ra_signing_info);
>>>> out_one (fre_info);
>>>> idx = sframe_fre_offset_func_map_index (fre_offset_size);
>>>> @@ -879,7 +882,7 @@ sframe_row_entry_new (void)
>>>> fre->merge_candidate = true;
>>>> /* Reset the mangled RA status bit to zero by default. We will
>>>> initialize it in
>>>> sframe_row_entry_initialize () with the sticky bit if set. */
>>>> - fre->mangled_ra_p = false;
>>>> + fre->ra_signing_info = ra_no_signing;
>>>> return fre;
>>>> }
>>>> @@ -927,7 +930,7 @@ sframe_row_entry_initialize (struct
>>>> sframe_row_entry *cur_fre,
>>>> cur_fre->ra_offset = prev_fre->ra_offset;
>>>> /* Treat RA mangling as a sticky bit. It retains its value
>>>> until another
>>>> .cfi_negate_ra_state is seen. */
>>>> - cur_fre->mangled_ra_p = prev_fre->mangled_ra_p;
>>>> + cur_fre->ra_signing_info = prev_fre->ra_signing_info;
>>>> }
>>>> /* Return SFrame register name for SP, FP, and RA, or NULL if
>>>> other. */
>>>> @@ -1276,7 +1279,10 @@ sframe_xlate_do_aarch64_negate_ra_state
>>>> (struct sframe_xlate_ctx *xlate_ctx,
>>>> gas_assert (cur_fre);
>>>> /* Toggle the mangled RA status bit. */
>>>> - cur_fre->mangled_ra_p = !cur_fre->mangled_ra_p;
>>>> + cur_fre->ra_signing_info
>>>> + = ((cur_fre->ra_signing_info == ra_no_signing)
>>>> + ? ra_signing_sp
>>>> + : ra_no_signing);
>>>> cur_fre->merge_candidate = false;
>>>> return SFRAME_XLATE_OK;
>>>> diff --git a/gas/gen-sframe.h b/gas/gen-sframe.h
>>>> index 8ed46dbb087..35cc01ff0be 100644
>>>> --- a/gas/gen-sframe.h
>>>> +++ b/gas/gen-sframe.h
>>>> @@ -21,11 +21,22 @@
>>>> #ifndef GENSFRAME_H
>>>> #define GENSFRAME_H
>>>> +#include "ansidecl.h"
>>>> +
>>>> #define SFRAME_FRE_ELEM_LOC_REG 0
>>>> #define SFRAME_FRE_ELEM_LOC_STACK 1
>>>> #define SFRAME_FRE_BASE_REG_INVAL ((unsigned int)-1)
>>>> +/* Signing method used for return address authentication.
>>>> + (AArch64 extension) */
>>>> +
>>>> +typedef enum ATTRIBUTE_PACKED
>>>> +{
>>>> + ra_no_signing = 0x0,
>>>> + ra_signing_sp = 0x1,
>>>> +} ra_signing_method_t;
>>>> +
>>>> /* SFrame Frame Row Entry (FRE).
>>>> A frame row entry is a slice of the frame and can be valid for
>>>> a set of
>>>> @@ -52,8 +63,8 @@ struct sframe_row_entry
>>>> on it. */
>>>> bool merge_candidate;
>>>> - /* Whether the return address is mangled with pauth code. */
>>>> - bool mangled_ra_p;
>>>> + /* Track the PAuth mangling information for RA. */
>>>> + ra_signing_method_t ra_signing_info;
>>>
>>> I think at the FRE level, we will continue to track whether RA is
>>> signed or not.
>>>
>>> But later on, when we add necessary bits in SFrame V3 to convey the
>>> signing method, we can keep the 'ra_signing_method_t ra_signing_info'
>>> in the sframe_xlate_ctx object and pass it to SFrame FDE at relevant
>>> points.
>>>
>>>> /* Track CFA base (architectural) register ID. */
>>>> unsigned int cfa_base_reg;
>>>> @@ -146,7 +157,7 @@ struct sframe_version_ops
>>>> unsigned char format_version; /* SFrame format version. */
>>>> /* set SFrame FRE info. */
>>>> unsigned char (*set_fre_info) (unsigned int, unsigned int,
>>>> unsigned int,
>>>> - bool);
>>>> + ra_signing_method_t);
>>>> /* set SFrame Func info. */
>>>> unsigned char (*set_func_info) (unsigned int, unsigned int,
>>>> unsigned int);
>>>> };
>>>> diff --git a/include/sframe.h b/include/sframe.h
>>>> index 347dbc73787..4e7092d0bbe 100644
>>>> --- a/include/sframe.h
>>>> +++ b/include/sframe.h
>>>> @@ -264,8 +264,8 @@ typedef struct sframe_fre_info
>>>> (((offset_num) & 0xf) << 1) | ((base_reg_id) & 0x1))
>>>> /* Set the mangled_ra_p bit as indicated. */
>>>> -#define SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P(mangled_ra_p,
>>>> fre_info) \
>>>> - ((((mangled_ra_p) & 0x1) << 7) | ((fre_info) & 0x7f))
>>>> +#define SFRAME_V1_FRE_INFO_UPDATE_RA_SIGNING_INFO(ra_signing_info,
>>>> fre_info) \
>>>> + ((((ra_signing_info) & 0x1) << 7) | ((fre_info) & 0x7f))
>>>
>>> We should not remove the SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P
>>> definition.
>>>
>>> The way I see it is that the signing method has two flavors now:
>>> - pauth (SP)
>>> - pauth_lr (SP + PC)
>>>
>>> Whether or not RA is mangled is still boolean. Also the signing
>>> method is chosen per function (correct ?), so we simply need to keep
>>> this information per SFrame FDE.
>>>
>>> Additionally, removing a macro may cause a consumer program of
>>> libsframe to run into build time failures.
>>>
>>>> #define SFRAME_V1_FRE_CFA_BASE_REG_ID(data) ((data) & 0x1)
>>>> #define SFRAME_V1_FRE_OFFSET_COUNT(data) (((data) >> 1) & 0xf)
>>>
>>
>
More information about the Binutils
mailing list