[07/12] gdb, bfd: amd64 linux coredump support with shadow stack.

Message ID 20241220200501.324191-8-christina.schimpe@intel.com
State New
Headers
Series Add CET shadow stack support |

Commit Message

Schimpe, Christina Dec. 20, 2024, 8:04 p.m. UTC
  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>
---
 bfd/elf.c                                     | 24 +++++++++
 gdb/amd64-linux-tdep.c                        | 52 +++++++++++++++++--
 .../gdb.arch/amd64-shadow-stack-corefile.exp  | 50 ++++++++++++++++++
 3 files changed, 122 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp
  

Comments

Thiago Jung Bauermann Feb. 6, 2025, 3:15 a.m. UTC | #1
"Schimpe, Christina" <christina.schimpe@intel.com> writes:

> 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>
> ---
>  bfd/elf.c                                     | 24 +++++++++

I don't know what is current practice: should BFD changes be sent
separately to the binutils mailing list, or in the same patch or patch
series as the GDB changes that use them?

If the latter, then the binutils mailing list needs to at least be
on Cc:

>  gdb/amd64-linux-tdep.c                        | 52 +++++++++++++++++--
>  .../gdb.arch/amd64-shadow-stack-corefile.exp  | 50 ++++++++++++++++++
>  3 files changed, 122 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp

Just some minor comments.

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

> @@ -1630,6 +1642,30 @@ static const struct regset amd64_linux_xstateregset =
>      amd64_linux_collect_xstateregset
>    };
>
> +static void
> +amd64_linux_supply_ssp (const struct regset *regset,
> +			struct regcache *regcache, int regnum,
> +			const void *ssp, size_t len)
> +{
> +  x86_supply_ssp (regcache, *static_cast<const uint64_t *> (ssp));
> +}
> +
> +static void
> +amd64_linux_collect_ssp (const struct regset *regset,
> +			 const struct regcache *regcache, int regnum,
> +			 void *ssp, size_t len)
> +{
> +  x86_collect_ssp (regcache, *static_cast<uint64_t *> (ssp));
> +}

These functions should have documentation comments.

> +
> +/* Shadow stack pointer register.  */
> +
> +static const struct regset amd64_linux_ssp_register
> +  {
> +    NULL, amd64_linux_supply_ssp, amd64_linux_collect_ssp
> +  };
> +
> +

Just one line of separation between functions is enough.

>  /* Iterate over core file register note sections.  */
>
>  static void

--
Thiago
  
Schimpe, Christina Feb. 7, 2025, 11:54 a.m. UTC | #2
> -----Original Message-----
> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Sent: Thursday, February 6, 2025 4:15 AM
> To: Schimpe, Christina <christina.schimpe@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 07/12] gdb, bfd: amd64 linux coredump support with shadow
> stack.
> 
> 
> "Schimpe, Christina" <christina.schimpe@intel.com> writes:
> 
> > 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>
> > ---
> >  bfd/elf.c                                     | 24 +++++++++
> 
> I don't know what is current practice: should BFD changes be sent separately to
> the binutils mailing list, or in the same patch or patch series as the GDB changes
> that use them?
> 
> If the latter, then the binutils mailing list needs to at least be on Cc:

Right, I think I should post it in the binutils mailing list. Thanks for pointing that out.
I will fix your comments and post it then there.

Christina


> >  gdb/amd64-linux-tdep.c                        | 52 +++++++++++++++++--
> >  .../gdb.arch/amd64-shadow-stack-corefile.exp  | 50 ++++++++++++++++++
> >  3 files changed, 122 insertions(+), 4 deletions(-)  create mode
> > 100644 gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp
> 
> Just some minor comments.
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 
> > @@ -1630,6 +1642,30 @@ static const struct regset amd64_linux_xstateregset
> =
> >      amd64_linux_collect_xstateregset
> >    };
> >
> > +static void
> > +amd64_linux_supply_ssp (const struct regset *regset,
> > +			struct regcache *regcache, int regnum,
> > +			const void *ssp, size_t len)
> > +{
> > +  x86_supply_ssp (regcache, *static_cast<const uint64_t *> (ssp)); }
> > +
> > +static void
> > +amd64_linux_collect_ssp (const struct regset *regset,
> > +			 const struct regcache *regcache, int regnum,
> > +			 void *ssp, size_t len)
> > +{
> > +  x86_collect_ssp (regcache, *static_cast<uint64_t *> (ssp)); }
> 
> These functions should have documentation comments.
> 
> > +
> > +/* Shadow stack pointer register.  */
> > +
> > +static const struct regset amd64_linux_ssp_register
> > +  {
> > +    NULL, amd64_linux_supply_ssp, amd64_linux_collect_ssp
> > +  };
> > +
> > +
> 
> Just one line of separation between functions is enough.
> 
> >  /* Iterate over core file register note sections.  */
> >
> >  static void
> 
> --
> Thiago
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
  

Patch

diff --git a/bfd/elf.c b/bfd/elf.c
index 78394319bf0..23f1ebcd83c 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10365,6 +10365,12 @@  elfcore_grok_xstatereg (bfd *abfd, Elf_Internal_Note *note)
   return elfcore_make_note_pseudosection (abfd, ".reg-xstate", note);
 }
 
+static bool
+elfcore_grok_sspreg (bfd *abfd, Elf_Internal_Note *note)
+{
+  return elfcore_make_note_pseudosection (abfd, ".reg-ssp", note);
+}
+
 static bool
 elfcore_grok_ppc_vmx (bfd *abfd, Elf_Internal_Note *note)
 {
@@ -11060,6 +11066,13 @@  elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
       else
 	return true;
 
+    case NT_X86_SHSTK:		/* Linux CET extension.  */
+      if (note->namesz == 6
+	  && strcmp (note->namedata, "LINUX") == 0)
+	return elfcore_grok_sspreg (abfd, note);
+      else
+	return true;
+
     case NT_PPC_VMX:
       if (note->namesz == 6
 	  && strcmp (note->namedata, "LINUX") == 0)
@@ -12510,6 +12523,15 @@  elfcore_write_xstatereg (bfd *abfd, char *buf, int *bufsiz,
 			     note_name, NT_X86_XSTATE, xfpregs, size);
 }
 
+static char *
+elfcore_write_sspreg (bfd *abfd, char *buf, int *bufsiz,
+		      const void *ssp, int size)
+{
+  const char *note_name = "LINUX";
+  return elfcore_write_note (abfd, buf, bufsiz,
+			     note_name, NT_X86_SHSTK, ssp, size);
+}
+
 char *
 elfcore_write_x86_segbases (bfd *abfd, char *buf, int *bufsiz,
 			    const void *regs, int size)
@@ -13105,6 +13127,8 @@  elfcore_write_register_note (bfd *abfd,
     return elfcore_write_xstatereg (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-x86-segbases") == 0)
     return elfcore_write_x86_segbases (abfd, buf, bufsiz, data, size);
+  if (strcmp (section, ".reg-ssp") == 0)
+    return elfcore_write_sspreg (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-ppc-vmx") == 0)
     return elfcore_write_ppc_vmx (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-ppc-vsx") == 0)
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 8ed381e1a2c..95f643b1217 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -44,6 +44,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"
@@ -1586,6 +1587,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;
+}
+
 /* Get Linux/x86 target description from core dump.  */
 
 static const struct target_desc *
@@ -1595,11 +1604,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);
 }
 
@@ -1630,6 +1642,30 @@  static const struct regset amd64_linux_xstateregset =
     amd64_linux_collect_xstateregset
   };
 
+static void
+amd64_linux_supply_ssp (const struct regset *regset,
+			struct regcache *regcache, int regnum,
+			const void *ssp, size_t len)
+{
+  x86_supply_ssp (regcache, *static_cast<const uint64_t *> (ssp));
+}
+
+static void
+amd64_linux_collect_ssp (const struct regset *regset,
+			 const struct 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
@@ -1646,6 +1682,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"
+}