[PATCH 0/4] Add AARCH64 pointer authentication support

[PATCH 0/4] Add AARCH64 pointer authentication support

[German Gomez]

Hi,

I've included a set of patches in order to demangle return addresses in
aarch64 platforms with pointer authentication.

Besides adding the implementation of the negate_ra_state opcode, there
is a new function in the libdwfl.h header to feed the PAC masks to the
library.

Let me know if there are any concerns with the current version.

Thanks,
German

German Gomez (4):
  aarch64: Create definitions for AARCH64_RA_SIGN_STATE register
  libdw,aarch64: Implement DW_CFA_AARCH64_negate_ra_state CFI
    instruction
  libdwfl,aarch64: Demangle return addresses using a PAC mask
  libdwfl,eu-stack,aarch64: Add API for setting AARCH64 PAC mask.

 backends/aarch64_init.c    |  6 +++---
 backends/aarch64_initreg.c |  2 ++
 backends/aarch64_regs.c    |  5 ++++-
 libdw/cfi.c                | 14 +++++++++++++-
 libdw/dwarf.h              |  5 +++++
 libdw/libdw.map            |  5 +++++
 libdwfl/dwfl_frame.c       |  3 +++
 libdwfl/dwfl_frame_regs.c  | 10 ++++++++++
 libdwfl/frame_unwind.c     | 14 +++++++++++++-
 libdwfl/libdwfl.h          |  6 ++++++
 libdwfl/libdwflP.h         |  7 +++++++
 libdwfl/linux-pid-attach.c | 34 ++++++++++++++++++++++++++++++++--
 tests/run-addrcfi.sh       |  1 +
 tests/run-allregs.sh       |  1 +
 14 files changed, 105 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/4] aarch64: Create definitions for AARCH64_RA_SIGN_STATE register

[German Gomez]

This register will be used to indicate whether a return address is
mangled with a PAC or not, in accordance with the DWARF AARCH64 ABI [1].

[1] https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst#41dwarf-register-names

Signed-off-by: German Gomez <german.gomez@arm.com>
---
 backends/aarch64_init.c    | 6 +++---
 backends/aarch64_initreg.c | 2 ++
 backends/aarch64_regs.c    | 5 ++++-
 libdw/dwarf.h              | 5 +++++
 tests/run-addrcfi.sh       | 1 +
 tests/run-allregs.sh       | 1 +
 6 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/backends/aarch64_init.c b/backends/aarch64_init.c
index bed92954..0a3a2c79 100644
--- a/backends/aarch64_init.c
+++ b/backends/aarch64_init.c
@@ -55,10 +55,10 @@ aarch64_init (Elf *elf __attribute__ ((unused)),
   HOOK (eh, data_marker_symbol);
   HOOK (eh, abi_cfi);
 
-  /* X0-X30 (31 regs) + SP + 1 Reserved + ELR, 30 Reserved regs (34-43)
+  /* X0-X30 (31 regs) + SP + 1 Reserved + ELR + RA_SIGN_STATE, 30 Reserved regs (34-43)
      + V0-V31 (32 regs, least significant 64 bits only)
-     + ALT_FRAME_RETURN_COLUMN (used when LR isn't used) = 97 DWARF regs. */
-  eh->frame_nregs = 97;
+     + ALT_FRAME_RETURN_COLUMN (used when LR isn't used) = 98 DWARF regs. */
+  eh->frame_nregs = 98;
   HOOK (eh, set_initial_registers_tid);
   HOOK (eh, unwind);
 
diff --git a/backends/aarch64_initreg.c b/backends/aarch64_initreg.c
index daf6f375..4661068a 100644
--- a/backends/aarch64_initreg.c
+++ b/backends/aarch64_initreg.c
@@ -73,6 +73,8 @@ aarch64_set_initial_registers_tid (pid_t tid __attribute__ ((unused)),
 
   /* ELR cannot be found.  */
 
+  /* RA_SIGN_STATE cannot be found */
+
   /* FP registers (only 64bits are used).  */
   struct user_fpsimd_struct fregs;
   iovec.iov_base = &fregs;
diff --git a/backends/aarch64_regs.c b/backends/aarch64_regs.c
index 23014bfc..e95ece37 100644
--- a/backends/aarch64_regs.c
+++ b/backends/aarch64_regs.c
@@ -87,7 +87,10 @@ aarch64_register_info (Ebl *ebl __attribute__ ((unused)),
     case 33:
       return regtype ("integer", DW_ATE_address, "elr");
 
-    case 34 ... 63:
+    case 34:
+      return regtype ("integer", DW_ATE_unsigned, "ra_sign_state");
+
+    case 35 ... 63:
       return 0;
 
     case 64 ... 95:
diff --git a/libdw/dwarf.h b/libdw/dwarf.h
index 3ce7f236..f234c411 100644
--- a/libdw/dwarf.h
+++ b/libdw/dwarf.h
@@ -1011,6 +1011,11 @@ enum
     DW_EH_PE_indirect = 0x80
   };
 
+/* AARCH64 DWARF registers. */
+enum
+  {
+    DW_AARCH64_RA_SIGN_STATE = 34
+  };
 
 /* DWARF XXX.  */
 #define DW_ADDR_none	0
diff --git a/tests/run-addrcfi.sh b/tests/run-addrcfi.sh
index 64fa24d7..ce9e753e 100755
--- a/tests/run-addrcfi.sh
+++ b/tests/run-addrcfi.sh
@@ -3639,6 +3639,7 @@ dwarf_cfi_addrframe (.eh_frame): no matching address range
 	integer reg30 (x30): same_value
 	integer reg31 (sp): location expression: call_frame_cfa stack_value
 	integer reg33 (elr): undefined
+	integer reg34 (ra_sign_state): undefined
 	FP/SIMD reg64 (v0): undefined
 	FP/SIMD reg65 (v1): undefined
 	FP/SIMD reg66 (v2): undefined
diff --git a/tests/run-allregs.sh b/tests/run-allregs.sh
index 87b16c95..ed086651 100755
--- a/tests/run-allregs.sh
+++ b/tests/run-allregs.sh
@@ -2693,6 +2693,7 @@ integer registers:
 	 30: x30 (x30), signed 64 bits
 	 31: sp (sp), address 64 bits
 	 33: elr (elr), address 64 bits
+	 34: ra_sign_state (ra_sign_state), unsigned 64 bits
 FP/SIMD registers:
 	 64: v0 (v0), unsigned 128 bits
 	 65: v1 (v1), unsigned 128 bits
-- 
2.25.1

Re: [PATCH 1/4] aarch64: Create definitions for AARCH64_RA_SIGN_STATE register

[Mark Wielaard]

Hi German,

On Mon, 2022-04-25 at 14:03 +0000, German Gomez via Elfutils-devel
wrote:
> This register will be used to indicate whether a return address is
> mangled with a PAC or not, in accordance with the DWARF AARCH64 ABI [1].
> 
> [1] https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst#41dwarf-register-names

This looks good. With one nit:

> diff --git a/libdw/dwarf.h b/libdw/dwarf.h
> index 3ce7f236..f234c411 100644
> --- a/libdw/dwarf.h
> +++ b/libdw/dwarf.h
> @@ -1011,6 +1011,11 @@ enum
>      DW_EH_PE_indirect = 0x80
>    };
>  
> +/* AARCH64 DWARF registers. */
> +enum
> +  {
> +    DW_AARCH64_RA_SIGN_STATE = 34
> +  };
>  
>  /* DWARF XXX.  */
>  #define DW_ADDR_none	0

dwarf.h is a public header. I rather not define this DW constant here.
Especially since it doesn't seem to have common name (binutils-gdb
calls this AARCH64_DWARF_RA_SIGN_STATE, but only defines it in a
private header). Lets move this into cfi.c as private implementation
detail in the next patch.

Thanks,

Mark

[PATCH 2/4] libdw, aarch64: Implement DW_CFA_AARCH64_negate_ra_state CFI instruction

[German Gomez]

Implement DW_CFA_AARCH64_negate_ra_state in accordance with the DWARF
AARCH64 ABI [1].

Followup commits will use the value of this register to remove the PAC
from return addresses.

[1] https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst#44call-frame-instructions

Signed-off-by: German Gomez <german.gomez@arm.com>
---
 libdw/cfi.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/libdw/cfi.c b/libdw/cfi.c
index a73fb03f..f985b4d8 100644
--- a/libdw/cfi.c
+++ b/libdw/cfi.c
@@ -125,6 +125,15 @@ execute_cfi (Dwarf_CFI *cache,
     fs->regs[regno].value = (r_value);			\
   } while (0)
 
+  /* The AARCH64 DWARF ABI states that register 34 (ra_sign_state) must
+     be initialized to 0. So do it before executing the CFI. */
+  if (cache->e_machine == EM_AARCH64)
+    {
+      if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs, &result)))
+	goto out;
+      fs->regs[DW_AARCH64_RA_SIGN_STATE].value = 0;
+    }
+
   while (program < end)
     {
       uint8_t opcode = *program++;
@@ -355,7 +364,10 @@ execute_cfi (Dwarf_CFI *cache,
 	    {
 	      /* Toggles the return address state, indicating whether
 		 the return address is encrypted or not on
-		 aarch64. XXX not handled yet.  */
+		 aarch64. */
+	      if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs, &result)))
+		goto out;
+	      fs->regs[DW_AARCH64_RA_SIGN_STATE].value ^= 0x1;
 	    }
 	  else
 	    {
-- 
2.25.1

Re: [PATCH 2/4] libdw, aarch64: Implement DW_CFA_AARCH64_negate_ra_state CFI instruction

[Mark Wielaard]

Hi German,

On Mon, 2022-04-25 at 14:03 +0000, German Gomez via Elfutils-devel
wrote:
> Implement DW_CFA_AARCH64_negate_ra_state in accordance with the DWARF
> AARCH64 ABI [1].
> 
> Followup commits will use the value of this register to remove the PAC
> from return addresses.
> 
> [1] https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst#44call-frame-instructions
> 
> Signed-off-by: German Gomez <german.gomez@arm.com>

This looks good, but two comments below.

> ---
>  libdw/cfi.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/libdw/cfi.c b/libdw/cfi.c
> index a73fb03f..f985b4d8 100644
> --- a/libdw/cfi.c
> +++ b/libdw/cfi.c
> @@ -125,6 +125,15 @@ execute_cfi (Dwarf_CFI *cache,
>      fs->regs[regno].value = (r_value);			\
>    } while (0)
>  
> +  /* The AARCH64 DWARF ABI states that register 34 (ra_sign_state) must
> +     be initialized to 0. So do it before executing the CFI. */
> +  if (cache->e_machine == EM_AARCH64)
> +    {
> +      if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs, &result)))
> +	goto out;
> +      fs->regs[DW_AARCH64_RA_SIGN_STATE].value = 0;
> +    }

Right. I thought this would be better expressed as part of the abi_cfi
(see aarch64_abi_cfi in backends/aarch64_cfi.c). But that would require
a DW_CFA_val_expression which we don't allow for abi_cfi. So this is
probably the best way to do it.

>    while (program < end)
>      {
>        uint8_t opcode = *program++;
> @@ -355,7 +364,10 @@ execute_cfi (Dwarf_CFI *cache,
>  	    {
>  	      /* Toggles the return address state, indicating whether
>  		 the return address is encrypted or not on
> -		 aarch64. XXX not handled yet.  */
> +		 aarch64. */
> +	      if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs, &result)))
> +		goto out;
> +	      fs->regs[DW_AARCH64_RA_SIGN_STATE].value ^= 0x1;
>  	    }
>  	  else
>  	    {

Looks good.

Lets also move the DW_AARCH64_RA_SIGN_STATE definition into cfi.h (from
libdw.h in the previous patch).

Thanks,

Mark

[PATCH 3/4] libdwfl, aarch64: Demangle return addresses using a PAC mask

[German Gomez]

Demangle mangled return addresses on AARCH64. The value of the masks is
stored in the struct Dwfl_Thread.

Signed-off-by: German Gomez <german.gomez@arm.com>
---
 libdwfl/dwfl_frame.c   |  3 +++
 libdwfl/frame_unwind.c | 14 +++++++++++++-
 libdwfl/libdwflP.h     |  7 +++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
index 77e0c5cb..88972d8e 100644
--- a/libdwfl/dwfl_frame.c
+++ b/libdwfl/dwfl_frame.c
@@ -270,6 +270,8 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
   thread.process = process;
   thread.unwound = NULL;
   thread.callbacks_arg = NULL;
+  thread.aarch64.pauth_insn_mask = 0;
+
   for (;;)
     {
       thread.tid = process->callbacks->next_thread (dwfl,
@@ -340,6 +342,7 @@ getthread (Dwfl *dwfl, pid_t tid,
       thread.process = process;
       thread.unwound = NULL;
       thread.callbacks_arg = NULL;
+      thread.aarch64.pauth_insn_mask = 0;
 
       if (process->callbacks->get_thread (dwfl, tid, process->callbacks_arg,
 					  &thread.callbacks_arg))
diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index 9ac33833..839d1eff 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -610,7 +610,19 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI *cfi, Dwarf_Addr bias)
 
       /* Some architectures encode some extra info in the return address.  */
       if (regno == frame->fde->cie->return_address_register)
-	regval &= ebl_func_addr_mask (ebl);
+	{
+	  regval &= ebl_func_addr_mask (ebl);
+
+	  /* In aarch64, pseudo-register RA_SIGN_STATE indicates whether the 
+	     return address needs demangling using the PAC mask from the
+	     thread. */
+	  if (cfi->e_machine == EM_AARCH64 &&
+	      frame->nregs > DW_AARCH64_RA_SIGN_STATE &&
+	      frame->regs[DW_AARCH64_RA_SIGN_STATE].value & 0x1)
+	    {
+	      regval &= ~(state->thread->aarch64.pauth_insn_mask);
+	    }
+	}
 
       /* This is another strange PPC[64] case.  There are two
 	 registers numbers that can represent the same DWARF return
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 7503a627..d3fe8118 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -244,6 +244,12 @@ struct Dwfl_Thread
   /* Bottom (innermost) frame while we're initializing, NULL afterwards.  */
   Dwfl_Frame *unwound;
   void *callbacks_arg;
+
+  /* Data for handling AARCH64 (currently limited to demangling PAC from
+     return addresses). */
+  struct {
+    Dwarf_Addr pauth_insn_mask;
+  } aarch64;
 };
 
 /* See its typedef in libdwfl.h.  */
@@ -782,6 +788,7 @@ INTDECL (dwfl_thread_tid)
 INTDECL (dwfl_frame_thread)
 INTDECL (dwfl_thread_state_registers)
 INTDECL (dwfl_thread_state_register_pc)
+INTDECL (dwfl_thread_state_aarch64_pauth)
 INTDECL (dwfl_getthread_frames)
 INTDECL (dwfl_getthreads)
 INTDECL (dwfl_thread_getframes)
-- 
2.25.1

[PATCH 4/4] libdwfl, eu-stack, aarch64: Add API for setting AARCH64 PAC mask.

[German Gomez]

Add user API for setting the PAC mask.
The function is intended to be called in Dwfl_Thread_Callbacks.set_initial_registers.

Testing notes:

 ... consider the following program.c

 | int a = 0;
 | void leaf(void) {
 |   for (;;)
 |     a += a;
 | }
 | void parent(void) {
 |   leaf();
 | }
 | int main(void) {
 |   parent();
 |   return 0;
 | }

 ... compiled with "gcc-10 -O0 -g -mbranch-protection=pac-ret+leaf program.c"
 ... should yield the correct call stack, without mangled addresses:

 | $ eu-stack -p <PID>
 |
 | PID 760267 - process
 | TID 760267:
 | #0  0x0000aaaaaebd0804 leaf
 | #1  0x0000aaaaaebd0818 parent
 | #2  0x0000aaaaaebd0838 main
 | #3  0x0000ffffbd52ad50 __libc_start_main
 | #4  0x0000aaaaaebd0694 $x

Signed-off-by: German Gomez <german.gomez@arm.com>
---
 libdw/libdw.map            |  5 +++++
 libdwfl/dwfl_frame_regs.c  | 10 ++++++++++
 libdwfl/libdwfl.h          |  6 ++++++
 libdwfl/linux-pid-attach.c | 34 ++++++++++++++++++++++++++++++++--
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/libdw/libdw.map b/libdw/libdw.map
index 4f530378..469c72ab 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -366,3 +366,8 @@ ELFUTILS_0.186 {
     dwarf_linecontext;
     dwarf_linefunctionname;
 } ELFUTILS_0.177;
+
+ELFUTILS_0.188 {
+  global:
+    dwfl_thread_state_aarch64_pauth;
+} ELFUTILS_0.186;
diff --git a/libdwfl/dwfl_frame_regs.c b/libdwfl/dwfl_frame_regs.c
index 83b1abef..f8baf6d3 100644
--- a/libdwfl/dwfl_frame_regs.c
+++ b/libdwfl/dwfl_frame_regs.c
@@ -59,3 +59,13 @@ dwfl_thread_state_register_pc (Dwfl_Thread *thread, Dwarf_Word pc)
   state->pc_state = DWFL_FRAME_STATE_PC_SET;
 }
 INTDEF(dwfl_thread_state_register_pc)
+
+void
+dwfl_thread_state_aarch64_pauth(Dwfl_Thread *thread, Dwarf_Word insn_mask)
+{
+  Dwfl_Frame *state = thread->unwound;
+  assert (state && state->unwound == NULL);
+  assert (state->initial_frame);
+  thread->aarch64.pauth_insn_mask = insn_mask;
+}
+INTDEF(dwfl_thread_state_aarch64_pauth)
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index f98f1d52..9eef703c 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -753,6 +753,12 @@ bool dwfl_thread_state_registers (Dwfl_Thread *thread, int firstreg,
 void dwfl_thread_state_register_pc (Dwfl_Thread *thread, Dwarf_Word pc)
   __nonnull_attribute__ (1);
 
+/* Called by Dwfl_Thread_Callbacks.set_initial_registers implementation.
+   On AARCH64 platforms with Pointer Authentication, the bits from this mask
+   indicate the position of the PAC bits in return addresses. */
+void dwfl_thread_state_aarch64_pauth (Dwfl_Thread *thread, Dwarf_Word insn_mask)
+  __nonnull_attribute__ (1);
+
 /* Iterate through the threads for a process.  Returns zero if all threads have
    been processed by the callback, returns -1 on error, or the value of the
    callback when not DWARF_CB_OK.  -1 returned on error will set dwfl_errno ().
diff --git a/libdwfl/linux-pid-attach.c b/libdwfl/linux-pid-attach.c
index 09cba07b..bf50a5bc 100644
--- a/libdwfl/linux-pid-attach.c
+++ b/libdwfl/linux-pid-attach.c
@@ -321,6 +321,28 @@ pid_thread_state_registers_cb (int firstreg, unsigned nregs,
   return INTUSE(dwfl_thread_state_registers) (thread, firstreg, nregs, regs);
 }
 
+#if defined(__aarch64__)
+
+#include <linux/ptrace.h> /* struct user_pac_mask */
+
+static void
+pid_set_aarch64_pauth(Dwfl_Thread *thread, pid_t tid)
+{
+  struct user_pac_mask pac_mask;
+  struct iovec iovec;
+
+  iovec.iov_base = &pac_mask;
+  iovec.iov_len = sizeof (pac_mask);
+
+  /* If the ptrace returns an error, the system may not support pointer
+     authentication. In that case, set the masks to 0 (no PAC bits). */
+  if (ptrace(PTRACE_GETREGSET, tid, NT_ARM_PAC_MASK, &iovec))
+    pac_mask.insn_mask = 0;
+
+  INTUSE(dwfl_thread_state_aarch64_pauth) (thread, pac_mask.insn_mask);
+}
+#endif /* __aarch64__ */
+
 static bool
 pid_set_initial_registers (Dwfl_Thread *thread, void *thread_arg)
 {
@@ -333,8 +355,16 @@ pid_set_initial_registers (Dwfl_Thread *thread, void *thread_arg)
   pid_arg->tid_attached = tid;
   Dwfl_Process *process = thread->process;
   Ebl *ebl = process->ebl;
-  return ebl_set_initial_registers_tid (ebl, tid,
-					pid_thread_state_registers_cb, thread);
+  if (!ebl_set_initial_registers_tid (ebl, tid,
+				      pid_thread_state_registers_cb, thread))
+    return false;
+
+#if defined(__aarch64__)
+  /* Set aarch64 pointer authentication data. */
+  pid_set_aarch64_pauth(thread, tid);
+#endif
+
+  return true;
 }
 
 static void
-- 
2.25.1

Re: [PATCH 0/4] Add AARCH64 pointer authentication support

[Mark Wielaard]

Hi German,

On Mon, Apr 25, 2022 at 02:03:07PM +0000, German Gomez via Elfutils-devel wrote:
> I've included a set of patches in order to demangle return addresses in
> aarch64 platforms with pointer authentication.
> 
> Besides adding the implementation of the negate_ra_state opcode, there
> is a new function in the libdwfl.h header to feed the PAC masks to the
> library.
> 
> Let me know if there are any concerns with the current version.

Thanks a lot for this. Last time I looked at this didn't have any
means to test this, so I skipped implementing it. How did you test? Do
distributions now enable PAC by default and is there hardware (qemu?)
support?

I haven't been able to look at the actual patches yet. And I am on
vacation this week. But I'll review next week after I am back.

A quick scan shows we need a aarch64 special public function, which
would be slightly ugly imho. I had hoped it could be a variant of the
func_addr_mask. But maybe this is too different to make more generic.

Cheers,

Mark

Re: [PATCH 0/4] Add AARCH64 pointer authentication support

[German Gomez]

Hi Mark, thanks for looking, and sorry for the delay

On 28/04/2022 20:56, Mark Wielaard wrote:
> Hi German,
>
> On Mon, Apr 25, 2022 at 02:03:07PM +0000, German Gomez via Elfutils-devel wrote:
>> I've included a set of patches in order to demangle return addresses in
>> aarch64 platforms with pointer authentication.
>>
>> Besides adding the implementation of the negate_ra_state opcode, there
>> is a new function in the libdwfl.h header to feed the PAC masks to the
>> library.
>>
>> Let me know if there are any concerns with the current version.
> Thanks a lot for this. Last time I looked at this didn't have any
> means to test this, so I skipped implementing it. How did you test? Do
> distributions now enable PAC by default and is there hardware (qemu?)
> support?

So far I've been testing on Graviton3 cores (running linux), which seem
to implement the PAC extension, and it came enabled by default.

https://www.kernel.org/doc/html/latest/arm64/pointer-authentication.html

> I haven't been able to look at the actual patches yet. And I am on
> vacation this week. But I'll review next week after I am back.

Thanks a lot for looking.

>
> A quick scan shows we need a aarch64 special public function, which
> would be slightly ugly imho. I had hoped it could be a variant of the
> func_addr_mask. But maybe this is too different to make more generic.

I did consider func_addr_mask initially, but when I wrote the patch it
wasn't exposed as a perf-thread value. Currently PAC masks are constant
but might be different from thread to thread in the future. So I placed
it in the Thread struct.

I agree the arch-specific naming is not pretty. I think I can certainly
rework it into a more generic feature. But I think I would need to make
sure that the masks can be supplied to the Thread struct before the   
unwind.

Thanks,
German

> Cheers,
>
> Mark
>

Re: [PATCH 0/4] Add AARCH64 pointer authentication support

[German Gomez]

On 19/05/2022 14:30, German Gomez via Elfutils-devel wrote:
> Hi Mark, thanks for looking, and sorry for the delay
>
> On 28/04/2022 20:56, Mark Wielaard wrote:
>> Hi German,
>>
>> On Mon, Apr 25, 2022 at 02:03:07PM +0000, German Gomez via Elfutils-devel wrote:
>>> [...]
>> Thanks a lot for this. Last time I looked at this didn't have any
>> means to test this, so I skipped implementing it. How did you test? Do
>> distributions now enable PAC by default and is there hardware (qemu?)
>> support?
> So far I've been testing on Graviton3 cores (running linux), which seem
> to implement the PAC extension, and it came enabled by default.
>

Regarding qemu support, I haven't used it myself so I can't really speak
for it, but according to the docs FEAT_PAuth is supported.

https://www.qemu.org/docs/master/system/arm/emulation.html

Re: [PATCH 0/4] Add AARCH64 pointer authentication support

[Mark Wielaard]

Hi German,

On Thu, May 19, 2022 at 02:30:23PM +0100, German Gomez via Elfutils-devel wrote:
> On 28/04/2022 20:56, Mark Wielaard wrote:
> > I haven't been able to look at the actual patches yet. And I am on
> > vacation this week. But I'll review next week after I am back.
> 
> Thanks a lot for looking.

And then it took a couple of months to actually review the patches,
sorry.

The first two patches look OK with one small issue where/how to
define DW_AARCH64_RA_SIGN_STATE (lets put it in cfi.h instead of
dwarf.h).

I have some concerns about the last two patches because they define
new public api that is aarch64 specific.

> > A quick scan shows we need a aarch64 special public function, which
> > would be slightly ugly imho. I had hoped it could be a variant of the
> > func_addr_mask. But maybe this is too different to make more generic.
> 
> I did consider func_addr_mask initially, but when I wrote the patch it
> wasn't exposed as a perf-thread value. Currently PAC masks are constant
> but might be different from thread to thread in the future. So I placed
> it in the Thread struct.

Aha. I assumed it was per process, but it makes sense if it is
per-thread.

> I agree the arch-specific naming is not pretty. I think I can certainly
> rework it into a more generic feature. But I think I would need to make
> sure that the masks can be supplied to the Thread struct before the   
> unwind.

Are there any other architectures with a similar "mask"?

Maybe we can make it a special dwfl_thread_state_registers call with
e.g. firstreg -1 and nregs 1? That way we don't need a new function,
just a "magic" negative register number.

Also we need to extract the pauth mask somehow in
libdwfl/linux-core-attach.c

Cheers,

Mark