Tim Deegan
2009-Dec-02 11:47 UTC
[Xen-devel] Crash with c/s 20097 (x86 vmx: Update EIP when appropriate during task switch)
C/S 20097 reads the instruction length field of the VMCS in the task
switch handler:
+ inst_len = __get_instruction_length(); /* Safe: See SDM 3B 23.2.4 */
+ if ( (source == 3) && (idtv_info & INTR_INFO_VALID_MASK) )
+ {
+ /* ExtInt, NMI, HWException: no instruction to skip over. */
+ if ( !(idtv_info & (1u<<10)) ) /* 0 <= IntrType <=
3? */
+ inst_len = 0;
+ /* If there''s an error code then we pass it along. */
+ if ( idtv_info & INTR_INFO_DELIVER_CODE_MASK )
+ ecode = __vmread(IDT_VECTORING_ERROR_CODE);
+ }
+ regs->eip += inst_len;
+ hvm_task_switch((uint16_t)exit_qualification, reasons[source],
ecode);
The __get_instruction_length() _isn''t_ safe in those cases where we
later explicitly set inst_len to zero (and possibly in other cases).
Which leads to:
(XEN) Xen BUG at vmx.c:1461
(XEN) ----[ Xen-3.4.2 x86_64 debug=n Not tainted ]----
(XEN) CPU: 1
(XEN) RIP: e008:[<ffff828c8019ac47>] __get_instruction_length+0x17/0x30
(XEN) RFLAGS: 0000000000010282 CONTEXT: hypervisor
(XEN) rax: 00000000ffffffff rbx: 0000000000000003 rcx: 0000000000000000
(XEN) rdx: ffff828c801d8b30 rsi: 00000000ffdffc50 rdi: ffff83027fdd7f28
(XEN) rbp: 00000000c0000058 rsp: ffff83027fdd7e40 r8: 0000000000000000
(XEN) r9: 0000000000000000 r10: 0000000000000000 r11: 0000000000000000
(XEN) r12: 0000000000000009 r13: 0000000080000202 r14: ffff83027fdd7f28
(XEN) r15: 0000000000000000 cr0: 000000008005003b cr4: 00000000000026f0
(XEN) cr3: 0000000250fb0000 cr2: 00000000f8364b95
(XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008
(XEN) Xen stack trace from rsp=ffff83027fdd7e40:
(XEN) ffff828c8019dd7b 000004761a686532 ffff828c8022c100 ffff828c801182ed
(XEN) 000004761a4dc151 ffff83027fdf1ea8 ffff83007f3de000 0000000001c9c380
(XEN) ffff83024b6fe648 ffff83007f3df760 ffff828c8018ed5a ffff828c8018f962
(XEN) ffff83007f3de000 0000000000000000 ffff828c8018a5d8 ffff83007f3de000
(XEN) ffff83007f3de000 ffff83007f71a000 ffff828c80197f22 ffff828c80296800
(XEN) 000004761a686532 ffff828c8022c100 ffff83007f3de000 0000000080557450
(XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN) ffff828c80197ce3 0000000000000000 0000000000000000 0000000000000000
(XEN) 0000000000000000 0000000080557450 00000000ffdffc70 0000000000000000
(XEN) 0000000000000000 0000000000000000 0000000000000000 00000000e0b93b63
(XEN) 00000000ffdffc70 0000000000000002 00000000ffdffc50 0000000081f12fa0
(XEN) 0000000000000009 00000000f877d162 0000000000000000 0000000000000246
(XEN) 0000000080557434 0000000000000000 0000000000000000 0000000000000000
(XEN) 0000000000000000 0000000000000000 0000000000000001 ffff83007f3de000
(XEN) Xen call trace:
(XEN) [<ffff828c8019ac47>] __get_instruction_length+0x17/0x30
(XEN) [<ffff828c8019dd7b>] vmx_vmexit_handler+0x85b/0x15b0
(XEN) [<ffff828c801182ed>] schedule+0x1cd/0x590
(XEN) [<ffff828c8018ed5a>] pt_update_irq+0x4a/0xf0
(XEN) [<ffff828c8018f962>] vlapic_has_pending_irq+0x42/0x70
(XEN) [<ffff828c8018a5d8>] hvm_vcpu_has_pending_irq+0x78/0x90
(XEN) [<ffff828c80197f22>] vmx_intr_assist+0x62/0x220
(XEN) [<ffff828c80197ce3>] vmx_asm_do_vmentry+0x0/0xdd
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 1:
(XEN) Xen BUG at vmx.c:1461
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...
Also, this paragraph in the referenced section of the SDM gives me the
fear:
The cases of VM exits encountered during delivery of a software
interrupt, privileged software exception, or software exception include
those encountered during delivery of events injected as part of VM
entry (see Section 22.5.1.2). If the original event was injected as
part of VM entry, this field receives the value of the VM-entry
instruction length.
Does that mean that if we inject a fault and the guest routes it through
a task switch gate, we''ll see the length of VMENTER here? (And should
we be advancing EIP in that case anyway?)
Cheers,
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2009-Dec-02 12:09 UTC
[Xen-devel] Re: Crash with c/s 20097 (x86 vmx: Update EIP when appropriate during task switch)
On 02/12/2009 11:47, "Tim Deegan" <Tim.Deegan@eu.citrix.com> wrote:> The __get_instruction_length() _isn''t_ safe in those cases where we > later explicitly set inst_len to zero (and possibly in other cases).Argh, stupid, and probably my mistake. I''ll push the __get_instruction_length() inside the if()s.> The cases of VM exits encountered during delivery of a software > interrupt, privileged software exception, or software exception include > those encountered during delivery of events injected as part of VM > entry (see Section 22.5.1.2). If the original event was injected as > part of VM entry, this field receives the value of the VM-entry > instruction length. > > Does that mean that if we inject a fault and the guest routes it through > a task switch gate, we''ll see the length of VMENTER here? (And should > we be advancing EIP in that case anyway?)Fortunately we never inject software interrupts or exceptions. I reworked the code to avoid that a long time ago, due to just such concerns. Possibly we should even ASSERT as such in the event-injection functions. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Dec-02 13:42 UTC
Re: [Xen-devel] Re: Crash with c/s 20097 (x86 vmx: Update EIP when appropriate during task switch)
On 02/12/2009 12:09, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:> Argh, stupid, and probably my mistake. I''ll push the > __get_instruction_length() inside the if()s.I refactored the code as xen-unstable:20561. If that looks agreeable and correct to you then I will also backport to 3.4-testing.> Fortunately we never inject software interrupts or exceptions. I reworked > the code to avoid that a long time ago, due to just such concerns. Possibly > we should even ASSERT as such in the event-injection functions.I decided against this because we write to VM_ENTRY_INTR_INFO in a few places so it''s not nice and centralised. But it''s pretty obvious that SWInts and SWExcs are not in the picture since we never write VM_ENTRY_INSTRUCTION_LENGTH. So we''re obviously safe (or even more utterly broken :-) ). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2009-Dec-02 13:56 UTC
Re: [Xen-devel] Re: Crash with c/s 20097 (x86 vmx: Update EIP when appropriate during task switch)
At 13:42 +0000 on 02 Dec (1259761336), Keir Fraser wrote:> I refactored the code as xen-unstable:20561. If that looks agreeable and > correct to you then I will also backport to 3.4-testing.Looks fine to me.> > Fortunately we never inject software interrupts or exceptions. I reworked > > the code to avoid that a long time ago, due to just such concerns. Possibly > > we should even ASSERT as such in the event-injection functions. > > I decided against this because we write to VM_ENTRY_INTR_INFO in a few > places so it''s not nice and centralised. But it''s pretty obvious that SWInts > and SWExcs are not in the picture since we never write > VM_ENTRY_INSTRUCTION_LENGTH. So we''re obviously safe (or even more utterly > broken :-) ).That''s grand then. :) Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel