RE: [PATCH 08/12] gdb: Handle shadow stack pointer register unwinding for amd64 linux.

["Schimpe, Christina" <christina.schimpe@intel.com>]

> -----Original Message-----
> From: Guinevere Larsen <guinevere@redhat.com>
> Sent: Thursday, January 30, 2025 5:14 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 08/12] gdb: Handle shadow stack pointer register unwinding
> for amd64 linux.
> 
> On 1/30/25 1:11 PM, Schimpe, Christina wrote:
> >> -----Original Message-----
> >> From: Guinevere Larsen <guinevere@redhat.com>
> >> Sent: Thursday, January 30, 2025 3:29 PM
> >> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> >> patches@sourceware.org
> >> Subject: Re: [PATCH 08/12] gdb: Handle shadow stack pointer register
> >> unwinding for amd64 linux.
> >>
> >> On 12/20/24 5:04 PM, Schimpe, Christina 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.
> >>> ---
> >>>    gdb/amd64-linux-tdep.c                        | 69 +++++++++++++++
> >>>    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, 224 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
> >>> 95f643b1217..895feac85e8 100644
> >>> --- a/gdb/amd64-linux-tdep.c
> >>> +++ b/gdb/amd64-linux-tdep.c
> >>> @@ -45,6 +45,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"
> >>> @@ -1873,6 +1875,72 @@
> >> amd64_linux_remove_non_address_bits_watchpoint (gdbarch *gdbarch,
> >>>      return (addr & amd64_linux_lam_untag_mask ());
> >>>    }
> >>>
> >>> +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.
> >>> +	 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.
> >>> +	     For x32 the shadow stack elements are still 64-bit aligned.
> >>> +	     Thus, we cannot use gdbarch_addr_bit to compute the new stack
> >>> +	     pointer.  */
> >>> +	  const bfd_arch_info *binfo = gdbarch_bfd_arch_info (gdbarch);
> >>> +	  const int bytes_per_word
> >>> +	    = (binfo->bits_per_word / binfo->bits_per_byte);
> >> In patch 10 of this series, you introduce
> >> amd64_linux_shadow_stack_element_size_aligned to simplify this
> >> calculation. is there any reason you didn't introduce it here?
> > Thanks a lot for looking at this.
> >
> > The reason is that at this state of the series I only had one
> > occurrence of this particular code line and its comment. To avoid
> > duplication I decided to make a small function for it in patch 10, but
> > before it seemed to introduce more overhead. Would that make sense to you?
> >
> This makes sense, but in that case I think it's better to just create this function at
> this point, so that code doesn't get created and deleted unnecessarily.

Mh right, that's also true. I will apply your feedback in the next version of this series.

Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928