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

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

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