From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id v8YgMgXfiGi7TT8AWB0awg (envelope-from ) for ; Tue, 29 Jul 2025 10:47:33 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=SG9LtnGp; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id B9E5C1E102; Tue, 29 Jul 2025 10:47:33 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-10.1 required=5.0 tests=ARC_SIGNED,ARC_VALID, BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED, RCVD_IN_VALIDITY_RPBL,RCVD_IN_VALIDITY_SAFE autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 8737B1E091 for ; Tue, 29 Jul 2025 10:47:32 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id F00503858CD1 for ; Tue, 29 Jul 2025 14:47:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F00503858CD1 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=SG9LtnGp Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id F27D73858D37 for ; Tue, 29 Jul 2025 14:46:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F27D73858D37 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org F27D73858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753800408; cv=none; b=VxPpoFODI5hi6FH8zDQeqtzYlcp+M115K+jCyC1rddBlr/roI9wNpko04JtJgZuTx9Y6UuRwXByhxh5+LFYLzhUW35Lq9+DaR4V5Ov0tg3P/MaOBlcfUjjldYQf+u50BKuXoOkTmOEgwHXSiznVSEEo4TLEPaDXFyic8QWiMP5U= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753800408; c=relaxed/simple; bh=wdeMlV7eH9ABHJUsPC6K5Dggu2YPRmGEKv6BMADHWw0=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=eOrex19XZFfPGlhwO74tybBFOOvT2tmIPwqBh10OXB3a2cXFTG9KbTqcjI+g4921M9kjj9LQYr/Z+uPNGlyLkTyDC4mfJkc1FxLUs5kn5uOcWz3U5JYafF1IIavrOBqZdCSWUi5z/b1bmld7AEhoM1w/bNaCrI5+Wge6nH6hPEQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F27D73858D37 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1753800407; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=7/Pna5P4TWzGZRRSwKMviWjLFVnVUAQbgo9JSbzbPL8=; b=SG9LtnGpLcgJ7qzNvFSnBgU8hDaBTf2UjyayQsVTzD/U0JUmXSFcOD+sFXNRGVzzesB6gY 3ZDzadCuhnpaxx8hKfYq0OuNf2wtqrSTanbRH3+GcnZ/5iqQi2ZTycTLvGWZTPZouTD09x XrNT1bJJAjQ+eTEArSdLgbBNmiy4UhA= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-638-njv_h5hZOkKTYFQ-Ms_pzw-1; Tue, 29 Jul 2025 10:46:45 -0400 X-MC-Unique: njv_h5hZOkKTYFQ-Ms_pzw-1 X-Mimecast-MFC-AGG-ID: njv_h5hZOkKTYFQ-Ms_pzw_1753800405 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-451ecc3be97so26644035e9.0 for ; Tue, 29 Jul 2025 07:46:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753800404; x=1754405204; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=7/Pna5P4TWzGZRRSwKMviWjLFVnVUAQbgo9JSbzbPL8=; b=LxlxSDcM4hkKN0cH2QRe1Gvr5Q7z/Nfh2I+JPqCm22UgCkPBmznHL01s5BjMULgs+t oVemIJkP0zUdtBhmubETqYENRRaG0bz6vG5jtH4m98ESEvTzwMT/v/qh9L7gnDpj21Tm 2U0fFISq9n3skxd/8NRiDXP16WqBSZSA/bnAbm3s1ik5ZaptJhK9nXsuPq94LguKU0nD s/ICztndsYmsFGiTfSPy4ukKLqeJoNxJaNVxrW9z7P7rgt6yhfollRu3S1HtOu3agY9I PbJcjxXNZTHFSsb3QVIQq4hZa+yqD4gjRocZ8Lmt1bke9WLwx8eeYFsfeoEPkaBCEMTt QM1Q== X-Forwarded-Encrypted: i=1; AJvYcCUSxrPaas1u91yEhxQGpI3GbS4oy45kp2NRliagoh19r2Weum05/JlgxnFp+A5gonDuxsITOSjrD13t2Q==@sourceware.org X-Gm-Message-State: AOJu0Ywl8A81lqJWqpcRHym0Dh1QgFSNb2uBvaSJKdGyRgttWltSvQfC ldtG9Xjd6/Z3R2LNbNOKevU5Q5BTlnLatD3jtjU69iCVnc8ggsQ4qgmEOCAStxKLbEWi6p2ioh0 b7sbmnOVfROeOvngggeRf2ZRNepL4zKF2G+Y6+f2LqUmL0GW+UeTdD7NBaYZY3is= X-Gm-Gg: ASbGncvjmYON5GvfY9dgIX0qveRUHOULyZNI4FHF7HSzNJkTTM1XDhLAcU3K0v1aiTb nFttoLHJU7q3E59dEMpePhn+4EiRHJh/+3gG0YVuti09m+5evpaxw5tjiYzFV77oNffg8uLh5Zb ajNlFS6fi3uD6zkyE9J+ZeFWBw3ZsR2doUhZnJpYlYXK4bvVXqwOX6CMcP9EKnsFaQtF0c9dLsw EJneJ24Vt00R19XSx/sjTNZJIvL6Nl9FMV9kafA0eLBvHPeDVAO0gI6A5cHdF9Q0CQU9vu9ezI1 +HzSFh5SXC8pdGqbIAD3iqknHXmhn5bbU0u4/rChtkkqJnJOAyio+vP8dy9LoA== X-Received: by 2002:a05:600c:8b81:b0:456:eb9:5236 with SMTP id 5b1f17b1804b1-45892ba3686mr877265e9.15.1753800404377; Tue, 29 Jul 2025 07:46:44 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHfRhPEmCbhiK+2HsG75E04XLNlfJd9oFm3kymYVlNW0AYEWnFsEvb+/n5oOQmzzOcJQZqSxg== X-Received: by 2002:a05:600c:8b81:b0:456:eb9:5236 with SMTP id 5b1f17b1804b1-45892ba3686mr876985e9.15.1753800403799; Tue, 29 Jul 2025 07:46:43 -0700 (PDT) Received: from localhost (120.81.93.209.dyn.plus.net. [209.93.81.120]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-458705ce781sm199482955e9.31.2025.07.29.07.46.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Jul 2025 07:46:43 -0700 (PDT) From: Andrew Burgess To: Christina Schimpe , gdb-patches@sourceware.org Cc: thiago.bauermann@linaro.org, luis.machado@arm.com Subject: Re: [PATCH v5 07/12] gdb: amd64 linux coredump support with shadow stack. In-Reply-To: <20250628082810.332526-8-christina.schimpe@intel.com> References: <20250628082810.332526-1-christina.schimpe@intel.com> <20250628082810.332526-8-christina.schimpe@intel.com> Date: Tue, 29 Jul 2025 15:46:42 +0100 Message-ID: <87o6t3cawd.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: SJjVsq27hPQKDTUsTzU2bcjYxdRP2RlPdNVzWePQmac_1753800405 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Christina Schimpe writes: > 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 > > Reviewed-by: Thiago Jung Bauermann > --- > gdb/amd64-linux-tdep.c | 57 ++++++++- > .../gdb.arch/amd64-shadow-stack-corefile.c | 42 +++++++ > .../gdb.arch/amd64-shadow-stack-corefile.exp | 110 ++++++++++++++++++ > 3 files changed, 205 insertions(+), 4 deletions(-) > create mode 100644 gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.c > 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 edb7d8da6ab..9af7a41ea26 100644 > --- a/gdb/amd64-linux-tdep.c > +++ b/gdb/amd64-linux-tdep.c > @@ -47,6 +47,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" > @@ -1593,6 +1594,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") != nullptr; I think the comment on this function is not correct. We're not getting the shadow stack pointer state, we're: /* Return true if the core file ABFD contains shadow stack pointer state. Otherwise, return false. */ > +} > + > /* Get Linux/x86 target description from core dump. */ > > static const struct target_desc * > @@ -1602,11 +1611,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); As with feedback on earlier patches, I don't think the inclusion of '_mask' is a good one here. I think what you call xstate_bv_mask is an actual value, not a mask, and should be named 'xstate_bv' > + 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); > } > > @@ -1637,6 +1649,35 @@ static const struct regset amd64_linux_xstateregset = > amd64_linux_collect_xstateregset > }; > > +/* Supply shadow stack pointer register from SSP to the register cache > + REGCACHE. */ > + > +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 (ssp)); Before this line I'd like to see: gdb_assert (len == sizeof (uint64_t)); > +} > + > +/* 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 (ssp)); And I think this should get the same 'len == sizeof ...' assert as above please. > +} > + > +/* 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 > @@ -1653,6 +1694,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 Have you really seen this function called with 'regcache == nullptr' ? I'm suspect not as e.g. i386_supply_gregset, which is part of i386_gregset, makes an unchecked dereference of regcache, so if regcache was nullptr you'd see UB (crash) before getting to this check. > + || 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.c b/gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.c > new file mode 100644 > index 00000000000..f078e33810d > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.c > @@ -0,0 +1,42 @@ > +/* This test program is part of GDB, the GNU debugger. > + > + Copyright 2025 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 . */ > + > +#include > + > +/* Call the return instruction before function epilogue to trigger a > + control-flow exception. */ > +void > +function () > +{ > + unsigned long ssp; > + asm volatile("xor %0, %0; rdsspq %0" : "=r" (ssp)); > + > + /* Print ssp to stdout so that the testcase can capture it. */ > + printf ("%p\n", (void *) ssp); > + fflush (stdout); > + > + /* Manually cause a control-flow exception by executing a return > + instruction before function epilogue, so the address atop the stack > + is not the return instruction. */ > + __asm__ volatile ("ret\n"); > +} > + > +int > +main (void) > +{ > + function (); /* Break here. */ > +} > 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..8784fc3622c > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/amd64-shadow-stack-corefile.exp > @@ -0,0 +1,110 @@ > +# 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 . > + > +# Test the shadow stack pointer note in core dumps. > + > +require allow_ssp_tests > + > +standard_testfile > + > +proc check_core_file {core_filename saved_pl3_ssp} { > + global decimal > + > + # Load the core file. > + if [gdb_test "core $core_filename" \ > + [multi_line \ > + "Core was generated by .*\\." \ > + "Program terminated with signal SIGSEGV, Segmentation fault.*" \ > + "#0 function \\(\\) at .*amd64-shadow-stack-corefile.c:$decimal" \ > + "$decimal.*__asm__ volatile \\(\"ret\\\\n\"\\);"] \ > + "load core file"] { > + return > + } > + > + # Check the value of ssp in the core file. > + gdb_test "print/x \$pl3_ssp" "\\$\[0-9\]+ = $saved_pl3_ssp" \ > + "pl3_ssp contents from core file $saved_pl3_ssp" > +} > + > +save_vars { ::env(GLIBC_TUNABLES) } { > + > + append_environment GLIBC_TUNABLES "glibc.cpu.hwcaps" "SHSTK" > + > + if { [prepare_for_testing "failed to prepare" $testfile $srcfile \ > + {debug additional_flags="-fcf-protection=return"}] } { > + return > + } > + > + set linespec ${srcfile}:[gdb_get_line_number "Break here"] > + > + if ![runto $linespec] { > + return > + } > + > + # Continue until a crash. The line with the hex number is optional because > + # it's printed by the test program, and doesn't appear in the Expect buffer > + # when testing a remote target. > + gdb_test "continue" \ > + [multi_line \ > + "Continuing\\." \ > + "($hex\r\n)?" \ > + "Program received signal SIGSEGV, Segmentation fault.*" \ > + "function \\(\\) at .*amd64-shadow-stack-corefile.c:$decimal" \ > + {.*__asm__ volatile \("ret\\n"\);}] \ > + "continue to SIGSEGV" > + > + set ssp_in_gcore [get_valueof "/x" "\$pl3_ssp" "*unknown*"] > + > + # Generate the gcore core file. > + set gcore_filename [standard_output_file "${testfile}.gcore"] > + set gcore_generated [gdb_gcore_cmd "$gcore_filename" "generate gcore file"] > + > + # Obtain an OS-generated core file. Save test program output to > + # ${binfile}.out. > + set core_filename [core_find $binfile {} {} "${binfile}.out"] > + set core_generated [expr {$core_filename != ""}] > + set os_core_name "${binfile}.core" > + remote_exec build "mv $core_filename $os_core_name" > + set core_filename $os_core_name I'm wondering what the point of this core_filename / os_core_name stuff is? My reading of `core_find` is that the returned core_filename will be '${binfile}.core', so this whole thing feels redundant, but maybe I'm missing something here? > + > + # At this point we have a couple of core files, the gcore one generated by > + # GDB and the one generated by the operating system. Make sure GDB can > + # read both correctly. > + > + if {$gcore_generated} { > + clean_restart $binfile > + > + with_test_prefix "gcore corefile" { > + check_core_file $gcore_filename $ssp_in_gcore > + } > + } else { > + fail "gcore corefile not generated" It's better, where possible, to avoid having pass/fail results that only show up down some code paths. In this case it's easy to avoid having a stray 'fail' by restructuring the code too: gdb_assert { $gcore_generated } "gcore corefile created" if { $gcore_generated } { ... etc ... } Now you'll always have either a pass or fail based on the gcore being generated. There is also the helper proc `gcore_cmd_available`. I'd guess for any x86 target that supports SSP, gcore will be available, but in theory you could consider using this to avoid a fail when gcore is not available maybe? > + } > + > + if {$core_generated} { > + clean_restart $binfile > + > + with_test_prefix "OS corefile" { > + # Read ssp value from saved output of the test program. > + set out_id [open ${binfile}.out "r"] > + set ssp_in_gcore [gets $out_id] > + > + close $out_id > + check_core_file $core_filename $ssp_in_gcore I'd move the blank line after the 'close' personally. Thanks, Andrew > + } > + } else { > + untested "OS corefile not generated" > + } > +} > -- > 2.43.0