[PATCH 0/1][SRU][B/C] CVE-2018-16882 - Nested KVM DoS

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1][SRU][B/C] CVE-2018-16882 - Nested KVM DoS

Tyler Hicks-2
https://people.canonical.com/~ubuntu-security/cve/2018/CVE-2018-16882.html

 A use after free issue was found in the way Linux kernel's KVM hypervisor
 processed posted interrupts, when nested(=1) virtualization is enabled. In
 nested_get_vmcs12_pages(), in case of an error while processing posted
 interrupt address, it unmaps the 'pi_desc_page' without resetting 'pi_desc'
 descriptor address. Which is latter used in pi_test_and_clear_on(). A guest
 user/process could use this flaw to crash the host kernel resulting in DoS.

This is a clean cherry pick to Bionic and Cosmic. Disco already has the patch
applied. I've smoke tested this patch by booting nested KVM instances using,
both, the Bionic and Cosmic kernels.

Tyler


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][SRU][B/C] KVM: Fix UAF in nested posted interrupt processing

Tyler Hicks-2
From: Cfir Cohen <[hidden email]>

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: [hidden email]
Reviewed-by: Andy Honig <[hidden email]>
Signed-off-by: Cfir Cohen <[hidden email]>
Reviewed-by: Liran Alon <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>

CVE-2018-16882

(cherry picked from commit c2dd5146e9fe1f22c77c1b011adf84eea0245806)
Signed-off-by: Tyler Hicks <[hidden email]>
---
 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))
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 1/1][SRU][B/C] KVM: Fix UAF in nested posted interrupt processing

Stefan Bader-2
On 09.01.19 23:48, Tyler Hicks wrote:

> From: Cfir Cohen <[hidden email]>
>
> 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: [hidden email]
> Reviewed-by: Andy Honig <[hidden email]>
> Signed-off-by: Cfir Cohen <[hidden email]>
> Reviewed-by: Liran Alon <[hidden email]>
> Signed-off-by: Paolo Bonzini <[hidden email]>
>
> CVE-2018-16882
>
> (cherry picked from commit c2dd5146e9fe1f22c77c1b011adf84eea0245806)
> Signed-off-by: Tyler Hicks <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  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))
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 1/1][SRU][B/C] KVM: Fix UAF in nested posted interrupt processing

Kleber Souza
In reply to this post by Tyler Hicks-2
On 1/9/19 11:48 PM, Tyler Hicks wrote:

> From: Cfir Cohen <[hidden email]>
>
> 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: [hidden email]
> Reviewed-by: Andy Honig <[hidden email]>
> Signed-off-by: Cfir Cohen <[hidden email]>
> Reviewed-by: Liran Alon <[hidden email]>
> Signed-off-by: Paolo Bonzini <[hidden email]>
>
> CVE-2018-16882
>
> (cherry picked from commit c2dd5146e9fe1f22c77c1b011adf84eea0245806)
> Signed-off-by: Tyler Hicks <[hidden email]>


Acked-by: Kleber Sacilotto de Souza <[hidden email]>


> ---
>  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))



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/1][SRU][B/C] CVE-2018-16882 - Nested KVM DoS

Kleber Souza
In reply to this post by Tyler Hicks-2
On 1/9/19 11:48 PM, Tyler Hicks wrote:

> https://people.canonical.com/~ubuntu-security/cve/2018/CVE-2018-16882.html
>
>  A use after free issue was found in the way Linux kernel's KVM hypervisor
>  processed posted interrupts, when nested(=1) virtualization is enabled. In
>  nested_get_vmcs12_pages(), in case of an error while processing posted
>  interrupt address, it unmaps the 'pi_desc_page' without resetting 'pi_desc'
>  descriptor address. Which is latter used in pi_test_and_clear_on(). A guest
>  user/process could use this flaw to crash the host kernel resulting in DoS.
>
> This is a clean cherry pick to Bionic and Cosmic. Disco already has the patch
> applied. I've smoke tested this patch by booting nested KVM instances using,
> both, the Bionic and Cosmic kernels.
>
> Tyler
>
>
Applied to bionic/master-next and cosmic/master-next branches.

Thanks,
Kleber


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team