Hello, please see the answer below blue:


From:   Radim Krčmář <rkrc...@redhat.com>
To:     Li Kaihang <li.kaih...@zte.com.cn>,
Cc:     g...@kernel.org, pbonz...@redhat.com, t...@linutronix.de, 
mi...@redhat.com, h...@zytor.com, x...@kernel.org, k...@vger.kernel.org, 
linux-kernel@vger.kernel.org
Date:   2015-01-16 上午 02:09
Subject:        Re: [PATCH 1/1] arch/x86/kvm/vmx.c: Fix external interrupts 
inject directly bug with guestos RFLAGS.IF=0



2015-01-15 20:36+0800, Li Kaihang:
> This patch fix a external interrupt injecting bug in linux 3.19-rc4.

Was the bug introduced in earlier 3.19 release candidate?

Li Kaihang: Yes, we also find this problem in 2.6.

> GuestOS is running and handling some interrupt with RFLAGS.IF = 0 while a 
> external interrupt coming,
> then can lead to a vm exit,in this case,we must avoid inject this external 
> interrupt or it will generate
> a processor hardware exception causing virtual machine crash.

What is the source of this exception?  (Is there a reproducer?)

Li Kaihang: exception is produced by intel processor hardware because injecting 
a external interrupt vector is forbidden by intel processor when GuestOS 
RFLAGS.IF = 0,
            this need to be ensured by hypervisor software according to Intel 
64 and IA-32 Architectures Software Developer's Manual Volume 3.
            This bug has a certain probability, if code is designed to be very 
short between cli and sti in a guestos's interrupt processing, probability of 
occurrence
            is very low, this event is like moving trap, bug is produced that 
guestos is running between cli and sti instruction while a external interrupt 
coming, it
            may be verified by constructing a special guestos interrupt code. 
General OS running on kvm vm has also probability to hit this bug.

> Now, I show more details about this problem:
>
> A general external interrupt processing for a running virtual machine is 
> shown in the following:
>
> Step 1:
>      a ext intr gen a vm_exit --> vmx_complete_interrupts --> 
> __vmx_complete_interrupts --> case INTR_TYPE_EXT_INR: 
> kvm_queue_interrupt(vcpu, vector, type == INTR_TYPE_SOFT_INTR);
>
> Step 2:
>      kvm_x86_ops->handle_external_intr(vcpu);
>
> Step 3:
>      get back to vcpu_enter_guest after a while cycle,then run 
> inject_pending_event
>
> Step 4:
>      if (vcpu->arch.interrupt.pending) {
>                                kvm_x86_ops->set_irq(vcpu);
>                                return 0;
>                }
>
> Step 5:
>      kvm_x86_ops->run(vcpu) --> vm_entry inject vector to guestos IDT
>
> for the above steps, step 4 and 5 will be a processor hardware exception if 
> step1 happen while guestos RFLAGS.IF = 0, that is to say, guestos interrupt 
> is disabled.
> So we should add a logic to judge in step 1 whether a external interrupt need 
> to be pended then inject directly, in the process, we don't need to worry 
> about
> this external interrupt lost because the next Step 2 will handle and choose a 
> best chance to inject it by virtual interrupt controller.

Can you explain the relation between vectored events (Step 1) and
external interrupts (Step 2)?
(The bug happens when external interrupt arrives during event delivery?)

Li Kaihang: a external interrupt to running vm can trigger a vm_exit event 
handled in step 1, then this interrupt vector can be processed in step2
            kvm_x86_ops->handle_external_intr(vcpu) and this function can jump 
to HOSTOS IDT to complete external interrupt handling,external interrupt 
handler in HOSTOS
            IDT may inject the external interrupt into virtual interrupt 
controller if it has been registered to be needed by virtual machine.
            The Bug has never happened in step 1 and 2, but 
vcpu->arch.interrupt.pending is set in step 1, if this pending should not be 
injected, it also will be passed
            to step4 to complete the dangerous external interrupt injecting. 
Please see the above answer about what is "pending should not be injected"? Our 
solution
            is that clearing invalid external interrupt pending to prevent 
error inject pass by adding a logical judge in step 1.

Why isn't the delivered event lost?
(It should be different from the external interrupt.)

Li Kaihang: please refer to the above answer, a external interrupt in step1 
only can get to case INTR_TYPE_EXT_INR branch in patch code, so it should not 
affect other
            type events delivering, but there is another possibility that the 
external interrupt needed by running vm is not registered in hostos idt handler 
chain,of
            course, this situation is another problem, even so it is dangerous 
action to inject the external interrupt directly if not judge current guestos 
RFLAGS.IF
            state

Thanks.

>
>
> Signed-off-by: Li kaihang <li.kaih...@zte.com.cn>
> ---
>  arch/x86/kvm/vmx.c |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index d4c58d8..e8311ee 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7711,10 +7711,26 @@ static void __vmx_complete_interrupts(struct kvm_vcpu 
> *vcpu,
>                 break;
>         case INTR_TYPE_SOFT_INTR:
>                 vcpu->arch.event_exit_inst_len = vmcs_read32(instr_len_field);
> -               /* fall through */
> -       case INTR_TYPE_EXT_INTR:
> +               /*
> +               * As software and external interrupts may all get here,
> +               * we should separate soft intr from ext intr code,and this
> +               * will ensure that software interrupts handling process is not
> +               * affected by solving external interrupt invalid injecting.
> +               */
>                 kvm_queue_interrupt(vcpu, vector, type == 
> INTR_TYPE_SOFT_INTR);

(No need for 'type == INTR_TYPE_SOFT_INTR', we know it is true.)
 Li Kaihang: I agree
>                 break;
> +       case INTR_TYPE_EXT_INTR:
> +               /*
> +               * GuestOS is running and handling some interrupt with
> +               * RFLAGS.IF = 0 while a external interrupt coming,
> +               * then can lead a vm exit getting here,in this case,
> +               * we must avoid inject this external interrupt or it will
> +               * generate a processor hardware exception causing vm crash.
> +               */
> +               if (kvm_x86_ops->interrupt_allowed(vcpu))
> +                       kvm_queue_interrupt(vcpu, vector,
> +                                       type == INTR_TYPE_SOFT_INTR);

(And false here.)
Li Kaihang: I agree--------------------------------------------------------
ZTE Information Security Notice: The information contained in this mail (and 
any attachment transmitted herewith) is privileged and confidential and is 
intended for the exclusive use of the addressee(s).  If you are not an intended 
recipient, any disclosure, reproduction, distribution or other dissemination or 
use of the information contained is strictly prohibited.  If you have received 
this mail in error, please delete it and notify us immediately.
N�����r��y����b�X��ǧv�^�)޺{.n�+����{����zX����ܨ}���Ơz�&j:+v�������zZ+��+zf���h���~����i���z��w���?�����&�)ߢf��^jǫy�m��@A�a���
0��h���i

Reply via email to