[PATCH v1 1/1] aarch64: make RA signing method extendable for upcoming PAuth_LR

[Indu Bhagat]

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.

>> 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)
>>
>