RE: [PATCH v4 05/11] gdb, gdbserver: Add support of Intel shadow stack pointer register.

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

Hi Luis, 

Thanks a lot for the review.
I will fix your comments asap.

Christina

> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Thursday, June 19, 2025 11:24 AM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: thiago.bauermann@linaro.org; eliz@gnu.org
> Subject: Re: [PATCH v4 05/11] gdb, gdbserver: Add support of Intel shadow stack
> pointer register.
> 
> On 6/17/25 13:11, Christina Schimpe wrote:
> > This patch adds the user mode register PL3_SSP which is part of the
> > Intel(R) Control-Flow Enforcement Technology (CET) feature for support
> > of shadow stack.
> > For now, only native and remote debugging support for shadow stack
> > userspace on amd64 linux are covered by this patch including 64 bit
> > and
> > x32 support.  32 bit support is not covered due to missing Linux
> > kernel support.
> >
> > This patch requires fixing the test gdb.base/inline-frame-cycle-unwind
> > which is failing in case the shadow stack pointer is unavailable.
> > Such a state is possible if shadow stack is disabled for the current
> > thread but supported by HW.
> >
> > This test uses the Python unwinder inline-frame-cycle-unwind.py which
> > fakes the cyclic stack cycle by reading the pending frame's registers
> > and adding them to the unwinder:
> >
> > ~~~
> > for reg in pending_frame.architecture().registers("general"):
> >      val = pending_frame.read_register(reg)
> >      unwinder.add_saved_register(reg, val)
> >      return unwinder
> > ~~~
> >
> > However, in case the python unwinder is used we add a register
> > (pl3_ssp) that is unavailable.  This leads to a NOT_AVAILABLE_ERROR
> > caught in gdb/frame-unwind.c:frame_unwind_try_unwinder and it is
> > continued with standard unwinders.  This destroys the faked cyclic
> > behavior and the stack is further unwinded after frame 5.
> >
> > In the working scenario an error should be triggered:
> > ~~~
> > bt
> > 0  inline_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:49^M
> > 1  normal_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:32^M
> > 2  0x000055555555516e in inline_func () at
> > /tmp/gdb.base/inline-frame-cycle-unwind.c:45^M
> > 3  normal_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:32^M
> > 4  0x000055555555516e in inline_func () at
> > /tmp/gdb.base/inline-frame-cycle-unwind.c:45^M
> > 5  normal_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:32^M
> > Backtrace stopped: previous frame identical to this frame (corrupt
> > stack?)
> > (gdb) PASS: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 5:
> > backtrace when the unwind is broken at frame 5 ~~~
> >
> > To fix the Python unwinder, we simply skip the unavailable registers.
> >
> > Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> > Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> > ---
> >  gdb/NEWS                                      |  3 +
> >  gdb/amd64-linux-nat.c                         | 17 +++++
> >  gdb/amd64-linux-tdep.c                        |  1 +
> >  gdb/amd64-tdep.c                              |  6 +-
> >  gdb/amd64-tdep.h                              |  1 +
> >  gdb/arch/amd64.c                              | 10 +++
> >  gdb/arch/i386.c                               |  4 ++
> >  gdb/arch/x86-linux-tdesc-features.c           |  1 +
> >  gdb/doc/gdb.texinfo                           |  4 ++
> >  gdb/features/Makefile                         |  2 +
> >  gdb/features/i386/32bit-ssp.c                 | 14 ++++
> >  gdb/features/i386/32bit-ssp.xml               | 11 +++
> >  gdb/features/i386/64bit-ssp.c                 | 14 ++++
> >  gdb/features/i386/64bit-ssp.xml               | 11 +++
> >  gdb/i386-tdep.c                               | 22 +++++-
> >  gdb/i386-tdep.h                               |  4 ++
> >  gdb/nat/x86-linux-tdesc.c                     |  2 +
> >  gdb/nat/x86-linux.c                           | 55 +++++++++++++++
> >  gdb/nat/x86-linux.h                           |  4 ++
> >  gdb/testsuite/gdb.arch/amd64-shadow-stack.c   | 22 ++++++
> >  gdb/testsuite/gdb.arch/amd64-ssp.exp          | 50 +++++++++++++
> >  .../gdb.base/inline-frame-cycle-unwind.py     |  4 ++
> >  gdb/testsuite/lib/gdb.exp                     | 70 +++++++++++++++++++
> >  gdb/x86-linux-nat.c                           | 50 +++++++++++--
> >  gdb/x86-linux-nat.h                           | 11 +++
> >  gdb/x86-tdep.c                                | 21 ++++++
> >  gdb/x86-tdep.h                                |  9 +++
> >  gdbserver/linux-x86-low.cc                    | 28 +++++++-
> >  gdbsupport/x86-xstate.h                       |  5 +-
> >  29 files changed, 446 insertions(+), 10 deletions(-)  create mode
> > 100644 gdb/features/i386/32bit-ssp.c  create mode 100644
> > gdb/features/i386/32bit-ssp.xml  create mode 100644
> > gdb/features/i386/64bit-ssp.c  create mode 100644
> > gdb/features/i386/64bit-ssp.xml  create mode 100644
> > gdb/testsuite/gdb.arch/amd64-shadow-stack.c
> >  create mode 100644 gdb/testsuite/gdb.arch/amd64-ssp.exp
> >
> 
> <...>
> 
> > diff --git a/gdb/nat/x86-linux.c b/gdb/nat/x86-linux.c index
> > 0bdff736f8a..bf603182164 100644
> > --- a/gdb/nat/x86-linux.c
> > +++ b/gdb/nat/x86-linux.c
> > @@ -17,6 +17,12 @@
> >     You should have received a copy of the GNU General Public License
> >     along with this program.  If not, see
> > <http://www.gnu.org/licenses/>.  */
> >
> > +#include "elf/common.h"
> > +#include "gdbsupport/common-defs.h"
> > +#include "nat/gdb_ptrace.h"
> > +#include "nat/linux-ptrace.h"
> > +#include "nat/x86-cpuid.h"
> > +#include <sys/uio.h>
> >  #include "x86-linux.h"
> >  #include "x86-linux-dregs.h"
> >  #include "nat/gdb_ptrace.h"
> > @@ -126,3 +132,52 @@ x86_linux_ptrace_get_arch_size (int tid)
> >    return x86_linux_arch_size (false, false);  #endif  }
> > +
> > +bool
> > +x86_check_ssp_support (const int tid)
> 
> Missing the usual comment pointing at the header file?
> 
> /* See nat/x86-linux.h.  */
> 
> > +{
> > +  /* It's not enough to check shadow stack support with the ptrace call
> > +     below only, as we cannot distinguish between shadow stack not enabled
> > +     for the current thread and shadow stack is not supported by HW.  In
> > +     both scenarios the ptrace call fails with ENODEV.  In case shadow
> > +     stack is not enabled for the current thread, we still want to return
> > +     true.  */
> > +  unsigned int eax, ebx, ecx, edx;
> > +
> > +  __get_cpuid_count (7, 0, &eax, &ebx, &ecx, &edx);
> > +
> > +  if ((ecx & bit_SHSTK) == 0)
> > +    return false;
> > +
> > +  /* Further check for NT_X86_SHSTK kernel support.  */  uint64_t
> > + ssp;  iovec iov {&ssp, sizeof (ssp) };
> > +
> > +  errno = 0;
> > +  int res = ptrace (PTRACE_GETREGSET, tid, NT_X86_SHSTK, &iov);
> > +  if (res < 0)
> > +    {
> > +      if (errno == EINVAL)
> > +	{
> > +	  /* The errno EINVAL for a PTRACE_GETREGSET call indicates that
> > +	     kernel support is not available.  */
> > +	  return false;
> > +	}
> > +      else if (errno == ENODEV)
> > +	{
> > +	  /* At this point, since we already checked CPUID, the errno
> > +	     ENODEV for a PTRACE_GETREGSET call indicates that shadow
> > +	     stack is not enabled for the current thread.  As it could be
> > +	     enabled later, we still want to return true here.  */
> > +	  return true;
> > +	}
> > +      else
> > +	{
> > +	  warning (_("Unknown ptrace error for NT_X86_SHSTK: %s"),
> > +		   safe_strerror (errno));
> > +	  return false;
> > +	}
> > +    }
> > +
> > +  return true;
> > +}
> 
> <...>
> 
> > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> > index 3f1cd55d727..7e56cf61a4c 100644
> > --- a/gdb/testsuite/lib/gdb.exp
> > +++ b/gdb/testsuite/lib/gdb.exp
> > @@ -4287,6 +4287,76 @@ gdb_caching_proc allow_tsx_tests {} {
> >      return $allow_tsx_tests
> >  }
> >
> > +# Run a test on the target to check if it supports x86 shadow stack.
> > +Return 1 # if shadow stack is enabled, 0 otherwise.
> > +
> > +gdb_caching_proc allow_ssp_tests {} {
> > +    global srcdir subdir gdb_prompt hex
> > +
> > +    set me "allow_ssp_tests"
> > +
> > +    if { ![istarget i?86-*-*] && ![istarget x86_64-*-* ] } {
> > +	verbose "$me: target known to not support shadow stack."
> > +	return 0
> > +    }
> > +
> > +    # There is no need to check the actual HW in addition to ptrace support.
> > +    # We need both checks and ptrace will tell us about the HW state.
> > +    set compile_flags "{additional_flags=-fcf-protection=return}"
> > +    set src { int main() { return 0; } }
> > +    if {![gdb_simple_compile $me $src executable $compile_flags]} {
> > +	return 0
> > +    }
> > +
> > +    save_vars { ::env(GLIBC_TUNABLES) } {
> > +
> > +	append_environment GLIBC_TUNABLES "glibc.cpu.hwcaps" "SHSTK"
> > +
> > +	# No error message, compilation succeeded so now run it via gdb.
> > +	gdb_exit
> > +	gdb_start
> > +	gdb_reinitialize_dir $srcdir/$subdir
> > +	gdb_load $obj
> > +	if {![runto_main]} {
> > +	    remote_file build delete $obj
> > +	    return 0
> > +	}
> > +	set shadow_stack_disabled_re "(<unavailable>)"
> > +	if {[istarget *-*-linux*]} {
> > +	    # Starting with v6.6., the Linux kernel supports CET shadow stack.
> 
> Small typo, period after v6.6.
> 
> > +	    # Dependent on the target we can see a nullptr or "<unavailable>"
> > +	    # when shadow stack is supported by HW and the Linux kernel but
> > +	    # not enabled for the current thread (for example due to a lack
> > +	    # of compiler or glibc support for -fcf-protection).
> > +	    set shadow_stack_disabled_re "$shadow_stack_disabled_re|(.*0x0)"
> > +	}
> > +
> > +	set allow_ssp_tests 0
> > +	gdb_test_multiple "print \$pl3_ssp" "test shadow stack support" {
> > +	    -re -wrap "(.*$hex)((?!(.*0x0)).)" {
> > +		verbose -log "$me: Shadow stack support detected."
> > +		set allow_ssp_tests 1
> > +	    }
> > +	    -re -wrap $shadow_stack_disabled_re {
> > +		# In case shadow stack is not enabled (for example due to a
> > +		# lack of compiler or glibc support for -fcf-protection).
> > +		verbose -log "$me: Shadow stack is not enabled."
> > +	    }
> > +	    -re -wrap "void" {
> > +		# In case we don't have hardware or kernel support.
> > +		verbose -log "$me: No shadow stack support."
> > +	    }
> > +	}
> > +
> > +	gdb_exit
> > +    }
> > +
> > +    remote_file build delete $obj
> > +
> > +    verbose "$me: returning $allow_ssp_tests" 2
> > +    return $allow_ssp_tests
> > +}
> > +
> >  # Run a test on the target to see if it supports avx512bf16.  Return
> > 1 if so,  # 0 if it does not.  Based on 'check_vmx_hw_available' from the GCC
> testsuite.
> >
> > diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c index
> > a82ad21da27..865c017404e 100644
> > --- a/gdb/x86-linux-nat.c
> > +++ b/gdb/x86-linux-nat.c
> > @@ -41,6 +41,7 @@
> >  #include "nat/x86-linux.h"
> >  #include "nat/x86-linux-dregs.h"
> >  #include "nat/linux-ptrace.h"
> > +#include "x86-tdep.h"
> >  #include "nat/x86-linux-tdesc.h"
> >
> >  /* linux_nat_target::low_new_fork implementation.  */ @@ -97,11
> > +98,10 @@ const struct target_desc *
> > x86_linux_nat_target::read_description ()  {
> >    /* The x86_linux_tdesc_for_tid call only reads xcr0 the first time it is
> > -     called.  The mask is stored in XSTATE_BV_STORAGE and reused on
> > -     subsequent calls.  Note that GDB currently supports features for user
> > -     state components only.  However, once supervisor state components are
> > -     supported in GDB XSTATE_BV_STORAGE will not be configured based on
> > -     xcr0 only.  */
> > +     called.  Also it checks the enablement state of features which are
> > +     not configured in xcr0, such as CET shadow stack.  Once the supported
> > +     features are identified, the XSTATE_BV_STORAGE value is configured
> > +     accordingly and preserved for subsequent calls of this function.
> > + */
> >    static uint64_t xstate_bv_storage;
> >
> >    if (inferior_ptid == null_ptid)
> > @@ -215,6 +215,46 @@ x86_linux_get_thread_area (pid_t pid, void *addr,
> > unsigned int *base_addr)  }
> >
> 
> 
> >
> > +/* See x86-linux-nat.h.  */
> > +
> > +void
> > +x86_linux_fetch_ssp (regcache *regcache, const int tid) {
> > +  uint64_t ssp = 0x0;
> > +  iovec iov {&ssp, sizeof (ssp)};
> > +
> > +  /* The shadow stack may be enabled and disabled at runtime.  Reading the
> > +     ssp might fail as shadow stack was not activated for the current
> > +     thread.  We don't want to show a warning but silently return.  The
> > +     register will be shown as unavailable for the user.  */  if
> > + (ptrace (PTRACE_GETREGSET, tid, NT_X86_SHSTK, &iov) != 0)
> > +    return;
> > +
> > +  x86_supply_ssp (regcache, ssp);
> > +}
> > +
> > +/* See x86-linux-nat.h.  */
> > +
> > +void
> > +x86_linux_store_ssp (const regcache *regcache, const int tid) {
> > +  uint64_t ssp = 0x0;
> > +  iovec iov {&ssp, sizeof (ssp)};
> > +  x86_collect_ssp (regcache, ssp);
> > +
> > +  /* Starting with v6.6., the Linux kernel supports CET shadow stack.
> 
> Same typo, period after v6.6
> 
> > +     Dependent on the target the ssp register can be unavailable or
> > +     nullptr when shadow stack is supported by HW and the Linux kernel but
> > +     not enabled for the current thread.  In case of nullptr, GDB tries to
> > +     restore the shadow stack pointer after an inferior call.  The ptrace
> > +     call with PTRACE_SETREGSET will fail here with errno ENODEV.  We
> > +     don't want to throw an error in this case but silently continue.
> > +*/
> > +  errno = 0;
> > +  if ((ptrace (PTRACE_SETREGSET, tid, NT_X86_SHSTK, &iov) != 0)
> > +      && (errno != ENODEV))
> > +    perror_with_name (_("Failed to write pl3_ssp register")); }
> > +
> >  void _initialize_x86_linux_nat ();
> >  void
> >  _initialize_x86_linux_nat ()
> 
> <...>
> 
> Other than the cosmetic points above, the patch looks OK to me. I'm not
> entirely familiar with the feature for x86, but the enablement makes sense to
> me.
> 
> Reviewed-By: Luis Machado <luis.machado@arm.com>
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