public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Luis Machado <luis.machado@arm.com>
To: gdb-patches@sourceware.org
Cc: jhb@FreeBSD.org, thiago.bauermann@linaro.org
Subject: Re: [PATCH] [AArch64] Enable pointer authentication support for aarch64 bare metal/kernel mode addresses
Date: Thu, 5 Jan 2023 13:16:17 +0000	[thread overview]
Message-ID: <19302961-c418-a266-567c-d52b87f112f8@arm.com> (raw)
In-Reply-To: <20221216105722.1413765-1-luis.machado@arm.com>

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


  parent reply	other threads:[~2023-01-05 13:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-16 10:57 Luis Machado
2022-12-20  3:20 ` Thiago Jung Bauermann
2022-12-20  9:17   ` Luis Machado
2023-01-05 13:16 ` Luis Machado [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=19302961-c418-a266-567c-d52b87f112f8@arm.com \
    --to=luis.machado@arm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jhb@FreeBSD.org \
    --cc=thiago.bauermann@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).