All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Subject: [PATCH 4/4] x86: prefer shadow stack for producing call traces
Date: Wed, 28 Feb 2024 14:53:21 +0100	[thread overview]
Message-ID: <fdacbb8b-5a58-4d6b-89b2-ea98b4fed895@suse.com> (raw)
In-Reply-To: <33da6f8b-af22-48c8-acce-3aa55c0e0414@suse.com>

Shadow stacks contain little more than return addresses, and they in
particular allow precise call traces also without FRAME_POINTER.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
While the 'E' for exception frames is probably okay, I'm not overly
happy with the 'C' (for CET). I would have preferred 'S' (for shadow),
but we use that character already.

As an alternative to suppressing output for the top level exception
frame, adding the new code ahead of the 'R' output line (and then also
ahead of the stack top read) could be considered.

Perhaps having a printk() for the PV entry case is meaningless, for
- no frame being pushed when entered from CPL=3 (64-bit PV),
- no entry possible from CPL<3 (32-bit PV disabled when CET is active)?
In which case the comment probably should just be "Bogus." and the code
merely be "break;".

Quite likely a number of other uses of is_active_kernel_text() also want
amending with in_stub().

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -449,6 +449,11 @@ unsigned long get_stack_dump_bottom(unsi
     }
 }
 
+static bool in_stub(unsigned long addr)
+{
+    return !((this_cpu(stubs.addr) ^ addr) >> STUB_BUF_SHIFT);
+}
+
 #if !defined(CONFIG_FRAME_POINTER)
 
 /*
@@ -539,6 +544,50 @@ static void show_trace(const struct cpu_
          !is_active_kernel_text(tos) )
         printk("   [<%p>] R %pS\n", _p(regs->rip), _p(regs->rip));
 
+    if ( IS_ENABLED(CONFIG_XEN_SHSTK) && rdssp() != SSP_NO_SHSTK )
+    {
+        const unsigned long *ptr = _p(regs->entry_ssp);
+        unsigned int n;
+
+        for ( n = 0; (unsigned long)ptr & (PAGE_SIZE - sizeof(*ptr)); ++n )
+        {
+            unsigned long val = *ptr;
+
+            if ( is_active_kernel_text(val) || in_stub(val) )
+            {
+                /* Normal return address entry.  */
+                printk("   [<%p>] C %pS\n", _p(val), _p(val));
+                ++ptr;
+            }
+            else if ( !((val ^ *ptr) >> (PAGE_SHIFT + STACK_ORDER)) )
+            {
+                if ( val & (sizeof(val) - 1) )
+                {
+                    /* Most likely a supervisor token. */
+                    break;
+                }
+
+                /*
+                 * Ought to be a hypervisor interruption frame.  But don't
+                 * (re)log the current frame's %rip.
+                 */
+                if ( n || ptr[1] != regs->rip )
+                    printk("   [<%p>] E %pS\n", _p(ptr[1]), _p(ptr[1]));
+                ptr = _p(val);
+            }
+            else
+            {
+                /* Ought to be a PV guest hypercall/interruption frame.  */
+                printk("   %04lx:[<%p>] E\n", ptr[2], _p(ptr[1]));
+                ptr = 0;
+            }
+        }
+
+        /* Fall back to legacy stack trace if nothing was logged at all. */
+        if ( n )
+            return;
+    }
+
     if ( fault )
     {
         printk("   [Fault on access]\n");



  parent reply	other threads:[~2024-02-28 13:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-28 13:50 [PATCH 0/4] x86: CET-SS related adjustments Jan Beulich
2024-02-28 13:51 ` [PATCH 1/4] x86: remove redundant XEN_SHSTK check from reinit_bsp_stack() Jan Beulich
2024-02-28 13:54   ` Andrew Cooper
2024-02-28 13:52 ` [PATCH 2/4] x86: record SSP at non-guest entry points Jan Beulich
2024-02-28 15:16   ` Andrew Cooper
2024-02-29  9:39     ` Jan Beulich
2024-02-28 13:52 ` [PATCH 3/4] x86/traps: use entry_ssp in fixup_exception_return() Jan Beulich
2024-02-28 15:21   ` Andrew Cooper
2024-02-28 15:26     ` Jan Beulich
2024-02-28 13:53 ` Jan Beulich [this message]
2024-02-28 16:15   ` [PATCH 4/4] x86: prefer shadow stack for producing call traces Andrew Cooper
2024-02-29  8:28     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fdacbb8b-5a58-4d6b-89b2-ea98b4fed895@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.