Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis Machado <luis.machado@arm.com>
To: Christina Schimpe <christina.schimpe@intel.com>,
	gdb-patches@sourceware.org
Cc: thiago.bauermann@linaro.org, eliz@gnu.org
Subject: Re: [PATCH v4 07/11] gdb: Handle shadow stack pointer register unwinding for amd64 linux.
Date: Thu, 19 Jun 2025 10:25:05 +0100	[thread overview]
Message-ID: <ddb2a7f7-776d-45b0-a464-df79d2bbd48f@arm.com> (raw)
In-Reply-To: <20250617121147.1956686-8-christina.schimpe@intel.com>

On 6/17/25 13:11, Christina Schimpe wrote:
> Unwind the $pl3_ssp register.
> We now have an updated value for the shadow stack pointer when
> moving up or down the frame level.  Note that $pl3_ssp can
> become unavailable when moving to a frame before the shadow
> stack enablement.  In the example below, shadow stack is enabled
> in the function 'call1'.  Thus, when moving to a frame level above
> the function, $pl3_ssp will become unavaiable.
> Following the restriction of the linux kernel, implement the unwinding
> for amd64 linux only.
> 
> Before this patch:
> ~~~
> Breakpoint 1, call2 (j=3) at sample.c:44
> 44	  return 42;
> (gdb) p $pl3_ssp
> $1 = (void *) 0x7ffff79ffff8
> (gdb) up
> 55	  call2 (3);
> (gdb) p $pl3_ssp
> $2 = (void *) 0x7ffff79ffff8
> (gdb) up
> 68	  call1 (43);
> (gdb) p $pl3_ssp
> $3 = (void *) 0x7ffff79ffff8
> ~~~
> 
> After this patch:
> ~~~
> Breakpoint 1, call2 (j=3) at sample.c:44
> 44	  return 42;
> (gdb) p $pl3_ssp
> $1 = (void *) 0x7ffff79ffff8
> (gdb) up
> 55	  call2 (3);
> (gdb) p $pl3_ssp
> $2 = (void *) 0x7ffff7a00000
> (gdb) up
> 68	  call1 (43i);
> (gdb) p $pl3_ssp
> $3 = <unavailable>
> ~~~
> 
> As we now have an updated value for each selected frame, the
> return command is now enabled for shadow stack enabled programs, too.
> 
> We therefore add a test for the return command and shadow stack support,
> and for an updated shadow stack pointer after a frame level change.
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> ---
>  gdb/amd64-linux-tdep.c                        | 84 ++++++++++++++++++
>  gdb/linux-tdep.c                              | 47 ++++++++++
>  gdb/linux-tdep.h                              |  7 ++
>  .../gdb.arch/amd64-shadow-stack-cmds.exp      | 88 +++++++++++++++++++
>  gdb/testsuite/gdb.arch/amd64-shadow-stack.c   | 13 +++
>  5 files changed, 239 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp
> 
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index d806d3cb1f7..9436f0b190c 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -47,6 +47,8 @@
>  #include "arch/amd64-linux-tdesc.h"
>  #include "inferior.h"
>  #include "x86-tdep.h"
> +#include "dwarf2/frame.h"
> +#include "frame-unwind.h"
>  
>  /* The syscall's XML filename for i386.  */
>  #define XML_SYSCALL_FILENAME_AMD64 "syscalls/amd64-linux.xml"
> @@ -1917,6 +1919,87 @@ amd64_linux_get_tls_dtv_addr (struct gdbarch *gdbarch, ptid_t ptid,
>    return dtv_addr;
>  }
>  
> +/* Return the number of bytes required to update the shadow stack pointer
> +   by one element.  For x32 the shadow stack elements are still 64-bit
> +   aligned.  Thus, gdbarch_addr_bit cannot be used to compute the new
> +   stack pointer.  */
> +
> +static inline int
> +amd64_linux_shadow_stack_element_size_aligned (gdbarch *gdbarch)
> +{
> +  const bfd_arch_info *binfo = gdbarch_bfd_arch_info (gdbarch);
> +  return (binfo->bits_per_word / binfo->bits_per_byte);
> +}
> +
> +
> +/* Implement shadow stack pointer unwinding. For each new shadow stack
> +   pointer check if its address is still in the shadow stack memory range.
> +   If it's outside the range set the returned value to unavailable,
> +   otherwise return a value containing the new shadow stack pointer.  */
> +
> +static value *
> +amd64_linux_dwarf2_prev_ssp (const frame_info_ptr &this_frame,
> +			     void **this_cache, int regnum)
> +{
> +  value *v = frame_unwind_got_register (this_frame, regnum, regnum);
> +  gdb_assert (v != nullptr);
> +
> +  gdbarch *gdbarch = get_frame_arch (this_frame);
> +
> +  if (v->entirely_available () && !v->optimized_out ())
> +    {
> +      int size = register_size (gdbarch, regnum);
> +      bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +      CORE_ADDR ssp = extract_unsigned_integer (v->contents_all ().data (),
> +						size, byte_order);
> +
> +      /* Starting with v6.6., the Linux kernel supports CET shadow stack.

Same typo as before, period after "v6.6".

> +	 Using /proc/PID/smaps we can only check if the current shadow
> +	 stack pointer SSP points to shadow stack memory.  Only if this is
> +	 the case a valid previous shadow stack pointer can be
> +	 calculated.  */
> +      std::pair<CORE_ADDR, CORE_ADDR> range;
> +      if (linux_address_in_shadow_stack_mem_range (ssp, &range))
> +	{
> +	  /* The shadow stack grows downwards.  To compute the previous
> +	     shadow stack pointer, we need to increment SSP.  */
> +	  CORE_ADDR new_ssp
> +	    = ssp + amd64_linux_shadow_stack_element_size_aligned (gdbarch);
> +
> +	  /* If NEW_SSP points to the end of or before (<=) the current
> +	     shadow stack memory range we consider NEW_SSP as valid (but
> +	     empty).  */

I couldn't quite understand the difference between the empty case and the
unavailable case. But maybe I just don't fully understand the feature.

Would it be possible to make the comment a bit more clear?

> +	  if (new_ssp <= range.second)
> +	    return frame_unwind_got_address (this_frame, regnum, new_ssp);
> +	}
> +    }
> +
> +  /* Return a value which is marked as unavailable in case we could not
> +     calculate a valid previous shadow stack pointer.  */
> +  value *retval
> +    = value::allocate_register (get_next_frame_sentinel_okay (this_frame),
> +				regnum, register_type (gdbarch, regnum));
> +  retval->mark_bytes_unavailable (0, retval->type ()->length ());
> +  return retval;
> +}
> +
> +/* Implement the "init_reg" dwarf2_frame_ops method.  */
> +
> +static void
> +amd64_init_reg (gdbarch *gdbarch, int regnum, dwarf2_frame_state_reg *reg,
> +		const frame_info_ptr &this_frame)
> +{
> +  if (regnum == gdbarch_pc_regnum (gdbarch))
> +    reg->how = DWARF2_FRAME_REG_RA;
> +  else if (regnum == gdbarch_sp_regnum (gdbarch))
> +    reg->how = DWARF2_FRAME_REG_CFA;
> +  else if (regnum == AMD64_PL3_SSP_REGNUM)
> +    {
> +      reg->how = DWARF2_FRAME_REG_FN;
> +      reg->loc.fn = amd64_linux_dwarf2_prev_ssp;
> +    }
> +}
> +
>  static void
>  amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
>  			    int num_disp_step_buffers)
> @@ -1974,6 +2057,7 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
>  
>    set_gdbarch_remove_non_address_bits_watchpoint
>      (gdbarch, amd64_linux_remove_non_address_bits_watchpoint);
> +  dwarf2_frame_set_init_reg (gdbarch, amd64_init_reg);
>  }
>  
>  static void

<...>

Otherwise the rest looks OK to me.

Reviewed-By: Luis Machado <luis.machado@arm.com>

  reply	other threads:[~2025-06-19  9:27 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-17 12:11 [PATCH v4 00/11] Add CET shadow stack support Christina Schimpe
2025-06-17 12:11 ` [PATCH v4 01/11] gdbserver: Add optional runtime register set type Christina Schimpe
2025-06-19  9:27   ` Luis Machado
2025-06-17 12:11 ` [PATCH v4 02/11] gdbserver: Add assert in x86_linux_read_description Christina Schimpe
2025-06-19  9:27   ` Luis Machado
2025-06-17 12:11 ` [PATCH v4 03/11] gdb: Sync up x86-gcc-cpuid.h with cpuid.h from gcc 14 branch Christina Schimpe
2025-06-17 18:12   ` Tom Tromey
2025-06-20 12:39     ` Schimpe, Christina
2025-06-17 12:11 ` [PATCH v4 04/11] gdb, gdbserver: Use xstate_bv for target description creation on x86 Christina Schimpe
2025-06-19  9:23   ` Luis Machado
2025-06-23 12:46     ` Schimpe, Christina
2025-06-23 12:56       ` Luis Machado
2025-06-24 13:46         ` Schimpe, Christina
2025-06-26 16:03           ` Luis Machado
2025-06-17 12:11 ` [PATCH v4 05/11] gdb, gdbserver: Add support of Intel shadow stack pointer register Christina Schimpe
2025-06-17 12:20   ` Eli Zaretskii
2025-06-19  9:24   ` Luis Machado
2025-06-23 13:05     ` Schimpe, Christina
2025-06-17 12:11 ` [PATCH v4 06/11] gdb: amd64 linux coredump support with shadow stack Christina Schimpe
2025-06-19  9:24   ` Luis Machado
2025-06-23 13:16     ` Schimpe, Christina
2025-06-17 12:11 ` [PATCH v4 07/11] gdb: Handle shadow stack pointer register unwinding for amd64 linux Christina Schimpe
2025-06-19  9:25   ` Luis Machado [this message]
2025-06-20  1:42     ` Thiago Jung Bauermann
2025-06-23 14:55       ` Schimpe, Christina
2025-06-23 23:26         ` Thiago Jung Bauermann
2025-06-23 15:00     ` Schimpe, Christina
2025-06-23 15:06       ` Luis Machado
2025-06-23 23:36         ` Thiago Jung Bauermann
2025-06-20  1:52   ` Thiago Jung Bauermann
2025-06-17 12:11 ` [PATCH v4 08/11] gdb, gdbarch: Enable inferior calls for shadow stack support Christina Schimpe
2025-06-19  9:25   ` Luis Machado
2025-06-23 17:49     ` Schimpe, Christina
2025-06-17 12:11 ` [PATCH v4 09/11] gdb: Implement amd64 linux shadow stack support for inferior calls Christina Schimpe
2025-06-17 12:21   ` Eli Zaretskii
2025-06-19  9:25   ` Luis Machado
2025-06-27 19:52     ` Schimpe, Christina
2025-06-28 10:38       ` Luis Machado
2025-06-28 20:03         ` Thiago Jung Bauermann
2025-06-28 21:05           ` Thiago Jung Bauermann
2025-06-17 12:11 ` [PATCH v4 10/11] gdb, gdbarch: Introduce gdbarch method to get the shadow stack pointer Christina Schimpe
2025-06-17 18:16   ` Tom Tromey
2025-06-20 12:59     ` Schimpe, Christina
2025-06-19  9:26   ` Luis Machado
2025-06-23 18:00     ` Schimpe, Christina
2025-06-17 12:11 ` [PATCH v4 11/11] gdb: Enable displaced stepping with shadow stack on amd64 linux Christina Schimpe
2025-06-17 12:22   ` Eli Zaretskii
2025-06-17 15:16     ` Schimpe, Christina
2025-06-19  9:26   ` Luis Machado
2025-06-23 18:24     ` Schimpe, Christina
2025-06-24  8:05       ` Luis Machado
2025-06-27 19:26         ` Schimpe, Christina
2025-06-28 10:35           ` 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=ddb2a7f7-776d-45b0-a464-df79d2bbd48f@arm.com \
    --to=luis.machado@arm.com \
    --cc=christina.schimpe@intel.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.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