| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANRm+CyprXnLbtWGJuAecY6DOgNyFJmzb-JkWL-W2=+TTiBiSw@mail.gmail.com>
Date: Wed, 23 Aug 2017 18:16:19 +0800
From: Wanpeng Li <kernellwp@...il.com>
To: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
kvm <kvm@...r.kernel.org>
Cc: Paolo Bonzini <pbonzini@...hat.com>,
Radim Krčmář <rkrcmar@...hat.com>,
Wanpeng Li <wanpeng.li@...mail.com>
Subject: Re: [PATCH 2/2] KVM: X86: Fix loss of exception which has not yet injected
2017-08-23 18:12 GMT+08:00 Wanpeng Li <kernellwp@...il.com>:
> From: Wanpeng Li <wanpeng.li@...mail.com>
>
> vmx_complete_interrupts() assumes that the exception is always injected,
> so it would be dropped by kvm_clear_exception_queue(). This patch separates
> exception.pending from exception.injected, exception.inject represents the
> exception is injected or the exception should be reinjected due to vmexit
> occurs during event delivery in VMX non-root operation. exception.pending
> represents the exception is queued and will be cleared when injecting the
> exception to the guest. So exception.pending and exception.injected can
> cooperate to guarantee exception will not be lost.
>
> Reported-by: Radim Krčmář <rkrcmar@...hat.com>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Radim Krčmář <rkrcmar@...hat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@...mail.com>
I will complete the qemu part for VCPU_EVENTS after this patch is in
an acceptable state.
Regards,
Wanpeng Li
> ---
> Documentation/virtual/kvm/api.txt | 2 +-
> arch/x86/include/asm/kvm_host.h | 2 +-
> arch/x86/include/uapi/asm/kvm.h | 2 +-
> arch/x86/kvm/vmx.c | 4 ++--
> arch/x86/kvm/x86.c | 21 ++++++++++++++++-----
> arch/x86/kvm/x86.h | 4 ++--
> tools/arch/x86/include/uapi/asm/kvm.h | 2 +-
> 7 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index e63a35f..6bfb178 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -830,9 +830,9 @@ states of the vcpu.
> struct kvm_vcpu_events {
> struct {
> __u8 injected;
> + __u8 pending;
> __u8 nr;
> __u8 has_error_code;
> - __u8 pad;
> __u32 error_code;
> } exception;
> struct {
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e1e6f00..f3899a3 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -546,8 +546,8 @@ struct kvm_vcpu_arch {
>
> struct kvm_queued_exception {
> bool pending;
> + bool injected;
> bool has_error_code;
> - bool reinject;
> u8 nr;
> u32 error_code;
> u8 nested_apf;
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index c2824d0..7c55916 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -296,9 +296,9 @@ struct kvm_reinject_control {
> struct kvm_vcpu_events {
> struct {
> __u8 injected;
> + __u8 pending;
> __u8 nr;
> __u8 has_error_code;
> - __u8 pad;
> __u32 error_code;
> } exception;
> struct {
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index a56e7f3..f96829c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2510,7 +2510,7 @@ static int vmx_queue_exception(struct kvm_vcpu *vcpu)
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> unsigned nr = vcpu->arch.exception.nr;
> bool has_error_code = vcpu->arch.exception.has_error_code;
> - bool reinject = vcpu->arch.exception.reinject;
> + bool reinject = vcpu->arch.exception.injected;
> u32 error_code = vcpu->arch.exception.error_code;
> u32 intr_info = nr | INTR_INFO_VALID_MASK;
>
> @@ -10891,7 +10891,7 @@ static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
> u32 idt_vectoring;
> unsigned int nr;
>
> - if (vcpu->arch.exception.pending && vcpu->arch.exception.reinject) {
> + if (vcpu->arch.exception.injected) {
> nr = vcpu->arch.exception.nr;
> idt_vectoring = nr | VECTORING_INFO_VALID_MASK;
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7414c26..5c680f1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -389,15 +389,18 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu,
>
> kvm_make_request(KVM_REQ_EVENT, vcpu);
>
> - if (!vcpu->arch.exception.pending) {
> + if (!vcpu->arch.exception.pending ||
> + !vcpu->arch.exception.injected) {
> queue:
> if (has_error && !is_protmode(vcpu))
> has_error = false;
> - vcpu->arch.exception.pending = true;
> + if (reinject)
> + vcpu->arch.exception.injected = true;
> + else
> + vcpu->arch.exception.pending = true;
> vcpu->arch.exception.has_error_code = has_error;
> vcpu->arch.exception.nr = nr;
> vcpu->arch.exception.error_code = error_code;
> - vcpu->arch.exception.reinject = reinject;
> return;
> }
>
> @@ -3069,12 +3072,12 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
> struct kvm_vcpu_events *events)
> {
> process_nmi(vcpu);
> + events->exception.injected = vcpu->arch.exception.injected;
> events->exception.injected =
> vcpu->arch.exception.pending &&
> !kvm_exception_is_soft(vcpu->arch.exception.nr);
> events->exception.nr = vcpu->arch.exception.nr;
> events->exception.has_error_code = vcpu->arch.exception.has_error_code;
> - events->exception.pad = 0;
> events->exception.error_code = vcpu->arch.exception.error_code;
>
> events->interrupt.injected =
> @@ -3125,6 +3128,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
> return -EINVAL;
>
> process_nmi(vcpu);
> + vcpu->arch.exception.injected = events->exception.injected;
> vcpu->arch.exception.pending = events->exception.injected;
> vcpu->arch.exception.nr = events->exception.nr;
> vcpu->arch.exception.has_error_code = events->exception.has_error_code;
> @@ -6341,7 +6345,8 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
> int r;
>
> /* try to reinject previous events if any */
> - if (vcpu->arch.exception.pending) {
> + if (vcpu->arch.exception.pending ||
> + vcpu->arch.exception.injected) {
> trace_kvm_inj_exception(vcpu->arch.exception.nr,
> vcpu->arch.exception.has_error_code,
> vcpu->arch.exception.error_code);
> @@ -6357,6 +6362,11 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
> }
>
> r = kvm_x86_ops->queue_exception(vcpu);
> + if (r == 0 && vcpu->arch.exception.pending &&
> + !vcpu->arch.exception.injected) {
> + vcpu->arch.exception.pending = false;
> + vcpu->arch.exception.injected = true;
> + }
> return r;
> }
>
> @@ -7727,6 +7737,7 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> vcpu->arch.nmi_injected = false;
> kvm_clear_interrupt_queue(vcpu);
> kvm_clear_exception_queue(vcpu);
> + vcpu->arch.exception.pending = false;
>
> memset(vcpu->arch.db, 0, sizeof(vcpu->arch.db));
> kvm_update_dr0123(vcpu);
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 1134603..e6ec0de 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -11,7 +11,7 @@
>
> static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
> {
> - vcpu->arch.exception.pending = false;
> + vcpu->arch.exception.injected = false;
> }
>
> static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vector,
> @@ -29,7 +29,7 @@ static inline void kvm_clear_interrupt_queue(struct kvm_vcpu *vcpu)
>
> static inline bool kvm_event_needs_reinjection(struct kvm_vcpu *vcpu)
> {
> - return vcpu->arch.exception.pending || vcpu->arch.interrupt.pending ||
> + return vcpu->arch.exception.injected || vcpu->arch.interrupt.pending ||
> vcpu->arch.nmi_injected;
> }
>
> diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
> index c2824d0..7c55916 100644
> --- a/tools/arch/x86/include/uapi/asm/kvm.h
> +++ b/tools/arch/x86/include/uapi/asm/kvm.h
> @@ -296,9 +296,9 @@ struct kvm_reinject_control {
> struct kvm_vcpu_events {
> struct {
> __u8 injected;
> + __u8 pending;
> __u8 nr;
> __u8 has_error_code;
> - __u8 pad;
> __u32 error_code;
> } exception;
> struct {
> --
> 2.7.4
>
Powered by blists - more mailing lists