diff mbox

[v3,for-4.9] x86/emul: Poision the stubs with debug traps

Message ID 1491573658-3243-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper April 7, 2017, 2 p.m. UTC
...rather than leaving fragments of old instructions in place.  This reduces
the chances of something going further-wrong (as the debug trap will be caught
and terminate the guest) in a cascade-failure where we end up executing the
instruction fragments.

Before:
    (XEN) d2v0 exception 6 (ec=0000) in emulation stub (line 6239)
    (XEN) d2v0 stub: c4 e1 44 77 c3 80 d0 82 ff ff ff d1 90 ec 90

After:
    (XEN) d3v0 exception 6 (ec=0000) in emulation stub (line 6239)
    (XEN) d3v0 stub: c4 e1 44 77 c3 cc cc cc cc cc cc cc cc cc cc

To make this work, the int3 handler needs to be extended to attempt recovery
rather than simply returning back to Xen context.  While altering do_int3(),
leave an obvious sign if an embedded breakpoint has been hit and not dealt
with by debugging facilities.

    (XEN) Hit embedded breakpoint at ffff82d0803d01f6 [extable.c#stub_selftest+0xda/0xee]

Extend the selftests to include int3, and add an extra printk indicating the
start of the recovery selftests, to avoid leaving otherwise-spurious faults
visible in the log.

    (XEN) build-id: 55d7e6f420b4f0ce277f776be620f43d7cb8646c
    (XEN) Running stub recovery selftests...
    (XEN) traps.c:3466: GPF (0000): ffff82d0bffff041 [ffff82d0bffff041] -> ffff82d08035937a
    (XEN) traps.c:813: Trap 12: ffff82d0bffff040 [ffff82d0bffff040] -> ffff82d08035937a
    (XEN) traps.c:1215: Trap 3: ffff82d0bffff041 [ffff82d0bffff041] -> ffff82d08035937a
    (XEN) ACPI sleep modes: S3

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>

v2:
 * Add selftest.  Recover from int3.
v3:
 * Leave embedded int3s as non-fatal, but make them obvious.
---
 xen/arch/x86/extable.c     |  4 ++++
 xen/arch/x86/traps.c       | 18 ++++++++++++++++--
 xen/arch/x86/x86_emulate.c |  4 ++--
 3 files changed, 22 insertions(+), 4 deletions(-)

Comments

Jan Beulich April 7, 2017, 2:27 p.m. UTC | #1
>>> On 07.04.17 at 16:00, <andrew.cooper3@citrix.com> wrote:
> ...rather than leaving fragments of old instructions in place.  This reduces
> the chances of something going further-wrong (as the debug trap will be 
> caught
> and terminate the guest) in a cascade-failure where we end up executing the
> instruction fragments.
> 
> Before:
>     (XEN) d2v0 exception 6 (ec=0000) in emulation stub (line 6239)
>     (XEN) d2v0 stub: c4 e1 44 77 c3 80 d0 82 ff ff ff d1 90 ec 90
> 
> After:
>     (XEN) d3v0 exception 6 (ec=0000) in emulation stub (line 6239)
>     (XEN) d3v0 stub: c4 e1 44 77 c3 cc cc cc cc cc cc cc cc cc cc
> 
> To make this work, the int3 handler needs to be extended to attempt recovery
> rather than simply returning back to Xen context.  While altering do_int3(),
> leave an obvious sign if an embedded breakpoint has been hit and not dealt
> with by debugging facilities.
> 
>     (XEN) Hit embedded breakpoint at ffff82d0803d01f6 
> [extable.c#stub_selftest+0xda/0xee]
> 
> Extend the selftests to include int3, and add an extra printk indicating the
> start of the recovery selftests, to avoid leaving otherwise-spurious faults
> visible in the log.
> 
>     (XEN) build-id: 55d7e6f420b4f0ce277f776be620f43d7cb8646c
>     (XEN) Running stub recovery selftests...
>     (XEN) traps.c:3466: GPF (0000): ffff82d0bffff041 [ffff82d0bffff041] -> ffff82d08035937a
>     (XEN) traps.c:813: Trap 12: ffff82d0bffff040 [ffff82d0bffff040] -> ffff82d08035937a
>     (XEN) traps.c:1215: Trap 3: ffff82d0bffff041 [ffff82d0bffff041] -> ffff82d08035937a
>     (XEN) ACPI sleep modes: S3
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff mbox

Patch

diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 03af2c9..6fffe05 100644
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -140,10 +140,14 @@  static int __init stub_selftest(void)
         { .opc = { 0x02, 0x04, 0x04, 0xc3 }, /* add (%rsp,%rax),%al */
           .rax = 0xfedcba9876543210,
           .res.fields.trapnr = TRAP_stack_error },
+        { .opc = { 0xcc, 0xc3, 0xc3, 0xc3 }, /* int3 */
+          .res.fields.trapnr = TRAP_int3 },
     };
     unsigned long addr = this_cpu(stubs.addr) + STUB_BUF_SIZE / 2;
     unsigned int i;
 
+    printk("Running stub recovery selftests...\n");
+
     for ( i = 0; i < ARRAY_SIZE(tests); ++i )
     {
         uint8_t *ptr = map_domain_page(_mfn(this_cpu(stubs.mfn))) +
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 5b9bf21..d69769f 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1206,9 +1206,23 @@  void do_int3(struct cpu_user_regs *regs)
 
     if ( !guest_mode(regs) )
     {
-        debugger_trap_fatal(TRAP_int3, regs);
+        unsigned long fixup;
+
+        if ( (fixup = search_exception_table(regs)) != 0 )
+        {
+            this_cpu(last_extable_addr) = regs->rip;
+            dprintk(XENLOG_DEBUG, "Trap %u: %p [%ps] -> %p\n",
+                    TRAP_int3, _p(regs->rip), _p(regs->rip), _p(fixup));
+            regs->rip = fixup;
+            return;
+        }
+
+        if ( !debugger_trap_fatal(TRAP_int3, regs) )
+            printk(XENLOG_DEBUG "Hit embedded breakpoint at %p [%ps]\n",
+                   _p(regs->rip), _p(regs->rip));
+
         return;
-    } 
+    }
 
     do_guest_trap(TRAP_int3, regs);
 }
diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c
index 51df340..cc334ca 100644
--- a/xen/arch/x86/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate.c
@@ -30,8 +30,8 @@ 
     BUILD_BUG_ON(STUB_BUF_SIZE / 2 < MAX_INST_LEN + 1);         \
     ASSERT(!(stb).ptr);                                         \
     (stb).addr = this_cpu(stubs.addr) + STUB_BUF_SIZE / 2;      \
-    ((stb).ptr = map_domain_page(_mfn(this_cpu(stubs.mfn)))) +  \
-        ((stb).addr & ~PAGE_MASK);                              \
+    memset(((stb).ptr = map_domain_page(_mfn(this_cpu(stubs.mfn)))) +  \
+           ((stb).addr & ~PAGE_MASK), 0xcc, STUB_BUF_SIZE / 2);        \
 })
 #define put_stub(stb) ({                                   \
     if ( (stb).ptr )                                       \