[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] Xen Security Advisory 58 (CVE-2013-1432) - Page reference counting error due to XSA-4
From: Xen.org security team <security () xen ! org>
Date: 2013-06-26 13:19:17
Message-ID: E1Urpcz-000275-5T () xenbits ! xen ! org
[Download RAW message or body]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Xen Security Advisory CVE-2013-1432 / XSA-58
version 2
Page reference counting error due to XSA-45/CVE-2013-1918 fixes
UPDATES IN VERSION 2
====================
Public release. Credits section added.
ISSUE DESCRIPTION
=================
The XSA-45/CVE-2013-1918 patch making error handling paths preemptible broke
page reference counting by not retaining a reference on pages stored for
deferred cleanup. This would lead to the hypervisor prematurely attempting to
free the page, generally crashing upon finding the page still in use.
CREDITS
=======
Thanks to Andrew Cooper and the Citrix XenServer team for discovering
and reporting this vulnerability, and helping investigate it.
IMPACT
======
Malicious or buggy PV guest kernels can mount a denial of service attack
affecting the whole system. It can't be excluded that this could also be
exploited to mount a privilege escalation attack.
VULNERABLE SYSTEMS
==================
All Xen versions having the XSA-45/CVE-2013-1918 fixes applied are vulnerable.
The vulnerability is only exposed by PV guests.
MITIGATION
==========
Running only HVM guests, or PV guests with trusted kernels, will avoid this
vulnerability.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
xsa58-4.1.patch Xen 4.1.x
xsa58-4.2.patch Xen 4.2.x
xsa58-unstable.patch xen-unstable
$ sha256sum xsa58*.patch
3623ec87e5a2830f0d41de19a8e448d618954973c3264727a1f3a095f15a8641 xsa58-4.1.patch
194d6610fc38b767d643e5d58a1268f45921fb35e309b47aca6a388b861311c2 xsa58-4.2.patch
2c94b099d7144d03c0f7f44e892a521537fc040d11bc46f84a2438eece46a0f5 xsa58-unstable.patch
$
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iQEcBAEBAgAGBQJRyuoNAAoJEIP+FMlX6CvZY3EH/04uBhD797FdBhCRkq/y1ACc
Dvg1BRZ4lHkURDp97gD4Fdyf95Lw4qtniYBq8H/kpVPWJgN7+Dmj8uoluWhOI62Y
Q7a97CZ3O39VcuNRQnZG8c6dduGwMTzbJMkftG0CcltygAxVVRU4uHSG4+MHQ5PZ
N1xauljWrbw49iZz0shxZv4BA/1MQyuyZGFIpOaYoom0vV67pfrQJ2kgCMDUctmq
WXNkVcOiS7lwS/+++goPIboSEy6UJCIVrhZmL7GhbNfiznlOFVgExMttQRcUDi/D
4SS4ghl3IyB34TwoX1P7TPEeHGbfonObGpzBQNduBIJDM32nqO7P8097XG0j0Tw=
=aw1s
-----END PGP SIGNATURE-----
["xsa58-4.1.patch" (application/octet-stream)]
x86: fix page refcount handling in page table pin error path
In the original patch 7 of the series addressing XSA-45 I mistakenly
took the addition of the call to get_page_light() in alloc_page_type()
to cover two decrements that would happen: One for the PGT_partial bit
that is getting set along with the call, and the other for the page
reference the caller hold (and would be dropping on its error path).
But of course the additional page reference is tied to the PGT_partial
bit, and hence any caller of a function that may leave
->arch.old_guest_table non-NULL for error cleanup purposes has to make
sure a respective page reference gets retained.
Similar issues were then also spotted elsewhere: In effect all callers
of get_page_type_preemptible() need to deal with errors in similar
ways. To make sure error handling can work this way without leaking
page references, a respective assertion gets added to that function.
This is CVE-2013-1432 / XSA-58.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -798,6 +798,10 @@ int arch_set_info_guest(
if ( v->vcpu_id == 0 )
d->vm_assist = c(vm_assist);
+ rc = put_old_guest_table(current);
+ if ( rc )
+ return rc;
+
if ( !compat )
rc = (int)set_gdt(v, c.nat->gdt_frames, c.nat->gdt_ents);
#ifdef CONFIG_COMPAT
@@ -840,18 +844,24 @@ int arch_set_info_guest(
}
else
{
- /*
- * Since v->arch.guest_table{,_user} are both NULL, this effectively
- * is just a call to put_old_guest_table().
- */
if ( !compat )
- rc = vcpu_destroy_pagetables(v);
+ rc = put_old_guest_table(v);
if ( !rc )
rc = get_page_type_preemptible(cr3_page,
!compat ? PGT_root_page_table
: PGT_l3_page_table);
- if ( rc == -EINTR )
+ switch ( rc )
+ {
+ case -EINTR:
rc = -EAGAIN;
+ case -EAGAIN:
+ case 0:
+ break;
+ default:
+ if ( cr3_page == current->arch.old_guest_table )
+ cr3_page = NULL;
+ break;
+ }
}
if ( rc )
@@ -883,6 +893,11 @@ int arch_set_info_guest(
pagetable_get_page(v->arch.guest_table);
v->arch.guest_table = pagetable_null();
break;
+ default:
+ if ( cr3_page == current->arch.old_guest_table )
+ cr3_page = NULL;
+ case 0:
+ break;
}
}
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -682,7 +682,8 @@ static int get_page_and_type_from_pagenr
get_page_type_preemptible(page, type) :
(get_page_type(page, type) ? 0 : -EINVAL));
- if ( unlikely(rc) && partial >= 0 )
+ if ( unlikely(rc) && partial >= 0 &&
+ (!preemptible || page != current->arch.old_guest_table) )
put_page(page);
return rc;
@@ -2555,6 +2556,7 @@ int put_page_type_preemptible(struct pag
int get_page_type_preemptible(struct page_info *page, unsigned long type)
{
+ ASSERT(!current->arch.old_guest_table);
return __get_page_type(page, type, 1);
}
@@ -2765,7 +2767,7 @@ static void put_superpage(unsigned long
#endif
-static int put_old_guest_table(struct vcpu *v)
+int put_old_guest_table(struct vcpu *v)
{
int rc;
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -337,6 +337,7 @@ void put_page_type(struct page_info *pag
int get_page_type(struct page_info *page, unsigned long type);
int put_page_type_preemptible(struct page_info *page);
int get_page_type_preemptible(struct page_info *page, unsigned long type);
+int put_old_guest_table(struct vcpu *);
int get_page_from_l1e(
l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner);
void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner);
["xsa58-4.2.patch" (application/octet-stream)]
x86: fix page refcount handling in page table pin error path
In the original patch 7 of the series addressing XSA-45 I mistakenly
took the addition of the call to get_page_light() in alloc_page_type()
to cover two decrements that would happen: One for the PGT_partial bit
that is getting set along with the call, and the other for the page
reference the caller hold (and would be dropping on its error path).
But of course the additional page reference is tied to the PGT_partial
bit, and hence any caller of a function that may leave
->arch.old_guest_table non-NULL for error cleanup purposes has to make
sure a respective page reference gets retained.
Similar issues were then also spotted elsewhere: In effect all callers
of get_page_type_preemptible() need to deal with errors in similar
ways. To make sure error handling can work this way without leaking
page references, a respective assertion gets added to that function.
This is CVE-2013-1432 / XSA-58.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -941,6 +941,10 @@ int arch_set_info_guest(
if ( v->vcpu_id == 0 )
d->vm_assist = c(vm_assist);
+ rc = put_old_guest_table(current);
+ if ( rc )
+ return rc;
+
if ( !compat )
rc = (int)set_gdt(v, c.nat->gdt_frames, c.nat->gdt_ents);
#ifdef CONFIG_COMPAT
@@ -980,18 +984,24 @@ int arch_set_info_guest(
}
else
{
- /*
- * Since v->arch.guest_table{,_user} are both NULL, this effectively
- * is just a call to put_old_guest_table().
- */
if ( !compat )
- rc = vcpu_destroy_pagetables(v);
+ rc = put_old_guest_table(v);
if ( !rc )
rc = get_page_type_preemptible(cr3_page,
!compat ? PGT_root_page_table
: PGT_l3_page_table);
- if ( rc == -EINTR )
+ switch ( rc )
+ {
+ case -EINTR:
rc = -EAGAIN;
+ case -EAGAIN:
+ case 0:
+ break;
+ default:
+ if ( cr3_page == current->arch.old_guest_table )
+ cr3_page = NULL;
+ break;
+ }
}
if ( rc )
/* handled below */;
@@ -1018,6 +1028,11 @@ int arch_set_info_guest(
pagetable_get_page(v->arch.guest_table);
v->arch.guest_table = pagetable_null();
break;
+ default:
+ if ( cr3_page == current->arch.old_guest_table )
+ cr3_page = NULL;
+ case 0:
+ break;
}
}
if ( !rc )
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -718,7 +718,8 @@ static int get_page_and_type_from_pagenr
get_page_type_preemptible(page, type) :
(get_page_type(page, type) ? 0 : -EINVAL));
- if ( unlikely(rc) && partial >= 0 )
+ if ( unlikely(rc) && partial >= 0 &&
+ (!preemptible || page != current->arch.old_guest_table) )
put_page(page);
return rc;
@@ -2638,6 +2639,7 @@ int put_page_type_preemptible(struct pag
int get_page_type_preemptible(struct page_info *page, unsigned long type)
{
+ ASSERT(!current->arch.old_guest_table);
return __get_page_type(page, type, 1);
}
@@ -2848,7 +2850,7 @@ static void put_superpage(unsigned long
#endif
-static int put_old_guest_table(struct vcpu *v)
+int put_old_guest_table(struct vcpu *v)
{
int rc;
@@ -3253,7 +3255,8 @@ long do_mmuext_op(
rc = -EAGAIN;
else if ( rc != -EAGAIN )
MEM_LOG("Error while pinning mfn %lx", page_to_mfn(page));
- put_page(page);
+ if ( page != curr->arch.old_guest_table )
+ put_page(page);
break;
}
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -374,6 +374,7 @@ void put_page_type(struct page_info *pag
int get_page_type(struct page_info *page, unsigned long type);
int put_page_type_preemptible(struct page_info *page);
int get_page_type_preemptible(struct page_info *page, unsigned long type);
+int put_old_guest_table(struct vcpu *);
int get_page_from_l1e(
l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner);
void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner);
["xsa58-unstable.patch" (application/octet-stream)]
x86: fix page refcount handling in page table pin error path
In the original patch 7 of the series addressing XSA-45 I mistakenly
took the addition of the call to get_page_light() in alloc_page_type()
to cover two decrements that would happen: One for the PGT_partial bit
that is getting set along with the call, and the other for the page
reference the caller hold (and would be dropping on its error path).
But of course the additional page reference is tied to the PGT_partial
bit, and hence any caller of a function that may leave
->arch.old_guest_table non-NULL for error cleanup purposes has to make
sure a respective page reference gets retained.
Similar issues were then also spotted elsewhere: In effect all callers
of get_page_type_preemptible() need to deal with errors in similar
ways. To make sure error handling can work this way without leaking
page references, a respective assertion gets added to that function.
This is CVE-2013-1432 / XSA-58.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -829,6 +829,10 @@ int arch_set_info_guest(
if ( v->vcpu_id == 0 )
d->vm_assist = c(vm_assist);
+ rc = put_old_guest_table(current);
+ if ( rc )
+ return rc;
+
if ( !compat )
rc = (int)set_gdt(v, c.nat->gdt_frames, c.nat->gdt_ents);
else
@@ -864,18 +868,24 @@ int arch_set_info_guest(
}
else
{
- /*
- * Since v->arch.guest_table{,_user} are both NULL, this effectively
- * is just a call to put_old_guest_table().
- */
if ( !compat )
- rc = vcpu_destroy_pagetables(v);
+ rc = put_old_guest_table(v);
if ( !rc )
rc = get_page_type_preemptible(cr3_page,
!compat ? PGT_root_page_table
: PGT_l3_page_table);
- if ( rc == -EINTR )
+ switch ( rc )
+ {
+ case -EINTR:
rc = -EAGAIN;
+ case -EAGAIN:
+ case 0:
+ break;
+ default:
+ if ( cr3_page == current->arch.old_guest_table )
+ cr3_page = NULL;
+ break;
+ }
}
if ( rc )
/* handled below */;
@@ -901,6 +911,11 @@ int arch_set_info_guest(
pagetable_get_page(v->arch.guest_table);
v->arch.guest_table = pagetable_null();
break;
+ default:
+ if ( cr3_page == current->arch.old_guest_table )
+ cr3_page = NULL;
+ case 0:
+ break;
}
}
if ( !rc )
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -640,7 +640,8 @@ static int get_page_and_type_from_pagenr
get_page_type_preemptible(page, type) :
(get_page_type(page, type) ? 0 : -EINVAL));
- if ( unlikely(rc) && partial >= 0 )
+ if ( unlikely(rc) && partial >= 0 &&
+ (!preemptible || page != current->arch.old_guest_table) )
put_page(page);
return rc;
@@ -2427,6 +2428,7 @@ int put_page_type_preemptible(struct pag
int get_page_type_preemptible(struct page_info *page, unsigned long type)
{
+ ASSERT(!current->arch.old_guest_table);
return __get_page_type(page, type, 1);
}
@@ -2617,7 +2619,7 @@ static void put_superpage(unsigned long
return;
}
-static int put_old_guest_table(struct vcpu *v)
+int put_old_guest_table(struct vcpu *v)
{
int rc;
@@ -2988,7 +2990,8 @@ long do_mmuext_op(
rc = -EAGAIN;
else if ( rc != -EAGAIN )
MEM_LOG("Error while pinning mfn %lx", page_to_mfn(page));
- put_page(page);
+ if ( page != curr->arch.old_guest_table )
+ put_page(page);
break;
}
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -354,6 +354,7 @@ void put_page_type(struct page_info *pag
int get_page_type(struct page_info *page, unsigned long type);
int put_page_type_preemptible(struct page_info *page);
int get_page_type_preemptible(struct page_info *page, unsigned long type);
+int put_old_guest_table(struct vcpu *);
int get_page_from_l1e(
l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner);
void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner);
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic