Re: [Xen-users] xc_hvm_inject_trap() failing for int3 traps under Xen 4.2.2

[Antony Saba <Antony.Saba@mandiant.com>]

On 06/10/2013 12:36 PM, Tim Deegan wrote:
> At 16:57 +0000 on 10 Jun (1370883430), Antony Saba wrote:
>> On 06/10/2013 05:29 AM, George Dunlap wrote:
>>> On Fri, Jun 7, 2013 at 4:51 PM, Steven Maresca <steve@zentific.com> wrote:
>>>> Tony,
>>>>
>>>> I can confirm INT3 re-injection does work on 4.2.x and 4.3, but the
>>>> problem you observed is certainly present.
>>>>
>>>> As suggested, it was necessary when invoking xc_hvm_inject_trap to
>>>> specify the 1-byte instruction length for 0xCC (without which the VM
>>>> was intentionally crashed by Xen).
>>>>
>>>> In this case, there's no need to inspect the actual instruction
>>>> referenced by the IP because it seems the trap is only fired for the
>>>> one-byte variant (0xCD03 of course works properly, but no event is
>>>> emitted).
>>>>
>>>> Mirroring your experience with 4.1.2, for my testing on 4.2+ the
>>>> return of xc_hvm_inject_trap is also always non-zero even for
>>>> successful re-injection..whether that's intended is another question.
>>>>
>>>> Steve
>>>>
>>>> NOTE: I would definitely consider it a bug that the xen-access.c
>>>> example crashes guests when attempting to use the INT3
>>>> mode...non-critical for most users, but nevertheless.
>>>
>>> I'm having a bit of trouble finding the conclusion here.
>>>
>>> So it seems the problem is that if a *guest* is doing int3
>>> instructions, that will interfere with the ability of the debugger to
>>> use int3 to do introspection -- is that right?
>>>
>> Yes, that is one scenario.  The one I was experiencing was some
>> (apparently legitimate) background process on a Windows 7 x64 guest that
>> just always executes an int3 when it runs.
>>
>> I'll try to summarize, someone please correct me if I'm wrong.  There
>> are 2 things going on here:
>>
>> 1) The patch previously posted by AP is the correct way to call
>> xc_hvm_inject_trap() for int 3 (0xcc).  That is, the instruction_length
>> parameter must be set to 1.
> 
> Not necessarily, AFAICT -- you'd need to fetch and decode the
> instruction in order to detect prefix bytes (other than LOCK, which is
> explicitly disallowed).

I just verified this again under 4.2.2, here is the crash dump from xl
dmesg:

(XEN) <vm_resume_fail> error code 7
(XEN) domain_crash_sync called from vmcs.c:1068
(XEN) Domain 2 (vcpu#0) crashed on cpu#6:
(XEN) ----[ Xen-4.2.2  x86_64  debug=n  Not tainted ]----
(XEN) CPU:    6
(XEN) RIP:    001b:[<0000000000401000>]
(XEN) RFLAGS: 0000000000000246   CONTEXT: hvm guest
(XEN) rax: ffff82c4801053e2   rbx: ffff83017838e000   rcx: 000000007ffd4000
(XEN) rdx: ffff82c4801cc600   rsi: 0000000000000000   rdi: ffff82c4801d5b2d
(XEN) rbp: ffff82c480180a99   rsp: 000000000012ff7c   r8:  000000000012ffc0
(XEN) r9:  ffff8302334f7f18   r10: 0000000000000002   r11: ffff82c4801053bb
(XEN) r12: 0000000000000000   r13: 0000000000000000   r14: 0000000000000000
(XEN) r15: ffff8302334f7f18   cr0: 000000008001003b   cr4: 00000000000006f9
(XEN) cr3: 000000000b600180   cr2: 0000000000153005
(XEN) ds: 0023   es: 0023   fs: 003b   gs: 0000   ss: 0023   cs: 001b

This is the change to xen_access to ignore the error and attempt to
resume that causes it:

diff --git a/tools/tests/xen-access/xen-access.c
b/tools/tests/xen-access/xen-access.c
index 9ec7332..77d7b12 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -668,8 +668,8 @@ int main(int argc, char *argv[])
                 if (rc < 0)
                 {
                     ERROR("Error %d injecting int3\n", rc);
-                    interrupted = -1;
-                    continue;
+                    //interrupted = -1;
+                    //continue;
                 }

                 break;

> 
>> 2) xc_hvm_inject_trap() always returns a negative value, even when there
>> is not a problem and the guest receives the trap as expected.  There
>> hasn't been a clarification as to whether it's supposed to return
>> non-negative, but one would assume that it should because of the way the
>> xen-access.c example checks for it.
> 
> That looks like a hypervisor bug to me: does this (untested) patch fix
> it for you?
> 
> commit 67b9272fcedcb5dc73cc77a2adf580f2572117d7
> Author: Tim Deegan <tim@xen.org>
> Date:   Mon Jun 10 19:35:34 2013 +0100
> 
>     x86/hvm: Fix HVMOP_inject_trap return value on success.
>     
>     Reported-by: Antony Saba <Antony.Saba@mandiant.com>
>     Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ce44bff..6c86fc2 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4430,6 +4430,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>              v->arch.hvm_vcpu.inject_trap.error_code = tr.error_code;
>              v->arch.hvm_vcpu.inject_trap.insn_len = tr.insn_len;
>              v->arch.hvm_vcpu.inject_trap.cr2 = tr.cr2;
> +            rc = 0;
>          }
>  
>      param_fail8:
> 
> 
> 

This works, but the instruction size must be set to 1, at least on 4.2.2
to work for me.  Here is the patch against RELEASE-4.2.2.


diff --git a/tools/tests/xen-access/xen-access.c
b/tools/tests/xen-access/xen-access.c
index 9ec7332..8bcd88b 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -664,7 +664,7 @@ int main(int argc, char *argv[])
                 /* Reinject */
                 rc = xc_hvm_inject_trap(
                     xch, domain_id, req.vcpu_id, 3,
-                    HVMOP_TRAP_sw_exc, -1, 0, 0);
+                    HVMOP_TRAP_sw_exc, -1, 1, 0);
                 if (rc < 0)
                 {
                     ERROR("Error %d injecting int3\n", rc);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3d471a5..4c2320e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4372,6 +4372,7 @@ long do_hvm_op(unsigned long op,
XEN_GUEST_HANDLE(void) arg)
             v->arch.hvm_vcpu.inject_trap.error_code = tr.error_code;
             v->arch.hvm_vcpu.inject_trap.insn_len = tr.insn_len;
             v->arch.hvm_vcpu.inject_trap.cr2 = tr.cr2;
+            rc = 0;
         }

     param_fail8:


-- 
Antony Saba, antony.saba@mandiant.com