[xen] XSA-45/CVE-2013-1918 breaks page reference counting let pygrub handle set default="${next_entry}" li

myoung myoung at fedoraproject.org
Wed Jun 26 18:49:23 UTC 2013


commit 145d87ab249d2f19ec7db4e17ed510ea32948c53
Author: Michael Young <m.a.young at durham.ac.uk>
Date:   Wed Jun 26 19:46:54 2013 +0100

    XSA-45/CVE-2013-1918 breaks page reference counting
    let pygrub handle set default="${next_entry}" line in F19
    libxl: Set vfb and vkb devid if not done so by the caller

 pygrub.next_entryfix.patch |   13 +++++
 xen.setdevid.patch         |   86 +++++++++++++++++++++++++++++
 xen.spec                   |   17 +++++-
 xsa58-4.2.patch            |  129 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 243 insertions(+), 2 deletions(-)
---
diff --git a/pygrub.next_entryfix.patch b/pygrub.next_entryfix.patch
new file mode 100644
index 0000000..e9eb782
--- /dev/null
+++ b/pygrub.next_entryfix.patch
@@ -0,0 +1,13 @@
+diff --git a/tools/pygrub/src/GrubConf.py b/tools/pygrub/src/GrubConf.py
+index 629951f..6324c62 100644
+--- a/tools/pygrub/src/GrubConf.py
++++ b/tools/pygrub/src/GrubConf.py
+@@ -427,6 +427,8 @@ class Grub2ConfigFile(_GrubConfigFile):
+                 if self.commands[com] is not None:
+                     if arg.strip() == "${saved_entry}":
+                         arg = "0"
++                    elif arg.strip() == "${next_entry}":
++                        arg = "0"
+                     setattr(self, self.commands[com], arg.strip())
+                 else:
+                     logging.info("Ignored directive %s" %(com,))
diff --git a/xen.setdevid.patch b/xen.setdevid.patch
new file mode 100644
index 0000000..15fe3da
--- /dev/null
+++ b/xen.setdevid.patch
@@ -0,0 +1,86 @@
+--- xen-4.2.2/tools/libxl/libxl.c.orig	2013-06-26 19:10:38.850364477 +0100
++++ xen-4.2.2/tools/libxl/libxl.c	2013-06-26 19:10:45.953275678 +0100
+@@ -1710,6 +1710,26 @@
+     return;
+ }
+ 
++/* common function to get next device id */
++static int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device)
++{
++    char *dompath, **l;
++    unsigned int nb;
++    int nextid = -1;
++
++    if (!(dompath = libxl__xs_get_dompath(gc, domid)))
++        return nextid;
++
++    l = libxl__xs_directory(gc, XBT_NULL,
++                            GCSPRINTF("%s/device/%s", dompath, device), &nb);
++    if (l == NULL || nb == 0)
++        nextid = 0;
++    else
++        nextid = strtoul(l[nb - 1], NULL, 10) + 1;
++
++    return nextid;
++}
++
+ /******************************************************************************/
+ 
+ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
+@@ -2550,8 +2570,7 @@
+     flexarray_t *front;
+     flexarray_t *back;
+     libxl__device *device;
+-    char *dompath, **l;
+-    unsigned int nb, rc;
++    unsigned int rc;
+ 
+     rc = libxl__device_nic_setdefault(gc, nic, domid);
+     if (rc) goto out;
+@@ -2568,17 +2587,10 @@
+     }
+ 
+     if (nic->devid == -1) {
+-        if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
++        if ((nic->devid = libxl__device_nextid(gc, domid, "vif")) < 0) {
+             rc = ERROR_FAIL;
+             goto out_free;
+         }
+-        if (!(l = libxl__xs_directory(gc, XBT_NULL,
+-                                     libxl__sprintf(gc, "%s/device/vif", dompath), &nb)) ||
+-            nb == 0) {
+-            nic->devid = 0;
+-        } else {
+-            nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;
+-        }
+     }
+ 
+     GCNEW(device);
+@@ -2976,6 +2988,13 @@
+         goto out_free;
+     }
+ 
++    if (vkb->devid == -1) {
++        if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb")) < 0) {
++            rc = ERROR_FAIL;
++            goto out_free;
++        }
++    }
++
+     rc = libxl__device_from_vkb(gc, domid, vkb, &device);
+     if (rc != 0) goto out_free;
+ 
+@@ -3078,6 +3097,13 @@
+         goto out_free;
+     }
+ 
++    if (vfb->devid == -1) {
++        if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb")) < 0) {
++            rc = ERROR_FAIL;
++            goto out_free;
++        }
++    }
++
+     rc = libxl__device_from_vfb(gc, domid, vfb, &device);
+     if (rc != 0) goto out_free;
+ 
diff --git a/xen.spec b/xen.spec
index 5e7ee0e..312d193 100644
--- a/xen.spec
+++ b/xen.spec
@@ -27,7 +27,7 @@
 Summary: Xen is a virtual machine monitor
 Name:    xen
 Version: 4.2.2
-Release: 9%{?dist}
+Release: 10%{?dist}
 Group:   Development/Libraries
 License: GPLv2+ and LGPLv2+ and BSD
 URL:     http://xen.org/
@@ -116,6 +116,9 @@ Patch99: xsa55-4.20022-libxc-check-blob-size-before-proceeding-in-xc_dom_ch.patc
 Patch101: xsa55-4.20023-libxc-Better-range-check-in-xc_dom_alloc_segment.patch
 Patch102: xsa57-4.2.patch
 Patch103: xen.git-934a5253d932b6f67fe40fc48975a2b0117e4cce.patch
+Patch104: xsa58-4.2.patch
+Patch105: pygrub.next_entryfix.patch
+Patch106: xen.setdevid.patch
 
 Patch100: xen-configure-xend.patch
 
@@ -323,6 +326,9 @@ manage Xen virtual machines.
 %patch101 -p1
 %patch102 -p1
 %patch103 -p1
+%patch104 -p1
+%patch105 -p1
+%patch106 -p1
 
 %patch100 -p1
 
@@ -812,11 +818,18 @@ rm -rf %{buildroot}
 %endif
 
 %changelog
+* Wed Jun 26 2013 Michael Young <m.a.young at durham.ac.uk> - 4.2.2-10
+- XSA-45/CVE-2013-1918 breaks page reference counting [XSA-58,
+  CVE-2013-1432] (#978383)
+- let pygrub handle set default="${next_entry}" line in F19 (#978036)
+- libxl: Set vfb and vkb devid if not done so by the caller (#977987)
+
 * Mon Jun 24 2013 Michael Young <m.a.young at durham.ac.uk> - 4.2.2-9
 - add upstream patch for PCI passthrough problems after XSA-46 (#977310)
 
 * Fri Jun 21 2013 Michael Young <m.a.young at durham.ac.uk> - 4.2.2-8
-- xenstore permissions not set correctly by libxl [XSA-57] (#976779)
+- xenstore permissions not set correctly by libxl [XSA-57,
+  CVE-2013-2211] (#976779)
 
 * Fri Jun 14 2013 Michael Young <m.a.young at durham.ac.uk> - 4.2.2-7
 - Revised fixes for [XSA-55, CVE-2013-2194 CVE-2013-2195
diff --git a/xsa58-4.2.patch b/xsa58-4.2.patch
new file mode 100644
index 0000000..1ea3aaa
--- /dev/null
+++ b/xsa58-4.2.patch
@@ -0,0 +1,129 @@
+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 at citrix.com>
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+Tested-by: Andrew Cooper <andrew.cooper3 at citrix.com>
+Reviewed-by: Tim Deegan <tim at 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);


More information about the scm-commits mailing list