RE: [PATCH v5 10/12] gdb: Implement amd64 linux shadow stack support for inferior calls.

Schimpe, Christina
2025-07-31T12:33:37Z

Hi Andrew,

Thanks a lot for the review. Please find my comments to your feedback below.

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Wednesday, July 30, 2025 1:58 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Cc: thiago.bauermann@linaro.org; luis.machado@arm.com
> Subject: Re: [PATCH v5 10/12] gdb: Implement amd64 linux shadow stack
> support for inferior calls.
>
> Christina Schimpe <christina.schimpe@intel.com> writes:
>
> > This patch enables inferior calls to support Intel's Control-Flow
> > Enforcement Technology (CET), which provides the shadow stack feature
> > for the x86 architecture.
> > Following the restriction of the linux kernel, enable inferior calls
> > for amd64 only.
> >
> > Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> > Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> > Reviewed-By: Luis Machado <luis.machado@arm.com>
> > ---
> > gdb/amd64-linux-tdep.c | 63 +++++++++++++++++++
> > gdb/doc/gdb.texinfo | 29 +++++++++
> > .../gdb.arch/amd64-shadow-stack-cmds.exp | 55 +++++++++++++++-
> > 3 files changed, 146 insertions(+), 1 deletion(-)
> >
> > diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c index
> > f6ae5395870..899fe2df02c 100644
> > --- a/gdb/amd64-linux-tdep.c
> > +++ b/gdb/amd64-linux-tdep.c
> > @@ -1932,6 +1932,67 @@
> amd64_linux_shadow_stack_element_size_aligned (gdbarch *gdbarch)
> > return (binfo->bits_per_word / binfo->bits_per_byte); }
> >
> > +/* Read the shadow stack pointer register and return its value, if
> > + possible. */
> > +
> > +static std::optional<CORE_ADDR>
> > +amd64_linux_get_shadow_stack_pointer (gdbarch *gdbarch, regcache
> > +*regcache) {
> > + const i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep>
> > +(gdbarch);
> > +
> > + if (tdep == nullptr || tdep->ssp_regnum < 0)
> > + return {};
>
> We don't check for 'tdep == nullptr' anywhere else (for x86), so I don't think
> this check is needed. If you _really_ want you could gdb_assert, but I'm
> pretty sure you'll have crashed long before now if tdep == nullptr.

I agree and will remove.

> > +
> > + CORE_ADDR ssp;
> > + if (regcache_raw_read_unsigned (regcache, tdep->ssp_regnum, &ssp)
> > + != REG_VALID)
> > + return {};
> > +
> > + /* Dependent on the target in case the shadow stack pointer is
> > + unavailable, the ssp register can be invalid or 0x0 when shadow stack
> > + is supported by HW and the linux kernel but not enabled for the
> > + current thread. */
>
> Just so I understand, is the 0 coming from actual inferior state? Or is the 0
> creeping in from somewhere in GDB when we should be creating an
> unavailable value, but get it wrong?
>
> Using 0 a magic address value is something I really dislike (having worked on
> targets where 0 is a valid address), so they always make me uncomfortable.
> If this is an actual h/w thing then there's nothing we could or should do
> about it ... but if this is a GDB thing, then maybe we can fix that?

We see this dependent on the linux kernel version. So the value 0x0 is set by the kernel I think.
With kernels older than 6.13 we shouldn't see this anymore, due to this patch:
https://github.com/torvalds/linux/commit/a9d9c33132d49329ada647e4514d210d15e31d81

>
> > + if (ssp == 0x0)
> > + return {};
> > +
> > + return ssp;
> > +}
> > +
> > +/* If shadow stack is enabled, push the address NEW_ADDR to the
> shadow
> > + stack and increment the shadow stack pointer accordingly. */
> > +
> > +static void
> > +amd64_linux_shadow_stack_push (gdbarch *gdbarch, CORE_ADDR
> new_addr,
> > + regcache *regcache)
> > +{
> > + std::optional<CORE_ADDR> ssp
> > + = amd64_linux_get_shadow_stack_pointer (gdbarch, regcache);
> > + if (!ssp.has_value ())
> > + return;
> > +
> > + /* The shadow stack grows downwards. To push addresses to the stack,
> > + we need to decrement SSP. */
> > + const int element_size
> > + = amd64_linux_shadow_stack_element_size_aligned (gdbarch); const
> > + CORE_ADDR new_ssp = *ssp - element_size;
> > +
> > + /* Using /proc/PID/smaps we can only check if NEW_SSP points to
> shadow
> > + stack memory. If it doesn't, we assume the stack is full. */
> > + std::pair<CORE_ADDR, CORE_ADDR> memrange; if
> > + (!linux_address_in_shadow_stack_mem_range (new_ssp, &memrange))
> > + error (_("No space left on the shadow stack."));
> > +
> > + /* On x86 there can be a shadow stack token at bit 63. For x32,
> > + the
>
> I don't understand this first sentence and how it relates to either the rest of
> the comment, or the following code.

Yes, this is confusing I agree. I guess all what I wanted to say is that we have to
write the full 8 bytes due to the token even though the actual address of the
shadow stack pointer is only 4 bytes for x32.

Example shadow stack token for x32 in frame #1:
bt shadow^M
#0 0x00000000f7d22040 in __restore_rt from /libx32/libc.so.6^M
#1 0x80000000f7ce6fd8^M
#2 0x00000000f7d21f8b in raise from /libx32/libc.so.6

The output is taken from a patch for the "bt shadow" command,
which is not part of this series.

How about:

On x86 there can be a shadow stack token at bit 63. For x32, the
address size is only 32 bit. Always write back the full 8 bytes to include
the shadow stack token.

>
> > + address size is only 32 bit. Thus, we must use ELEMENT_SIZE (and
> > + not gdbarch_addr_bit) to determine the width of the address to be
> > + written. */
> > + const bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > + write_memory_unsigned_integer (new_ssp, element_size, byte_order,
> > + (ULONGEST) new_addr);
> > +
> > + i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep>
> > + (gdbarch); regcache_raw_write_unsigned (regcache, tdep->ssp_regnum,
> > + new_ssp);
>
> By this point we know that tdep->ssp_regnum must be a valid regnum. But
> the checks for that are in a separate function, so maybe we should:
>
> gdb_assert (tdep->ssp_regnum > -1);

Good idea, will add.

>
> > +}
> >
> > /* Implement shadow stack pointer unwinding. For each new shadow
> stack
> > pointer check if its address is still in the shadow stack memory range.
> > @@ -2059,6 +2120,8 @@ amd64_linux_init_abi_common(struct
> gdbarch_info
> > info, struct gdbarch *gdbarch,
> >
> > set_gdbarch_remove_non_address_bits_watchpoint
> > (gdbarch, amd64_linux_remove_non_address_bits_watchpoint);
> > +
> > + set_gdbarch_shadow_stack_push (gdbarch,
> > + amd64_linux_shadow_stack_push);
> > dwarf2_frame_set_init_reg (gdbarch, amd64_init_reg); }
> >
> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index
> > 0881ac4aee5..b5120b78426 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -27037,6 +27037,35 @@ registers
> >
> > @end itemize
> >
> > +@subsubsection Intel Control-Flow Enforcement Technology.
> > +@cindex Intel Control-Flow Enforcement Technology.
> > +
> > +The @dfn{Intel Control-Flow Enforcement Technology} (@acronym{Intel
> > +CET}) provides two capabilities to defend against ``Return-oriented
> Programming''
> > +and ``call/jmp-oriented programming'' style control-flow attacks:
> > +
> > +@itemize @bullet
> > +@item Shadow Stack:
> > +A shadow stack is a second stack for a program. It holds the return
> > +addresses pushed by the call instruction. The @code{RET} instruction
> > +pops the return addresses from both call and shadow stack. If the
> > +return addresses from the two stacks do not match, the processor
> > +signals a control protection exception.
> > +@item Indirect Branch Tracking (IBT):
> > +When IBT is enabled, the CPU implements a state machine that tracks
> > +indirect @code{JMP} and @code{CALL} instructions. The state machine
> > +can be either IDLE or WAIT_FOR_ENDBRANCH. In
> WAIT_FOR_ENDBRANCH
> > +state the next instruction in the program stream must be an
> > +@code{ENDBR} instruction, otherwise the processor signals a control
> protection exception.
>
> If I understand it, the IBT doesn't currently have an impact on GDB, right?
> The inferior function being called likely starts with an `endbr` instruction, but
> GDB will just leave the IBT mechanism in the IDLE state, and the endbr will
> be interpreted as a nop.

There is no linux kernel support for IBT in userspace, yet.

>
> I also found this description a little too light on the details. Just having the
> name was enough to go and find the real docs, but I think the text could be
> made cleared with two additional sentences:
>
> @item Indirect Branch Tracking (IBT):
> When IBT is enabled, the CPU implements a state machine that tracks
> indirect
> @code{JMP} and @code{CALL} instructions. The state machine can be
> either IDLE
> or WAIT_FOR_ENDBRANCH. When a @code{JMP} or @code{CALL} is
> executed
> the state machine chages to the WAIT_FOR_ENDBRANCH state. In
> WAIT_FOR_ENDBRANCH state the next instruction in the program stream
> must be an @code{ENDBR} instruction, otherwise the
> processor signals a control protection exception. After executing a
> @code{ENDBR} instruction the state machine returns to the IDLE state.
>
> This change isn't a hard requirement, but I do think this makes the
> description more useful.

I agree, your suggestion improves the description, thank you. Will improve the docs here.

> > +@end itemize
> > +
> > +Impact on Call/Print:
> > +Inferior calls in @value{GDBN} reset the current PC to the beginning
> > +of the function that is called. No call instruction is executed, but
> > +the @code{RET} instruction actually is. To avoid a control
> > +protection exception due to the missing return address on the shadow
> > +stack, @value{GDBN} pushes the new return address to the shadow stack
> and updates the shadow stack pointer.
> > +
> > @node Alpha
> > @subsection Alpha
> >
> > diff --git a/gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp
> > b/gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp
> > index 17f32ce3964..622612d2f7d 100644
> > --- a/gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp
> > +++ b/gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp
> > @@ -13,12 +13,29 @@
> > # 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 shadow stack enabling for frame level update and the return
> command.
> > +# Test shadow stack enabling for frame level update, the return and
> > +the # call commands.
> > +# As potential CET violations often only occur after resuming normal
> > +# execution, test normal program continuation after each return or
> > +call # commands.
> >
> > require allow_ssp_tests
> >
> > standard_testfile amd64-shadow-stack.c
> >
> > +proc restart_and_run_infcall_call2 {} {
>
> There should be a comment before each proc please.

True, will add.

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