RE: [PATCH v4 06/11] gdb: amd64 linux coredump support with shadow stack.

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

Hi Luis,

> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Thursday, June 19, 2025 11:25 AM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: thiago.bauermann@linaro.org; eliz@gnu.org
> Subject: Re: [PATCH v4 06/11] gdb: amd64 linux coredump support with shadow
> stack.
> 
> On 6/17/25 13:11, Christina Schimpe wrote:
> > From: Felix Willgerodt <felix.willgerodt@intel.com>
> >
> > Intel's Control-Flow Enforcement Technology (CET) provides the shadow
> > stack feature for the x86 architecture.
> >
> > This commit adds support to write and read the shadow-stack node in
> > corefiles.  This helps debugging return address violations post-mortem.
> > The format is synced with the linux kernel commit "x86: Add PTRACE
> > interface for shadow stack".  As the linux kernel restricts shadow
> > stack support to 64-bit, apply the fix for amd64 only.
> >
> > Co-Authored-By: Christina Schimpe <christina.schimpe@intel.com>
> >
> > Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> > ---
> >  gdb/amd64-linux-tdep.c                        | 57 +++++++++++++++++--
> >  .../gdb.arch/amd64-shadow-stack-corefile.exp  | 50 ++++++++++++++++
> >  2 files changed, 103 insertions(+), 4 deletions(-)  create mode
> > 100644 gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp
> >
> > diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c index
> > f23db4dce22..d806d3cb1f7 100644
> > --- a/gdb/amd64-linux-tdep.c
> > +++ b/gdb/amd64-linux-tdep.c
> > @@ -46,6 +46,7 @@
> >  #include "expop.h"
> >  #include "arch/amd64-linux-tdesc.h"
> >  #include "inferior.h"
> > +#include "x86-tdep.h"
> >
> >  /* The syscall's XML filename for i386.  */  #define
> > XML_SYSCALL_FILENAME_AMD64 "syscalls/amd64-linux.xml"
> > @@ -1592,6 +1593,14 @@ amd64_linux_record_signal (struct gdbarch
> *gdbarch,
> >    return 0;
> >  }
> >
> > +/* Get shadow stack pointer state from core dump.  */
> > +
> > +static bool
> > +amd64_linux_core_read_ssp_state_p (bfd *abfd) {
> > +  return bfd_get_section_by_name (abfd, ".reg-ssp") != NULL;
> 
> We're using nullptr nowadays.

Will fix.

> 
> > +}
> > +
> >  /* Get Linux/x86 target description from core dump.  */
> >
> >  static const struct target_desc *
> > @@ -1601,11 +1610,14 @@ amd64_linux_core_read_description (struct
> > gdbarch *gdbarch,  {
> >    /* Linux/x86-64.  */
> >    x86_xsave_layout layout;
> > -  uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
> > -  if (xcr0 == 0)
> > -    xcr0 = X86_XSTATE_SSE_MASK;
> > +  uint64_t xstate_bv_mask = i386_linux_core_read_xsave_info (abfd,
> > + layout);  if (xstate_bv_mask == 0)
> > +    xstate_bv_mask = X86_XSTATE_SSE_MASK;
> > +
> > +  if (amd64_linux_core_read_ssp_state_p (abfd))
> > +    xstate_bv_mask |= X86_XSTATE_CET_U;
> >
> > -  return amd64_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK,
> > +  return amd64_linux_read_description (xstate_bv_mask &
> > + X86_XSTATE_ALL_MASK,
> >  				       gdbarch_ptr_bit (gdbarch) == 32);  }
> >
> > @@ -1636,6 +1648,35 @@ static const struct regset
> amd64_linux_xstateregset =
> >      amd64_linux_collect_xstateregset
> >    };
> >
> > +/* Supply shadow stack pointer register from the buffer SSP to the
> > +   register cache REGCACHE.  */
> 
> s/from buffer SSP/from SSP? Otherwise it seems to read strangely.

Will fix.

> > +
> > +static void
> > +amd64_linux_supply_ssp (const regset *regset,
> > +			regcache *regcache, int regnum,
> > +			const void *ssp, size_t len)
> > +{
> > +  x86_supply_ssp (regcache, *static_cast<const uint64_t *> (ssp)); }
> > +
> > +/* Collect the shadow stack pointer register from the register cache
> > +   REGCACHE and store it in SSP.  */
> > +
> > +static void
> > +amd64_linux_collect_ssp (const regset *regset,
> > +			 const regcache *regcache, int regnum,
> > +			 void *ssp, size_t len)
> > +{
> > +  x86_collect_ssp (regcache, *static_cast<uint64_t *> (ssp)); }
> > +
> > +/* Shadow stack pointer register.  */
> > +
> > +static const struct regset amd64_linux_ssp_register
> > +  {
> > +    NULL, amd64_linux_supply_ssp, amd64_linux_collect_ssp
> > +  };
> > +
> >  /* Iterate over core file register note sections.  */
> >
> >  static void
> > @@ -1652,6 +1693,14 @@ amd64_linux_iterate_over_regset_sections (struct
> gdbarch *gdbarch,
> >      cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
> >  	tdep->xsave_layout.sizeof_xsave, &amd64_linux_xstateregset,
> >  	"XSAVE extended state", cb_data);
> > +
> > +  /* SSP can be unavailable.  Thus, we need to check the register status
> > +     in case we write a core file (regcache != nullptr).  */
> > +  if (tdep->ssp_regnum > 0
> > +      && (regcache == nullptr
> > +	  || REG_VALID == regcache->get_register_status (tdep->ssp_regnum)))
> > +    cb (".reg-ssp", 8, 8, &amd64_linux_ssp_register,
> > +	"shadow stack pointer", cb_data);
> >  }
> >
> >  /* The instruction sequences used in x86_64 machines for a diff --git
> > a/gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp
> > b/gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp
> > new file mode 100644
> > index 00000000000..25cc1529f0d
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp
> > @@ -0,0 +1,50 @@
> > +# Copyright 2021-2024 Free Software Foundation, Inc.
> > +
> > +# This program is free software; you can redistribute it and/or
> > +modify # it under the terms of the GNU General Public License as
> > +published by # the Free Software Foundation; either version 3 of the
> > +License, or # (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful, #
> > +but WITHOUT ANY WARRANTY; without even the implied warranty of #
> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the # GNU
> > +General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License #
> > +along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > +
> > +# Test the shadow stack pointer note in core dumps.
> > +
> > +require allow_ssp_tests
> > +
> > +standard_testfile amd64-shadow-stack.c set gcorefile ${binfile}.gcore
> > +
> > +save_vars { ::env(GLIBC_TUNABLES) } {
> > +
> > +    append_environment GLIBC_TUNABLES "glibc.cpu.hwcaps" "SHSTK"
> > +
> > +    if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> > +	  additional_flags="-fcf-protection=return"] } {
> > +	return -1
> > +    }
> > +
> > +    if { ![runto_main] } {
> > +	return -1
> > +    }
> > +
> > +    # Save ssp for comparison in the corefile session.
> > +    set ssp [get_hexadecimal_valueof "\$pl3_ssp" ""]
> > +
> > +    if { ![gdb_gcore_cmd $gcorefile "save a corefile"] } {
> > +	return -1
> > +    }
> > +
> > +    # Now restart gdb and load the corefile.
> > +    clean_restart ${binfile}
> > +
> > +    gdb_test "core ${gcorefile}" \
> > +	"Core was generated by .*" "re-load generated corefile"
> > +
> > +    gdb_test "print /x \$pl3_ssp" "= $ssp"
> > +}
> 
> The above test seems to exercise the case where a core file is generated by the
> gcore command.
> 
> Do we also need to exercise the case where the linux kernel dumps the core
> file? Like Thiago's
> AArch64 series for GCS?

Hm, good question -  I believe it's better to test this actually.
If I don't hear other feedback against it, I'll extend the test for v5.

Thanks,
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