| Message ID | 1547074093-6066-2-git-send-email-tyhicks@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2018-16882 - Nested KVM DoS | expand |
On 09.01.19 23:48, Tyler Hicks wrote: > From: Cfir Cohen <cfir@google.com> > > nested_get_vmcs12_pages() processes the posted_intr address in vmcs12. It > caches the kmap()ed page object and pointer, however, it doesn't handle > errors correctly: it's possible to cache a valid pointer, then release > the page and later dereference the dangling pointer. > > I was able to reproduce with the following steps: > > 1. Call vmlaunch with valid posted_intr_desc_addr but an invalid > MSR_EFER. This causes nested_get_vmcs12_pages() to cache the kmap()ed > pi_desc_page and pi_desc. Later the invalid EFER value fails > check_vmentry_postreqs() which fails the first vmlaunch. > > 2. Call vmlanuch with a valid EFER but an invalid posted_intr_desc_addr > (I set it to 2G - 0x80). The second time we call nested_get_vmcs12_pages > pi_desc_page is unmapped and released and pi_desc_page is set to NULL > (the "shouldn't happen" clause). Due to the invalid > posted_intr_desc_addr, kvm_vcpu_gpa_to_page() fails and > nested_get_vmcs12_pages() returns. It doesn't return an error value so > vmlaunch proceeds. Note that at this time we have a dangling pointer in > vmx->nested.pi_desc and POSTED_INTR_DESC_ADDR in L0's vmcs. > > 3. Issue an IPI in L2 guest code. This triggers a call to > vmx_complete_nested_posted_interrupt() and pi_test_and_clear_on() which > dereferences the dangling pointer. > > Vulnerable code requires nested and enable_apicv variables to be set to > true. The host CPU must also support posted interrupts. > > Fixes: 5e2f30b756a37 "KVM: nVMX: get rid of nested_get_page()" > Cc: stable@vger.kernel.org > Reviewed-by: Andy Honig <ahonig@google.com> > Signed-off-by: Cfir Cohen <cfir@google.com> > Reviewed-by: Liran Alon <liran.alon@oracle.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > CVE-2018-16882 > > (cherry picked from commit c2dd5146e9fe1f22c77c1b011adf84eea0245806) > Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > arch/x86/kvm/vmx.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 9fcc3ec3ab78..cb8c74ab0586 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -11028,6 +11028,8 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu, > kunmap(vmx->nested.pi_desc_page); > kvm_release_page_dirty(vmx->nested.pi_desc_page); > vmx->nested.pi_desc_page = NULL; > + vmx->nested.pi_desc = NULL; > + vmcs_write64(POSTED_INTR_DESC_ADDR, -1ull); > } > page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->posted_intr_desc_addr); > if (is_error_page(page)) >
On 1/9/19 11:48 PM, Tyler Hicks wrote: > From: Cfir Cohen <cfir@google.com> > > nested_get_vmcs12_pages() processes the posted_intr address in vmcs12. It > caches the kmap()ed page object and pointer, however, it doesn't handle > errors correctly: it's possible to cache a valid pointer, then release > the page and later dereference the dangling pointer. > > I was able to reproduce with the following steps: > > 1. Call vmlaunch with valid posted_intr_desc_addr but an invalid > MSR_EFER. This causes nested_get_vmcs12_pages() to cache the kmap()ed > pi_desc_page and pi_desc. Later the invalid EFER value fails > check_vmentry_postreqs() which fails the first vmlaunch. > > 2. Call vmlanuch with a valid EFER but an invalid posted_intr_desc_addr > (I set it to 2G - 0x80). The second time we call nested_get_vmcs12_pages > pi_desc_page is unmapped and released and pi_desc_page is set to NULL > (the "shouldn't happen" clause). Due to the invalid > posted_intr_desc_addr, kvm_vcpu_gpa_to_page() fails and > nested_get_vmcs12_pages() returns. It doesn't return an error value so > vmlaunch proceeds. Note that at this time we have a dangling pointer in > vmx->nested.pi_desc and POSTED_INTR_DESC_ADDR in L0's vmcs. > > 3. Issue an IPI in L2 guest code. This triggers a call to > vmx_complete_nested_posted_interrupt() and pi_test_and_clear_on() which > dereferences the dangling pointer. > > Vulnerable code requires nested and enable_apicv variables to be set to > true. The host CPU must also support posted interrupts. > > Fixes: 5e2f30b756a37 "KVM: nVMX: get rid of nested_get_page()" > Cc: stable@vger.kernel.org > Reviewed-by: Andy Honig <ahonig@google.com> > Signed-off-by: Cfir Cohen <cfir@google.com> > Reviewed-by: Liran Alon <liran.alon@oracle.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > CVE-2018-16882 > > (cherry picked from commit c2dd5146e9fe1f22c77c1b011adf84eea0245806) > Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > arch/x86/kvm/vmx.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 9fcc3ec3ab78..cb8c74ab0586 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -11028,6 +11028,8 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu, > kunmap(vmx->nested.pi_desc_page); > kvm_release_page_dirty(vmx->nested.pi_desc_page); > vmx->nested.pi_desc_page = NULL; > + vmx->nested.pi_desc = NULL; > + vmcs_write64(POSTED_INTR_DESC_ADDR, -1ull); > } > page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->posted_intr_desc_addr); > if (is_error_page(page))
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 9fcc3ec3ab78..cb8c74ab0586 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -11028,6 +11028,8 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu, kunmap(vmx->nested.pi_desc_page); kvm_release_page_dirty(vmx->nested.pi_desc_page); vmx->nested.pi_desc_page = NULL; + vmx->nested.pi_desc = NULL; + vmcs_write64(POSTED_INTR_DESC_ADDR, -1ull); } page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->posted_intr_desc_addr); if (is_error_page(page))