public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
@ 2022-12-16 10:57 Luis Machado
  2022-12-20  3:20 ` Thiago Jung Bauermann
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Luis Machado @ 2022-12-16 10:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: jhb, thiago.bauermann

At the moment GDB only handles pointer authentication (pauth) for userspace
addresses and if we're debugging a Linux-hosted program.

The Linux Kernel can be configured to use pauth instructions for some
additional security hardening, but GDB doesn't handle this well.

To overcome this limitation, GDB needs a couple things:

1 - The target needs to advertise pauth support.
2 - The hook to remove non-address bits from a pointer needs to be registered
    in aarch64-tdep.c as opposed to aarch64-linux-tdep.c.

There is a patch for QEMU [1] that addresses the first point, and it makes
QEMU's gdbstub expose a couple more pauth mask registers, so overall we will
have up to 4 pauth masks (2 masks or 4 masks):

pauth_dmask
pauth_cmask
pauth_dmask_high
pauth_cmask_high

pauth_dmask and pauth_cmask are the masks used to remove pauth signatures
from userspace addresses. pauth_dmask_high and pauth_cmask_high masks are used
to remove pauth signatures from kernel addresses.

The second point is easily addressed by moving code around.

When debugging a Linux Kernel built with pauth with an unpatched GDB, we get
the following backtrace:

 #0  __fput (file=0xffff0000c17a6400) at /repos/linux/fs/file_table.c:296
 #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
 #2  0x30008000080ade30 [PAC] in ?? ()
 #3  0x30d48000080ade30 in ?? ()
 Backtrace stopped: previous frame identical to this frame (corrupt stack?)

With a patched GDB, we get something a lot more meaningful:

 #0  __fput (file=0xffff0000c1bcfa00) at /repos/linux/fs/file_table.c:296
 #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
 #2  0xffff8000080ade30 [PAC] in task_work_run () at /repos/linux/kernel/task_work.c:179
 #3  0xffff80000801db90 [PAC] in resume_user_mode_work (regs=0xffff80000a96beb0) at /repos/linux/include/linux/resume_user_mode.h:49
 #4  do_notify_resume (regs=regs@entry=0xffff80000a96beb0, thread_flags=4) at /repos/linux/arch/arm64/kernel/signal.c:1127
 #5  0xffff800008fb9974 [PAC] in prepare_exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:137
 #6  exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:142
 #7  el0_svc (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:638
 #8  0xffff800008fb9d34 [PAC] in el0t_64_sync_handler (regs=<optimized out>) at /repos/linux/arch/arm64/kernel/entry-common.c:655
 #9  0xffff800008011548 [PAC] in el0t_64_sync () at /repos/linux/arch/arm64/kernel/entry.S:586
 Backtrace stopped: Cannot access memory at address 0xffff80000a96c0c8

[1] https://gitlab.com/rth7680/qemu/-/commit/e440ce6de3e14bf19ee70935be9086c05359f07b
---
 gdb/aarch64-linux-tdep.c |  40 ---------------
 gdb/aarch64-tdep.c       | 103 ++++++++++++++++++++++++++++++++++-----
 gdb/aarch64-tdep.h       |   2 +
 gdb/arch/aarch64.h       |   6 +++
 4 files changed, 100 insertions(+), 51 deletions(-)

diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index 7e04bd9251f..1f091fbc540 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -1980,40 +1980,6 @@ aarch64_linux_decode_memtag_section (struct gdbarch *gdbarch,
   return tags;
 }
 
-/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
-   non address bits from a pointer value.  */
-
-static CORE_ADDR
-aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
-{
-  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
-
-  /* By default, we assume TBI and discard the top 8 bits plus the VA range
-     select bit (55).  */
-  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
-
-  if (tdep->has_pauth ())
-    {
-      /* Fetch the PAC masks.  These masks are per-process, so we can just
-	 fetch data from whatever thread we have at the moment.
-
-	 Also, we have both a code mask and a data mask.  For now they are the
-	 same, but this may change in the future.  */
-      struct regcache *regs = get_current_regcache ();
-      CORE_ADDR cmask, dmask;
-
-      if (regs->cooked_read (tdep->pauth_reg_base, &dmask) != REG_VALID)
-	dmask = mask;
-
-      if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) != REG_VALID)
-	cmask = mask;
-
-      mask |= aarch64_mask_from_pac_registers (cmask, dmask);
-    }
-
-  return aarch64_remove_top_bits (pointer, mask);
-}
-
 static void
 aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -2066,12 +2032,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Syscall record.  */
   tdep->aarch64_syscall_record = aarch64_linux_syscall_record;
 
-  /* The top byte of a user space address known as the "tag",
-     is ignored by the kernel and can be regarded as additional
-     data associated with the address.  */
-  set_gdbarch_remove_non_address_bits (gdbarch,
-				       aarch64_remove_non_address_bits);
-
   /* MTE-specific settings and hooks.  */
   if (tdep->has_mte ())
     {
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 024385a9fd8..d99c2b3e866 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -133,10 +133,14 @@ static const char *const aarch64_sve_register_names[] =
 
 static const char *const aarch64_pauth_register_names[] =
 {
-  /* Authentication mask for data pointer.  */
+  /* Authentication mask for data pointer, low half/user pointers.  */
   "pauth_dmask",
-  /* Authentication mask for code pointer.  */
-  "pauth_cmask"
+  /* Authentication mask for code pointer, low half/user pointers.  */
+  "pauth_cmask",
+  /* Authentication mask for data pointer, high half / kernel pointers.  */
+  "pauth_dmask_high",
+  /* Authentication mask for code pointer, high half / kernel pointers.  */
+  "pauth_cmask_high"
 };
 
 static const char *const aarch64_mte_register_names[] =
@@ -222,9 +226,19 @@ aarch64_frame_unmask_lr (aarch64_gdbarch_tdep *tdep,
       && frame_unwind_register_unsigned (this_frame,
 					 tdep->ra_sign_state_regnum))
     {
-      int cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
-      CORE_ADDR cmask = frame_unwind_register_unsigned (this_frame, cmask_num);
-      addr = addr & ~cmask;
+      /* VA range select (bit 55) tells us whether to use the low half masks
+	 or the high half masks.  */
+      int cmask_num;
+      if (tdep->pauth_reg_count > 2 && addr & VA_RANGE_SELECT_BIT_MASK)
+	cmask_num = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base);
+      else
+	cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
+
+      /* By default, we assume TBI and discard the top 8 bits plus the VA range
+	 select bit (55).  */
+      CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
+      mask |= frame_unwind_register_unsigned (this_frame, cmask_num);
+      addr = aarch64_remove_top_bits (addr, mask);
 
       /* Record in the frame that the link register required unmasking.  */
       set_frame_previous_pc_masked (this_frame);
@@ -1317,8 +1331,8 @@ aarch64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
 	  reg->loc.exp.len = 1;
 	  return;
 	}
-      else if (regnum == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base)
-	       || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base))
+      else if (regnum >= tdep->pauth_reg_base
+	       && regnum < tdep->pauth_reg_base + tdep->pauth_reg_count)
 	{
 	  reg->how = DWARF2_FRAME_REG_SAME_VALUE;
 	  return;
@@ -3492,8 +3506,8 @@ aarch64_cannot_store_register (struct gdbarch *gdbarch, int regnum)
     return 0;
 
   /* Pointer authentication registers are read-only.  */
-  return (regnum == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base)
-	  || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base));
+  return (regnum >= tdep->pauth_reg_base
+	  && regnum < tdep->pauth_reg_base + tdep->pauth_reg_count);
 }
 
 /* Implement the stack_frame_destroyed_p gdbarch method.  */
@@ -3521,6 +3535,51 @@ aarch64_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
   return streq (inst.opcode->name, "ret");
 }
 
+/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
+   non address bits from a pointer value.  */
+
+static CORE_ADDR
+aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
+{
+  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
+
+  /* By default, we assume TBI and discard the top 8 bits plus the VA range
+     select bit (55).  */
+  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
+
+  if (tdep->has_pauth ())
+    {
+      /* Fetch the PAC masks.  These masks are per-process, so we can just
+	 fetch data from whatever thread we have at the moment.
+
+	 Also, we have both a code mask and a data mask.  For now they are the
+	 same, but this may change in the future.  */
+      struct regcache *regs = get_current_regcache ();
+      CORE_ADDR cmask, dmask;
+      int dmask_regnum = AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base);
+      int cmask_regnum = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
+
+      /* If we have a kernel address and we have kernel-mode address mask
+	 registers, use those instead.  */
+      if (tdep->pauth_reg_count > 2
+	  && pointer & VA_RANGE_SELECT_BIT_MASK)
+	{
+	  dmask_regnum = AARCH64_PAUTH_DMASK_HIGH_REGNUM (tdep->pauth_reg_base);
+	  cmask_regnum = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base);
+	}
+
+      if (regs->cooked_read (dmask_regnum, &dmask) != REG_VALID)
+	dmask = mask;
+
+      if (regs->cooked_read (cmask_regnum, &cmask) != REG_VALID)
+	cmask = mask;
+
+      mask |= aarch64_mask_from_pac_registers (cmask, dmask);
+    }
+
+  return aarch64_remove_top_bits (pointer, mask);
+}
+
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
    architectures already created during this debugging session.
@@ -3659,19 +3718,35 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
     }
 
   /* Add the pauth registers.  */
+  int pauth_masks = 0;
   if (feature_pauth != NULL)
     {
       first_pauth_regnum = num_regs;
       ra_sign_state_offset = num_pseudo_regs;
+
+      /* Size of the expected register set with all 4 masks.  */
+      int set_size = ARRAY_SIZE (aarch64_pauth_register_names);
+
+      /* QEMU exposes a couple additional masks for the high half of the
+	 address.  We should either have 2 registers or 4 registers.  */
+      if (tdesc_unnumbered_register (feature_pauth,
+				     "pauth_dmask_high") == 0)
+	{
+	  /* We did not find pauth_dmask_high, assume we only have
+	     2 masks.  We are not dealing with QEMU/Emulators then.  */
+	  set_size -= 2;
+	}
+
       /* Validate the descriptor provides the mandatory PAUTH registers and
 	 allocate their numbers.  */
-      for (i = 0; i < ARRAY_SIZE (aarch64_pauth_register_names); i++)
+      for (i = 0; i < set_size; i++)
 	valid_p &= tdesc_numbered_register (feature_pauth, tdesc_data.get (),
 					    first_pauth_regnum + i,
 					    aarch64_pauth_register_names[i]);
 
       num_regs += i;
       num_pseudo_regs += 1;	/* Count RA_STATE pseudo register.  */
+      pauth_masks = set_size;
     }
 
   /* Add the MTE registers.  */
@@ -3706,6 +3781,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep->jb_elt_size = 8;
   tdep->vq = vq;
   tdep->pauth_reg_base = first_pauth_regnum;
+  tdep->pauth_reg_count = pauth_masks;
   tdep->ra_sign_state_regnum = -1;
   tdep->mte_reg_base = first_mte_regnum;
   tdep->tls_regnum_base = first_tls_regnum;
@@ -3822,6 +3898,11 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   if (tdep->has_pauth ())
     tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
 
+  /* Architecture hook to remove bits of a pointer that are not part of the
+     address, like memory tags (MTE) and pointer authentication signatures.  */
+  set_gdbarch_remove_non_address_bits (gdbarch,
+				       aarch64_remove_non_address_bits);
+
   /* Add standard register aliases.  */
   for (i = 0; i < ARRAY_SIZE (aarch64_register_aliases); i++)
     user_reg_add (gdbarch, aarch64_register_aliases[i].name,
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index ff94c0a23b0..9fbb2b510aa 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -94,6 +94,8 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base
   }
 
   int pauth_reg_base = 0;
+  /* Number of pauth masks.  */
+  int pauth_reg_count = 0;
   int ra_sign_state_regnum = 0;
 
   /* Returns true if the target supports pauth.  */
diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
index 68048198b4d..4eeb9f8aa2c 100644
--- a/gdb/arch/aarch64.h
+++ b/gdb/arch/aarch64.h
@@ -132,6 +132,12 @@ enum aarch64_regnum
 
 #define AARCH64_PAUTH_DMASK_REGNUM(pauth_reg_base) (pauth_reg_base)
 #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
+/* The high versions of these masks are used for bare metal/kernel-mode pointer
+   authentication support.  */
+#define AARCH64_PAUTH_DMASK_HIGH_REGNUM(pauth_reg_base) (pauth_reg_base + 2)
+#define AARCH64_PAUTH_CMASK_HIGH_REGNUM(pauth_reg_base) (pauth_reg_base + 3)
+
+/* This size is only meant for Linux, not bare metal.  QEMU exposes 4 masks.  */
 #define AARCH64_PAUTH_REGS_SIZE (16)
 
 #define AARCH64_X_REGS_NUM 31
-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
  2022-12-16 10:57 [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses Luis Machado
@ 2022-12-20  3:20 ` Thiago Jung Bauermann
  2022-12-20  9:17   ` Luis Machado
  2023-01-05 13:16 ` Luis Machado
  2023-02-21  9:11 ` Luis Machado
  2 siblings, 1 reply; 8+ messages in thread
From: Thiago Jung Bauermann @ 2022-12-20  3:20 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, jhb


Hello Luis,

Luis Machado <luis.machado@arm.com> writes:

> At the moment GDB only handles pointer authentication (pauth) for userspace
> addresses and if we're debugging a Linux-hosted program.
>
> The Linux Kernel can be configured to use pauth instructions for some
> additional security hardening, but GDB doesn't handle this well.
>
> To overcome this limitation, GDB needs a couple things:
>
> 1 - The target needs to advertise pauth support.
> 2 - The hook to remove non-address bits from a pointer needs to be registered
>     in aarch64-tdep.c as opposed to aarch64-linux-tdep.c.
>
> There is a patch for QEMU [1] that addresses the first point, and it makes
> QEMU's gdbstub expose a couple more pauth mask registers, so overall we will
> have up to 4 pauth masks (2 masks or 4 masks):
>
> pauth_dmask
> pauth_cmask
> pauth_dmask_high
> pauth_cmask_high
>
> pauth_dmask and pauth_cmask are the masks used to remove pauth signatures
> from userspace addresses. pauth_dmask_high and pauth_cmask_high masks are used
> to remove pauth signatures from kernel addresses.
>
> The second point is easily addressed by moving code around.
>
> When debugging a Linux Kernel built with pauth with an unpatched GDB, we get
> the following backtrace:
>
>  #0  __fput (file=0xffff0000c17a6400) at /repos/linux/fs/file_table.c:296
>  #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>  #2  0x30008000080ade30 [PAC] in ?? ()
>  #3  0x30d48000080ade30 in ?? ()
>  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>
> With a patched GDB, we get something a lot more meaningful:
>
>  #0  __fput (file=0xffff0000c1bcfa00) at /repos/linux/fs/file_table.c:296
>  #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>  #2  0xffff8000080ade30 [PAC] in task_work_run () at /repos/linux/kernel/task_work.c:179
>  #3  0xffff80000801db90 [PAC] in resume_user_mode_work (regs=0xffff80000a96beb0) at /repos/linux/include/linux/resume_user_mode.h:49
>  #4  do_notify_resume (regs=regs@entry=0xffff80000a96beb0, thread_flags=4) at /repos/linux/arch/arm64/kernel/signal.c:1127
>  #5  0xffff800008fb9974 [PAC] in prepare_exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:137
>  #6  exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:142
>  #7  el0_svc (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:638
>  #8  0xffff800008fb9d34 [PAC] in el0t_64_sync_handler (regs=<optimized out>) at /repos/linux/arch/arm64/kernel/entry-common.c:655
>  #9  0xffff800008011548 [PAC] in el0t_64_sync () at /repos/linux/arch/arm64/kernel/entry.S:586
>  Backtrace stopped: Cannot access memory at address 0xffff80000a96c0c8
>
> [1] https://gitlab.com/rth7680/qemu/-/commit/e440ce6de3e14bf19ee70935be9086c05359f07b
> ---
>  gdb/aarch64-linux-tdep.c |  40 ---------------
>  gdb/aarch64-tdep.c       | 103 ++++++++++++++++++++++++++++++++++-----
>  gdb/aarch64-tdep.h       |   2 +
>  gdb/arch/aarch64.h       |   6 +++
>  4 files changed, 100 insertions(+), 51 deletions(-)

I studied this patch and it looks good to me, so FWIW:

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

One question: is it possible to run the testsuite against QEMU bare
metal with pauth support? I would assume that there is at least one test
(probably a lot more?) that fails without this patch and passes with it.
Is that correct?

-- 
Thiago

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
  2022-12-20  3:20 ` Thiago Jung Bauermann
@ 2022-12-20  9:17   ` Luis Machado
  0 siblings, 0 replies; 8+ messages in thread
From: Luis Machado @ 2022-12-20  9:17 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: gdb-patches, jhb

On 12/20/22 03:20, Thiago Jung Bauermann wrote:
> 
> Hello Luis,
> 
> Luis Machado <luis.machado@arm.com> writes:
> 
>> At the moment GDB only handles pointer authentication (pauth) for userspace
>> addresses and if we're debugging a Linux-hosted program.
>>
>> The Linux Kernel can be configured to use pauth instructions for some
>> additional security hardening, but GDB doesn't handle this well.
>>
>> To overcome this limitation, GDB needs a couple things:
>>
>> 1 - The target needs to advertise pauth support.
>> 2 - The hook to remove non-address bits from a pointer needs to be registered
>>      in aarch64-tdep.c as opposed to aarch64-linux-tdep.c.
>>
>> There is a patch for QEMU [1] that addresses the first point, and it makes
>> QEMU's gdbstub expose a couple more pauth mask registers, so overall we will
>> have up to 4 pauth masks (2 masks or 4 masks):
>>
>> pauth_dmask
>> pauth_cmask
>> pauth_dmask_high
>> pauth_cmask_high
>>
>> pauth_dmask and pauth_cmask are the masks used to remove pauth signatures
>> from userspace addresses. pauth_dmask_high and pauth_cmask_high masks are used
>> to remove pauth signatures from kernel addresses.
>>
>> The second point is easily addressed by moving code around.
>>
>> When debugging a Linux Kernel built with pauth with an unpatched GDB, we get
>> the following backtrace:
>>
>>   #0  __fput (file=0xffff0000c17a6400) at /repos/linux/fs/file_table.c:296
>>   #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>>   #2  0x30008000080ade30 [PAC] in ?? ()
>>   #3  0x30d48000080ade30 in ?? ()
>>   Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>>
>> With a patched GDB, we get something a lot more meaningful:
>>
>>   #0  __fput (file=0xffff0000c1bcfa00) at /repos/linux/fs/file_table.c:296
>>   #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>>   #2  0xffff8000080ade30 [PAC] in task_work_run () at /repos/linux/kernel/task_work.c:179
>>   #3  0xffff80000801db90 [PAC] in resume_user_mode_work (regs=0xffff80000a96beb0) at /repos/linux/include/linux/resume_user_mode.h:49
>>   #4  do_notify_resume (regs=regs@entry=0xffff80000a96beb0, thread_flags=4) at /repos/linux/arch/arm64/kernel/signal.c:1127
>>   #5  0xffff800008fb9974 [PAC] in prepare_exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:137
>>   #6  exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:142
>>   #7  el0_svc (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:638
>>   #8  0xffff800008fb9d34 [PAC] in el0t_64_sync_handler (regs=<optimized out>) at /repos/linux/arch/arm64/kernel/entry-common.c:655
>>   #9  0xffff800008011548 [PAC] in el0t_64_sync () at /repos/linux/arch/arm64/kernel/entry.S:586
>>   Backtrace stopped: Cannot access memory at address 0xffff80000a96c0c8
>>
>> [1] https://gitlab.com/rth7680/qemu/-/commit/e440ce6de3e14bf19ee70935be9086c05359f07b
>> ---
>>   gdb/aarch64-linux-tdep.c |  40 ---------------
>>   gdb/aarch64-tdep.c       | 103 ++++++++++++++++++++++++++++++++++-----
>>   gdb/aarch64-tdep.h       |   2 +
>>   gdb/arch/aarch64.h       |   6 +++
>>   4 files changed, 100 insertions(+), 51 deletions(-)
> 
> I studied this patch and it looks good to me, so FWIW:
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 
> One question: is it possible to run the testsuite against QEMU bare
> metal with pauth support? I would assume that there is at least one test
> (probably a lot more?) that fails without this patch and passes with it.
> Is that correct?
> 

There isn't a specific test for this, but if you force pointer authentication in the compiler and the use of high
addresses (VA select bit 55 on), the regular GDB tests that do any sort of backtrace will fail without this patch.

Hardware watchpoints should also be affected by the pointer authentication bits.

The difficulty is setting up a gdb testsuite run against QEMU (or any other simulator). It would be nice to have that
though.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
  2022-12-16 10:57 [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses Luis Machado
  2022-12-20  3:20 ` Thiago Jung Bauermann
@ 2023-01-05 13:16 ` Luis Machado
  2023-01-18 18:39   ` John Baldwin
  2023-02-21  9:11 ` Luis Machado
  2 siblings, 1 reply; 8+ messages in thread
From: Luis Machado @ 2023-01-05 13:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: jhb, thiago.bauermann

Hi John,

Any thoughts on this patch from BSD's side?

On 12/16/22 10:57, Luis Machado via Gdb-patches wrote:
> At the moment GDB only handles pointer authentication (pauth) for userspace
> addresses and if we're debugging a Linux-hosted program.
> 
> The Linux Kernel can be configured to use pauth instructions for some
> additional security hardening, but GDB doesn't handle this well.
> 
> To overcome this limitation, GDB needs a couple things:
> 
> 1 - The target needs to advertise pauth support.
> 2 - The hook to remove non-address bits from a pointer needs to be registered
>      in aarch64-tdep.c as opposed to aarch64-linux-tdep.c.
> 
> There is a patch for QEMU [1] that addresses the first point, and it makes
> QEMU's gdbstub expose a couple more pauth mask registers, so overall we will
> have up to 4 pauth masks (2 masks or 4 masks):
> 
> pauth_dmask
> pauth_cmask
> pauth_dmask_high
> pauth_cmask_high
> 
> pauth_dmask and pauth_cmask are the masks used to remove pauth signatures
> from userspace addresses. pauth_dmask_high and pauth_cmask_high masks are used
> to remove pauth signatures from kernel addresses.
> 
> The second point is easily addressed by moving code around.
> 
> When debugging a Linux Kernel built with pauth with an unpatched GDB, we get
> the following backtrace:
> 
>   #0  __fput (file=0xffff0000c17a6400) at /repos/linux/fs/file_table.c:296
>   #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>   #2  0x30008000080ade30 [PAC] in ?? ()
>   #3  0x30d48000080ade30 in ?? ()
>   Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> 
> With a patched GDB, we get something a lot more meaningful:
> 
>   #0  __fput (file=0xffff0000c1bcfa00) at /repos/linux/fs/file_table.c:296
>   #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>   #2  0xffff8000080ade30 [PAC] in task_work_run () at /repos/linux/kernel/task_work.c:179
>   #3  0xffff80000801db90 [PAC] in resume_user_mode_work (regs=0xffff80000a96beb0) at /repos/linux/include/linux/resume_user_mode.h:49
>   #4  do_notify_resume (regs=regs@entry=0xffff80000a96beb0, thread_flags=4) at /repos/linux/arch/arm64/kernel/signal.c:1127
>   #5  0xffff800008fb9974 [PAC] in prepare_exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:137
>   #6  exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:142
>   #7  el0_svc (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:638
>   #8  0xffff800008fb9d34 [PAC] in el0t_64_sync_handler (regs=<optimized out>) at /repos/linux/arch/arm64/kernel/entry-common.c:655
>   #9  0xffff800008011548 [PAC] in el0t_64_sync () at /repos/linux/arch/arm64/kernel/entry.S:586
>   Backtrace stopped: Cannot access memory at address 0xffff80000a96c0c8
> 
> [1] https://gitlab.com/rth7680/qemu/-/commit/e440ce6de3e14bf19ee70935be9086c05359f07b
> ---
>   gdb/aarch64-linux-tdep.c |  40 ---------------
>   gdb/aarch64-tdep.c       | 103 ++++++++++++++++++++++++++++++++++-----
>   gdb/aarch64-tdep.h       |   2 +
>   gdb/arch/aarch64.h       |   6 +++
>   4 files changed, 100 insertions(+), 51 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 7e04bd9251f..1f091fbc540 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1980,40 +1980,6 @@ aarch64_linux_decode_memtag_section (struct gdbarch *gdbarch,
>     return tags;
>   }
>   
> -/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
> -   non address bits from a pointer value.  */
> -
> -static CORE_ADDR
> -aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> -{
> -  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> -
> -  /* By default, we assume TBI and discard the top 8 bits plus the VA range
> -     select bit (55).  */
> -  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> -
> -  if (tdep->has_pauth ())
> -    {
> -      /* Fetch the PAC masks.  These masks are per-process, so we can just
> -	 fetch data from whatever thread we have at the moment.
> -
> -	 Also, we have both a code mask and a data mask.  For now they are the
> -	 same, but this may change in the future.  */
> -      struct regcache *regs = get_current_regcache ();
> -      CORE_ADDR cmask, dmask;
> -
> -      if (regs->cooked_read (tdep->pauth_reg_base, &dmask) != REG_VALID)
> -	dmask = mask;
> -
> -      if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) != REG_VALID)
> -	cmask = mask;
> -
> -      mask |= aarch64_mask_from_pac_registers (cmask, dmask);
> -    }
> -
> -  return aarch64_remove_top_bits (pointer, mask);
> -}
> -
>   static void
>   aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>   {
> @@ -2066,12 +2032,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>     /* Syscall record.  */
>     tdep->aarch64_syscall_record = aarch64_linux_syscall_record;
>   
> -  /* The top byte of a user space address known as the "tag",
> -     is ignored by the kernel and can be regarded as additional
> -     data associated with the address.  */
> -  set_gdbarch_remove_non_address_bits (gdbarch,
> -				       aarch64_remove_non_address_bits);
> -
>     /* MTE-specific settings and hooks.  */
>     if (tdep->has_mte ())
>       {
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 024385a9fd8..d99c2b3e866 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -133,10 +133,14 @@ static const char *const aarch64_sve_register_names[] =
>   
>   static const char *const aarch64_pauth_register_names[] =
>   {
> -  /* Authentication mask for data pointer.  */
> +  /* Authentication mask for data pointer, low half/user pointers.  */
>     "pauth_dmask",
> -  /* Authentication mask for code pointer.  */
> -  "pauth_cmask"
> +  /* Authentication mask for code pointer, low half/user pointers.  */
> +  "pauth_cmask",
> +  /* Authentication mask for data pointer, high half / kernel pointers.  */
> +  "pauth_dmask_high",
> +  /* Authentication mask for code pointer, high half / kernel pointers.  */
> +  "pauth_cmask_high"
>   };
>   
>   static const char *const aarch64_mte_register_names[] =
> @@ -222,9 +226,19 @@ aarch64_frame_unmask_lr (aarch64_gdbarch_tdep *tdep,
>         && frame_unwind_register_unsigned (this_frame,
>   					 tdep->ra_sign_state_regnum))
>       {
> -      int cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
> -      CORE_ADDR cmask = frame_unwind_register_unsigned (this_frame, cmask_num);
> -      addr = addr & ~cmask;
> +      /* VA range select (bit 55) tells us whether to use the low half masks
> +	 or the high half masks.  */
> +      int cmask_num;
> +      if (tdep->pauth_reg_count > 2 && addr & VA_RANGE_SELECT_BIT_MASK)
> +	cmask_num = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base);
> +      else
> +	cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
> +
> +      /* By default, we assume TBI and discard the top 8 bits plus the VA range
> +	 select bit (55).  */
> +      CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> +      mask |= frame_unwind_register_unsigned (this_frame, cmask_num);
> +      addr = aarch64_remove_top_bits (addr, mask);
>   
>         /* Record in the frame that the link register required unmasking.  */
>         set_frame_previous_pc_masked (this_frame);
> @@ -1317,8 +1331,8 @@ aarch64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>   	  reg->loc.exp.len = 1;
>   	  return;
>   	}
> -      else if (regnum == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base)
> -	       || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base))
> +      else if (regnum >= tdep->pauth_reg_base
> +	       && regnum < tdep->pauth_reg_base + tdep->pauth_reg_count)
>   	{
>   	  reg->how = DWARF2_FRAME_REG_SAME_VALUE;
>   	  return;
> @@ -3492,8 +3506,8 @@ aarch64_cannot_store_register (struct gdbarch *gdbarch, int regnum)
>       return 0;
>   
>     /* Pointer authentication registers are read-only.  */
> -  return (regnum == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base)
> -	  || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base));
> +  return (regnum >= tdep->pauth_reg_base
> +	  && regnum < tdep->pauth_reg_base + tdep->pauth_reg_count);
>   }
>   
>   /* Implement the stack_frame_destroyed_p gdbarch method.  */
> @@ -3521,6 +3535,51 @@ aarch64_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
>     return streq (inst.opcode->name, "ret");
>   }
>   
> +/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
> +   non address bits from a pointer value.  */
> +
> +static CORE_ADDR
> +aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +{
> +  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> +
> +  /* By default, we assume TBI and discard the top 8 bits plus the VA range
> +     select bit (55).  */
> +  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> +
> +  if (tdep->has_pauth ())
> +    {
> +      /* Fetch the PAC masks.  These masks are per-process, so we can just
> +	 fetch data from whatever thread we have at the moment.
> +
> +	 Also, we have both a code mask and a data mask.  For now they are the
> +	 same, but this may change in the future.  */
> +      struct regcache *regs = get_current_regcache ();
> +      CORE_ADDR cmask, dmask;
> +      int dmask_regnum = AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base);
> +      int cmask_regnum = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
> +
> +      /* If we have a kernel address and we have kernel-mode address mask
> +	 registers, use those instead.  */
> +      if (tdep->pauth_reg_count > 2
> +	  && pointer & VA_RANGE_SELECT_BIT_MASK)
> +	{
> +	  dmask_regnum = AARCH64_PAUTH_DMASK_HIGH_REGNUM (tdep->pauth_reg_base);
> +	  cmask_regnum = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base);
> +	}
> +
> +      if (regs->cooked_read (dmask_regnum, &dmask) != REG_VALID)
> +	dmask = mask;
> +
> +      if (regs->cooked_read (cmask_regnum, &cmask) != REG_VALID)
> +	cmask = mask;
> +
> +      mask |= aarch64_mask_from_pac_registers (cmask, dmask);
> +    }
> +
> +  return aarch64_remove_top_bits (pointer, mask);
> +}
> +
>   /* Initialize the current architecture based on INFO.  If possible,
>      re-use an architecture from ARCHES, which is a list of
>      architectures already created during this debugging session.
> @@ -3659,19 +3718,35 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>       }
>   
>     /* Add the pauth registers.  */
> +  int pauth_masks = 0;
>     if (feature_pauth != NULL)
>       {
>         first_pauth_regnum = num_regs;
>         ra_sign_state_offset = num_pseudo_regs;
> +
> +      /* Size of the expected register set with all 4 masks.  */
> +      int set_size = ARRAY_SIZE (aarch64_pauth_register_names);
> +
> +      /* QEMU exposes a couple additional masks for the high half of the
> +	 address.  We should either have 2 registers or 4 registers.  */
> +      if (tdesc_unnumbered_register (feature_pauth,
> +				     "pauth_dmask_high") == 0)
> +	{
> +	  /* We did not find pauth_dmask_high, assume we only have
> +	     2 masks.  We are not dealing with QEMU/Emulators then.  */
> +	  set_size -= 2;
> +	}
> +
>         /* Validate the descriptor provides the mandatory PAUTH registers and
>   	 allocate their numbers.  */
> -      for (i = 0; i < ARRAY_SIZE (aarch64_pauth_register_names); i++)
> +      for (i = 0; i < set_size; i++)
>   	valid_p &= tdesc_numbered_register (feature_pauth, tdesc_data.get (),
>   					    first_pauth_regnum + i,
>   					    aarch64_pauth_register_names[i]);
>   
>         num_regs += i;
>         num_pseudo_regs += 1;	/* Count RA_STATE pseudo register.  */
> +      pauth_masks = set_size;
>       }
>   
>     /* Add the MTE registers.  */
> @@ -3706,6 +3781,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     tdep->jb_elt_size = 8;
>     tdep->vq = vq;
>     tdep->pauth_reg_base = first_pauth_regnum;
> +  tdep->pauth_reg_count = pauth_masks;
>     tdep->ra_sign_state_regnum = -1;
>     tdep->mte_reg_base = first_mte_regnum;
>     tdep->tls_regnum_base = first_tls_regnum;
> @@ -3822,6 +3898,11 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     if (tdep->has_pauth ())
>       tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
>   
> +  /* Architecture hook to remove bits of a pointer that are not part of the
> +     address, like memory tags (MTE) and pointer authentication signatures.  */
> +  set_gdbarch_remove_non_address_bits (gdbarch,
> +				       aarch64_remove_non_address_bits);
> +
>     /* Add standard register aliases.  */
>     for (i = 0; i < ARRAY_SIZE (aarch64_register_aliases); i++)
>       user_reg_add (gdbarch, aarch64_register_aliases[i].name,
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index ff94c0a23b0..9fbb2b510aa 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -94,6 +94,8 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base
>     }
>   
>     int pauth_reg_base = 0;
> +  /* Number of pauth masks.  */
> +  int pauth_reg_count = 0;
>     int ra_sign_state_regnum = 0;
>   
>     /* Returns true if the target supports pauth.  */
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index 68048198b4d..4eeb9f8aa2c 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -132,6 +132,12 @@ enum aarch64_regnum
>   
>   #define AARCH64_PAUTH_DMASK_REGNUM(pauth_reg_base) (pauth_reg_base)
>   #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
> +/* The high versions of these masks are used for bare metal/kernel-mode pointer
> +   authentication support.  */
> +#define AARCH64_PAUTH_DMASK_HIGH_REGNUM(pauth_reg_base) (pauth_reg_base + 2)
> +#define AARCH64_PAUTH_CMASK_HIGH_REGNUM(pauth_reg_base) (pauth_reg_base + 3)
> +
> +/* This size is only meant for Linux, not bare metal.  QEMU exposes 4 masks.  */
>   #define AARCH64_PAUTH_REGS_SIZE (16)
>   
>   #define AARCH64_X_REGS_NUM 31


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
  2023-01-05 13:16 ` Luis Machado
@ 2023-01-18 18:39   ` John Baldwin
  2023-01-19  9:34     ` Luis Machado
  0 siblings, 1 reply; 8+ messages in thread
From: John Baldwin @ 2023-01-18 18:39 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: thiago.bauermann

On 1/5/23 5:16 AM, Luis Machado wrote:
> Hi John,
> 
> Any thoughts on this patch from BSD's side?

Sorry, I missed the cc earlier.  FreeBSD does have support for the userland
registers that I haven't yet added for userland PAC and this seems to make
that easier as I will just need to add the registers to the tdesc when
present/supported.  However, one question I have is what does this do if the
registers aren't available and kernel addresses are used?  I have this use
case for existing FreeBSD/aarch64 kernel debugging.  The default version of
the gdbarch hook seems to always assume TBI and strip the upper bits, but
for kernel addresses what I kind of need is to sign-extend addresses based
on bit 55.  I do have a separate gdbarch for kernels vs userland programs
so it might just be I need a custom version of this gdbarch hook for the kernel
gdbarch?

-- 
John Baldwin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
  2023-01-18 18:39   ` John Baldwin
@ 2023-01-19  9:34     ` Luis Machado
  2023-01-19 18:35       ` John Baldwin
  0 siblings, 1 reply; 8+ messages in thread
From: Luis Machado @ 2023-01-19  9:34 UTC (permalink / raw)
  To: John Baldwin, gdb-patches; +Cc: thiago.bauermann

Hi,

On 1/18/23 18:39, John Baldwin wrote:
> On 1/5/23 5:16 AM, Luis Machado wrote:
>> Hi John,
>>
>> Any thoughts on this patch from BSD's side?
> 
> Sorry, I missed the cc earlier.  FreeBSD does have support for the userland
> registers that I haven't yet added for userland PAC and this seems to make
> that easier as I will just need to add the registers to the tdesc when
> present/supported.  However, one question I have is what does this do if the
> registers aren't available and kernel addresses are used?  I have this use
> case for existing FreeBSD/aarch64 kernel debugging.  The default version of
> the gdbarch hook seems to always assume TBI and strip the upper bits, but
> for kernel addresses what I kind of need is to sign-extend addresses based
> on bit 55.  I do have a separate gdbarch for kernels vs userland programs
> so it might just be I need a custom version of this gdbarch hook for the kernel
> gdbarch?> 

The default behavior in the absence of the PAC registers for the kernel addresses is to
assume TBI, remove the top bits and sign-extend based on bit 55.

This is done by aarch64_remove_top_bits.

I suppose that would work for you then.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
  2023-01-19  9:34     ` Luis Machado
@ 2023-01-19 18:35       ` John Baldwin
  0 siblings, 0 replies; 8+ messages in thread
From: John Baldwin @ 2023-01-19 18:35 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: thiago.bauermann

On 1/19/23 1:34 AM, Luis Machado wrote:
> Hi,
> 
> On 1/18/23 18:39, John Baldwin wrote:
>> On 1/5/23 5:16 AM, Luis Machado wrote:
>>> Hi John,
>>>
>>> Any thoughts on this patch from BSD's side?
>>
>> Sorry, I missed the cc earlier.  FreeBSD does have support for the userland
>> registers that I haven't yet added for userland PAC and this seems to make
>> that easier as I will just need to add the registers to the tdesc when
>> present/supported.  However, one question I have is what does this do if the
>> registers aren't available and kernel addresses are used?  I have this use
>> case for existing FreeBSD/aarch64 kernel debugging.  The default version of
>> the gdbarch hook seems to always assume TBI and strip the upper bits, but
>> for kernel addresses what I kind of need is to sign-extend addresses based
>> on bit 55.  I do have a separate gdbarch for kernels vs userland programs
>> so it might just be I need a custom version of this gdbarch hook for the kernel
>> gdbarch?>
> 
> The default behavior in the absence of the PAC registers for the kernel addresses is to
> assume TBI, remove the top bits and sign-extend based on bit 55.
> 
> This is done by aarch64_remove_top_bits.
> 
> I suppose that would work for you then.

That sounds perfect then, thanks!

-- 
John Baldwin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
  2022-12-16 10:57 [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses Luis Machado
  2022-12-20  3:20 ` Thiago Jung Bauermann
  2023-01-05 13:16 ` Luis Machado
@ 2023-02-21  9:11 ` Luis Machado
  2 siblings, 0 replies; 8+ messages in thread
From: Luis Machado @ 2023-02-21  9:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: jhb, thiago.bauermann

On 12/16/22 10:57, Luis Machado via Gdb-patches wrote:
> At the moment GDB only handles pointer authentication (pauth) for userspace
> addresses and if we're debugging a Linux-hosted program.
> 
> The Linux Kernel can be configured to use pauth instructions for some
> additional security hardening, but GDB doesn't handle this well.
> 
> To overcome this limitation, GDB needs a couple things:
> 
> 1 - The target needs to advertise pauth support.
> 2 - The hook to remove non-address bits from a pointer needs to be registered
>      in aarch64-tdep.c as opposed to aarch64-linux-tdep.c.
> 
> There is a patch for QEMU [1] that addresses the first point, and it makes
> QEMU's gdbstub expose a couple more pauth mask registers, so overall we will
> have up to 4 pauth masks (2 masks or 4 masks):
> 
> pauth_dmask
> pauth_cmask
> pauth_dmask_high
> pauth_cmask_high
> 
> pauth_dmask and pauth_cmask are the masks used to remove pauth signatures
> from userspace addresses. pauth_dmask_high and pauth_cmask_high masks are used
> to remove pauth signatures from kernel addresses.
> 
> The second point is easily addressed by moving code around.
> 
> When debugging a Linux Kernel built with pauth with an unpatched GDB, we get
> the following backtrace:
> 
>   #0  __fput (file=0xffff0000c17a6400) at /repos/linux/fs/file_table.c:296
>   #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>   #2  0x30008000080ade30 [PAC] in ?? ()
>   #3  0x30d48000080ade30 in ?? ()
>   Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> 
> With a patched GDB, we get something a lot more meaningful:
> 
>   #0  __fput (file=0xffff0000c1bcfa00) at /repos/linux/fs/file_table.c:296
>   #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
>   #2  0xffff8000080ade30 [PAC] in task_work_run () at /repos/linux/kernel/task_work.c:179
>   #3  0xffff80000801db90 [PAC] in resume_user_mode_work (regs=0xffff80000a96beb0) at /repos/linux/include/linux/resume_user_mode.h:49
>   #4  do_notify_resume (regs=regs@entry=0xffff80000a96beb0, thread_flags=4) at /repos/linux/arch/arm64/kernel/signal.c:1127
>   #5  0xffff800008fb9974 [PAC] in prepare_exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:137
>   #6  exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:142
>   #7  el0_svc (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:638
>   #8  0xffff800008fb9d34 [PAC] in el0t_64_sync_handler (regs=<optimized out>) at /repos/linux/arch/arm64/kernel/entry-common.c:655
>   #9  0xffff800008011548 [PAC] in el0t_64_sync () at /repos/linux/arch/arm64/kernel/entry.S:586
>   Backtrace stopped: Cannot access memory at address 0xffff80000a96c0c8
> 
> [1] https://gitlab.com/rth7680/qemu/-/commit/e440ce6de3e14bf19ee70935be9086c05359f07b
> ---
>   gdb/aarch64-linux-tdep.c |  40 ---------------
>   gdb/aarch64-tdep.c       | 103 ++++++++++++++++++++++++++++++++++-----
>   gdb/aarch64-tdep.h       |   2 +
>   gdb/arch/aarch64.h       |   6 +++
>   4 files changed, 100 insertions(+), 51 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 7e04bd9251f..1f091fbc540 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1980,40 +1980,6 @@ aarch64_linux_decode_memtag_section (struct gdbarch *gdbarch,
>     return tags;
>   }
>   
> -/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
> -   non address bits from a pointer value.  */
> -
> -static CORE_ADDR
> -aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> -{
> -  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> -
> -  /* By default, we assume TBI and discard the top 8 bits plus the VA range
> -     select bit (55).  */
> -  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> -
> -  if (tdep->has_pauth ())
> -    {
> -      /* Fetch the PAC masks.  These masks are per-process, so we can just
> -	 fetch data from whatever thread we have at the moment.
> -
> -	 Also, we have both a code mask and a data mask.  For now they are the
> -	 same, but this may change in the future.  */
> -      struct regcache *regs = get_current_regcache ();
> -      CORE_ADDR cmask, dmask;
> -
> -      if (regs->cooked_read (tdep->pauth_reg_base, &dmask) != REG_VALID)
> -	dmask = mask;
> -
> -      if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) != REG_VALID)
> -	cmask = mask;
> -
> -      mask |= aarch64_mask_from_pac_registers (cmask, dmask);
> -    }
> -
> -  return aarch64_remove_top_bits (pointer, mask);
> -}
> -
>   static void
>   aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>   {
> @@ -2066,12 +2032,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>     /* Syscall record.  */
>     tdep->aarch64_syscall_record = aarch64_linux_syscall_record;
>   
> -  /* The top byte of a user space address known as the "tag",
> -     is ignored by the kernel and can be regarded as additional
> -     data associated with the address.  */
> -  set_gdbarch_remove_non_address_bits (gdbarch,
> -				       aarch64_remove_non_address_bits);
> -
>     /* MTE-specific settings and hooks.  */
>     if (tdep->has_mte ())
>       {
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 024385a9fd8..d99c2b3e866 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -133,10 +133,14 @@ static const char *const aarch64_sve_register_names[] =
>   
>   static const char *const aarch64_pauth_register_names[] =
>   {
> -  /* Authentication mask for data pointer.  */
> +  /* Authentication mask for data pointer, low half/user pointers.  */
>     "pauth_dmask",
> -  /* Authentication mask for code pointer.  */
> -  "pauth_cmask"
> +  /* Authentication mask for code pointer, low half/user pointers.  */
> +  "pauth_cmask",
> +  /* Authentication mask for data pointer, high half / kernel pointers.  */
> +  "pauth_dmask_high",
> +  /* Authentication mask for code pointer, high half / kernel pointers.  */
> +  "pauth_cmask_high"
>   };
>   
>   static const char *const aarch64_mte_register_names[] =
> @@ -222,9 +226,19 @@ aarch64_frame_unmask_lr (aarch64_gdbarch_tdep *tdep,
>         && frame_unwind_register_unsigned (this_frame,
>   					 tdep->ra_sign_state_regnum))
>       {
> -      int cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
> -      CORE_ADDR cmask = frame_unwind_register_unsigned (this_frame, cmask_num);
> -      addr = addr & ~cmask;
> +      /* VA range select (bit 55) tells us whether to use the low half masks
> +	 or the high half masks.  */
> +      int cmask_num;
> +      if (tdep->pauth_reg_count > 2 && addr & VA_RANGE_SELECT_BIT_MASK)
> +	cmask_num = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base);
> +      else
> +	cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
> +
> +      /* By default, we assume TBI and discard the top 8 bits plus the VA range
> +	 select bit (55).  */
> +      CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> +      mask |= frame_unwind_register_unsigned (this_frame, cmask_num);
> +      addr = aarch64_remove_top_bits (addr, mask);
>   
>         /* Record in the frame that the link register required unmasking.  */
>         set_frame_previous_pc_masked (this_frame);
> @@ -1317,8 +1331,8 @@ aarch64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>   	  reg->loc.exp.len = 1;
>   	  return;
>   	}
> -      else if (regnum == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base)
> -	       || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base))
> +      else if (regnum >= tdep->pauth_reg_base
> +	       && regnum < tdep->pauth_reg_base + tdep->pauth_reg_count)
>   	{
>   	  reg->how = DWARF2_FRAME_REG_SAME_VALUE;
>   	  return;
> @@ -3492,8 +3506,8 @@ aarch64_cannot_store_register (struct gdbarch *gdbarch, int regnum)
>       return 0;
>   
>     /* Pointer authentication registers are read-only.  */
> -  return (regnum == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base)
> -	  || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base));
> +  return (regnum >= tdep->pauth_reg_base
> +	  && regnum < tdep->pauth_reg_base + tdep->pauth_reg_count);
>   }
>   
>   /* Implement the stack_frame_destroyed_p gdbarch method.  */
> @@ -3521,6 +3535,51 @@ aarch64_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
>     return streq (inst.opcode->name, "ret");
>   }
>   
> +/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
> +   non address bits from a pointer value.  */
> +
> +static CORE_ADDR
> +aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +{
> +  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> +
> +  /* By default, we assume TBI and discard the top 8 bits plus the VA range
> +     select bit (55).  */
> +  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> +
> +  if (tdep->has_pauth ())
> +    {
> +      /* Fetch the PAC masks.  These masks are per-process, so we can just
> +	 fetch data from whatever thread we have at the moment.
> +
> +	 Also, we have both a code mask and a data mask.  For now they are the
> +	 same, but this may change in the future.  */
> +      struct regcache *regs = get_current_regcache ();
> +      CORE_ADDR cmask, dmask;
> +      int dmask_regnum = AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base);
> +      int cmask_regnum = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
> +
> +      /* If we have a kernel address and we have kernel-mode address mask
> +	 registers, use those instead.  */
> +      if (tdep->pauth_reg_count > 2
> +	  && pointer & VA_RANGE_SELECT_BIT_MASK)
> +	{
> +	  dmask_regnum = AARCH64_PAUTH_DMASK_HIGH_REGNUM (tdep->pauth_reg_base);
> +	  cmask_regnum = AARCH64_PAUTH_CMASK_HIGH_REGNUM (tdep->pauth_reg_base);
> +	}
> +
> +      if (regs->cooked_read (dmask_regnum, &dmask) != REG_VALID)
> +	dmask = mask;
> +
> +      if (regs->cooked_read (cmask_regnum, &cmask) != REG_VALID)
> +	cmask = mask;
> +
> +      mask |= aarch64_mask_from_pac_registers (cmask, dmask);
> +    }
> +
> +  return aarch64_remove_top_bits (pointer, mask);
> +}
> +
>   /* Initialize the current architecture based on INFO.  If possible,
>      re-use an architecture from ARCHES, which is a list of
>      architectures already created during this debugging session.
> @@ -3659,19 +3718,35 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>       }
>   
>     /* Add the pauth registers.  */
> +  int pauth_masks = 0;
>     if (feature_pauth != NULL)
>       {
>         first_pauth_regnum = num_regs;
>         ra_sign_state_offset = num_pseudo_regs;
> +
> +      /* Size of the expected register set with all 4 masks.  */
> +      int set_size = ARRAY_SIZE (aarch64_pauth_register_names);
> +
> +      /* QEMU exposes a couple additional masks for the high half of the
> +	 address.  We should either have 2 registers or 4 registers.  */
> +      if (tdesc_unnumbered_register (feature_pauth,
> +				     "pauth_dmask_high") == 0)
> +	{
> +	  /* We did not find pauth_dmask_high, assume we only have
> +	     2 masks.  We are not dealing with QEMU/Emulators then.  */
> +	  set_size -= 2;
> +	}
> +
>         /* Validate the descriptor provides the mandatory PAUTH registers and
>   	 allocate their numbers.  */
> -      for (i = 0; i < ARRAY_SIZE (aarch64_pauth_register_names); i++)
> +      for (i = 0; i < set_size; i++)
>   	valid_p &= tdesc_numbered_register (feature_pauth, tdesc_data.get (),
>   					    first_pauth_regnum + i,
>   					    aarch64_pauth_register_names[i]);
>   
>         num_regs += i;
>         num_pseudo_regs += 1;	/* Count RA_STATE pseudo register.  */
> +      pauth_masks = set_size;
>       }
>   
>     /* Add the MTE registers.  */
> @@ -3706,6 +3781,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     tdep->jb_elt_size = 8;
>     tdep->vq = vq;
>     tdep->pauth_reg_base = first_pauth_regnum;
> +  tdep->pauth_reg_count = pauth_masks;
>     tdep->ra_sign_state_regnum = -1;
>     tdep->mte_reg_base = first_mte_regnum;
>     tdep->tls_regnum_base = first_tls_regnum;
> @@ -3822,6 +3898,11 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     if (tdep->has_pauth ())
>       tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
>   
> +  /* Architecture hook to remove bits of a pointer that are not part of the
> +     address, like memory tags (MTE) and pointer authentication signatures.  */
> +  set_gdbarch_remove_non_address_bits (gdbarch,
> +				       aarch64_remove_non_address_bits);
> +
>     /* Add standard register aliases.  */
>     for (i = 0; i < ARRAY_SIZE (aarch64_register_aliases); i++)
>       user_reg_add (gdbarch, aarch64_register_aliases[i].name,
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index ff94c0a23b0..9fbb2b510aa 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -94,6 +94,8 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base
>     }
>   
>     int pauth_reg_base = 0;
> +  /* Number of pauth masks.  */
> +  int pauth_reg_count = 0;
>     int ra_sign_state_regnum = 0;
>   
>     /* Returns true if the target supports pauth.  */
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index 68048198b4d..4eeb9f8aa2c 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -132,6 +132,12 @@ enum aarch64_regnum
>   
>   #define AARCH64_PAUTH_DMASK_REGNUM(pauth_reg_base) (pauth_reg_base)
>   #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1)
> +/* The high versions of these masks are used for bare metal/kernel-mode pointer
> +   authentication support.  */
> +#define AARCH64_PAUTH_DMASK_HIGH_REGNUM(pauth_reg_base) (pauth_reg_base + 2)
> +#define AARCH64_PAUTH_CMASK_HIGH_REGNUM(pauth_reg_base) (pauth_reg_base + 3)
> +
> +/* This size is only meant for Linux, not bare metal.  QEMU exposes 4 masks.  */
>   #define AARCH64_PAUTH_REGS_SIZE (16)
>   
>   #define AARCH64_X_REGS_NUM 31

Pushed now.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-02-21  9:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-16 10:57 [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses Luis Machado
2022-12-20  3:20 ` Thiago Jung Bauermann
2022-12-20  9:17   ` Luis Machado
2023-01-05 13:16 ` Luis Machado
2023-01-18 18:39   ` John Baldwin
2023-01-19  9:34     ` Luis Machado
2023-01-19 18:35       ` John Baldwin
2023-02-21  9:11 ` Luis Machado

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).