[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] x86/cet: Fix shskt manipulation error with BUGFRAME_{warn,run_fn}


  • To: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Thu, 12 Aug 2021 18:03:50 +0100
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 12 Aug 2021 17:10:28 +0000
  • Ironport-hdrordr: A9a23:EzCN06tmk1sPFCCCVc+KkKNi7skDdtV00zEX/kB9WHVpmszxra CTdZMgpGbJYVcqKRcdcL+7Scu9qB/nm6KdgrNhWotKPjOW3VdARbsKheCJrlGOJ8SXzJ8k6U 4KSdkcNDSfNzlHZL7BkW2FL+o=
  • Ironport-sdr: GPYmbTzhh2ySuasRWmQ1JLB/2GsEMOcXaTltihiZl3bQbJFQHG58xR+XQJ/V2PAx5UV8r1qZrt vhcwlg9xz6pV6ZTJj+Z6pmJ2Bge/TX7i2+XuQNU9gZMyahJiAZgV+WqlP4uhVYkEy7++kVuoIl 1tBT6+sRlEgaZ2NPcSdAdSBP5Ld2Hr4+HyJTkfkN3pODzEz1er5Yla4T5cjKmU1h8FuKAEIZdX J3uMpG0Z5n5nd5IFrVaWSItcK8Zemb34LsSWtgTAzlIEKCh/oww774VbBb4M1ya1JGPlgN90pi LHeTVznhFKIWa4xOELgvfqgl
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

This was a clear oversight in the original CET work.  The BUGFRAME_run_fn and
BUGFRAME_warn paths update regs->rip without an equivlenet adjustment to the
shadow stack, causes IRET to suffer #CP due to the mismatch.

One subtle, and therefore fragile, aspect of extable_shstk_fixup() was that it
required regs->rip to have its old value as a cross-check that the correct
word in the shadow stack was being adjusted.

Rework extable_shstk_fixup() into fixup_exception_return() which takes
ownership of the update to both the regular and shadow stacks, ensuring that
the regs->rip update is ordered suitably.

Use the new fixup_exception_return() for BUGFRAME_run_fn and BUGFRAME_warn to
ensure that the shadow stack is updated too.

Fixes: 209fb9919b50 ("x86/extable: Adjust extable handling to be shadow stack 
compatible")
Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

Backport to 4.14

Only compile tested so far.  My one CET-SS machine is in use for other
purposes right now.

I'm not a massive fan of the large ifdef area.  The logic could be rearranged
to use IS_ENABLED(CONFIG_XEN_SHSTK) by indenting most of the function, but I
can't see any way to drop the goto's, and this is certainly the least-invasive
diff.
---
 xen/arch/x86/traps.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e60af16ddd8c..30eefbad4863 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -777,13 +777,15 @@ static void do_reserved_trap(struct cpu_user_regs *regs)
           trapnr, vec_name(trapnr), regs->error_code);
 }
 
-static void extable_shstk_fixup(struct cpu_user_regs *regs, unsigned long 
fixup)
+static void fixup_exception_return(struct cpu_user_regs *regs,
+                                   unsigned long fixup)
 {
+#ifdef CONFIG_XEN_SHSTK
     unsigned long ssp, *ptr, *base;
 
     asm ( "rdsspq %0" : "=r" (ssp) : "0" (1) );
     if ( ssp == 1 )
-        return;
+        goto shstk_done;
 
     ptr = _p(ssp);
     base = _p(get_shstk_bottom(ssp));
@@ -814,7 +816,7 @@ static void extable_shstk_fixup(struct cpu_user_regs *regs, 
unsigned long fixup)
             asm ( "wrssq %[fix], %[stk]"
                   : [stk] "=m" (ptr[0])
                   : [fix] "r" (fixup) );
-            return;
+            goto shstk_done;
         }
     }
 
@@ -824,6 +826,12 @@ static void extable_shstk_fixup(struct cpu_user_regs 
*regs, unsigned long fixup)
      * executing the interrupted context.
      */
     BUG();
+
+ shstk_done:
+#endif /* CONFIG_XEN_SHSTK */
+
+    /* Fixup the regular stack. */
+    regs->rip = fixup;
 }
 
 static bool extable_fixup(struct cpu_user_regs *regs, bool print)
@@ -842,10 +850,7 @@ static bool extable_fixup(struct cpu_user_regs *regs, bool 
print)
                vec_name(regs->entry_vector), regs->error_code,
                _p(regs->rip), _p(regs->rip), _p(fixup));
 
-    if ( IS_ENABLED(CONFIG_XEN_SHSTK) )
-        extable_shstk_fixup(regs, fixup);
-
-    regs->rip = fixup;
+    fixup_exception_return(regs, fixup);
     this_cpu(last_extable_addr) = regs->rip;
 
     return true;
@@ -1138,7 +1143,7 @@ void do_invalid_op(struct cpu_user_regs *regs)
         void (*fn)(struct cpu_user_regs *) = bug_ptr(bug);
 
         fn(regs);
-        regs->rip = (unsigned long)eip;
+        fixup_exception_return(regs, (unsigned long)eip);
         return;
     }
 
@@ -1159,7 +1164,7 @@ void do_invalid_op(struct cpu_user_regs *regs)
     case BUGFRAME_warn:
         printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno);
         show_execution_state(regs);
-        regs->rip = (unsigned long)eip;
+        fixup_exception_return(regs, (unsigned long)eip);
         return;
 
     case BUGFRAME_bug:
-- 
2.11.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.