Re: [PATCH v4 09/11] gdb: Implement amd64 linux shadow stack support for inferior calls.

[Luis Machado <luis.machado@arm.com>]

On 6/17/25 13:11, Christina Schimpe wrote:
> 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>
> ---
>  gdb/amd64-linux-tdep.c                        | 64 +++++++++++++++++++
>  gdb/doc/gdb.texinfo                           | 29 +++++++++
>  .../gdb.arch/amd64-shadow-stack-cmds.exp      | 55 +++++++++++++++-
>  3 files changed, 147 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index 9436f0b190c..d847248659a 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1931,6 +1931,68 @@ 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 {};
> +
> +  CORE_ADDR ssp;
> +  if (regcache_raw_read_unsigned (regcache, tdep->ssp_regnum, &ssp)
> +      != REG_VALID)
> +    return {};
> +
> +  /* Starting with v6.6., the Linux kernel supports CET shadow stack.

Typo, period after "v6.6".

> +     Dependent on the target the ssp register can be invalid or nullptr
> +     when shadow stack is supported by HW and the linux kernel but not
> +     enabled for the current thread.  */

I feel the comment doesn't quite reflect this case very well. We don't have a
nullptr here, so I suppose ssp == 0x0 means ssp is unavailable? We should make
that clear here, but we don't need to have this more generic comment pasted
here again, as it was explained elsewhere already.

> +  if (ssp == 0x0)
> +    return {};
> +
> +  return ssp;
> +}
> +
> +/* If shadow stack is enabled, push the address NEW_ADDR on the shadow

s/the address NEW_ADDR on/NEW_ADDR to

> +   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 on the stack,

s/on the/to the

> +     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;
> +
> +  /* Starting with v6.6., the Linux kernel supports CET shadow stack.

Same typo, period after "v6.6". I feel this comment has been repeated enough
times throughout the code. If it is available at a visible location, I think
we can do without it elsewhere.

For instance, we could even mention the
kernel version in the news entry, or at the gdbarch initialization code when
we are fetching a target description. Then it should be enough.

> +     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
> +     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);
> +}
>  
>  /* Implement shadow stack pointer unwinding. For each new shadow stack
>     pointer check if its address is still in the shadow stack memory range.
> @@ -2057,6 +2119,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 0ae09f09c88..cf152bd1e6f 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -27033,6 +27033,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.
> +@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..96f83678f39 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 command.

s/command/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 {} {
> +    global binfile
> +    clean_restart ${binfile}
> +    if { ![runto_main] } {
> +	return -1
> +    }
> +    set inside_infcall_str "The program being debugged stopped while in a function called from GDB"
> +    gdb_breakpoint [ gdb_get_line_number "break call2" ]
> +    gdb_continue_to_breakpoint "break call2" ".*break call2.*"
> +    gdb_test "call (int) call2()" \
> +	"Breakpoint \[0-9\]*, call2.*$inside_infcall_str.*"
> +}
> +
>  save_vars { ::env(GLIBC_TUNABLES) } {
>  
>      append_environment GLIBC_TUNABLES "glibc.cpu.hwcaps" "SHSTK"
> @@ -33,6 +50,42 @@ save_vars { ::env(GLIBC_TUNABLES) } {
>  	return -1
>      }
>  
> +    with_test_prefix "test inferior call and continue" {
> +	gdb_breakpoint [ gdb_get_line_number "break call1" ]
> +	gdb_continue_to_breakpoint "break call1" ".*break call1.*"
> +
> +	gdb_test "call (int) call2()" "= 42"
> +
> +	gdb_continue_to_end
> +    }
> +
> +    with_test_prefix "test return inside an inferior call" {
> +	restart_and_run_infcall_call2
> +
> +	gdb_test "return" "\#0.*call2.*" \
> +	    "Test shadow stack return inside an inferior call" \
> +	    "Make.*return now\\? \\(y or n\\) " "y"
> +
> +	gdb_continue_to_end
> +    }
> +
> +    with_test_prefix "test return 'above' an inferior call" {
> +	restart_and_run_infcall_call2
> +
> +	gdb_test "frame 2" "call2 ().*" "move to frame 'above' inferior call"
> +
> +	gdb_test "return" "\#0.*call1.*" \
> +	    "Test shadow stack return 'above' an inferior call" \
> +	    "Make.*return now\\? \\(y or n\\) " "y"
> +
> +	gdb_continue_to_end
> +    }
> +
> +    clean_restart ${binfile}
> +    if { ![runto_main] } {
> +	return -1
> +    }
> +
>      set call1_line [ gdb_get_line_number "break call1" ]
>      set call2_line [ gdb_get_line_number "break call2" ]
>  

Otherwise this is OK.

Reviewed-By: Luis Machado <luis.machado@arm.com>