From patchwork Wed Jan 26 17:58:02 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Bader X-Patchwork-Id: 80534 X-Patchwork-Delegate: stefan.bader@canonical.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 96FB9B7106 for ; Thu, 27 Jan 2011 04:58:21 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Pi9d8-0005dm-I3; Wed, 26 Jan 2011 17:58:06 +0000 Received: from adelie.canonical.com ([91.189.90.139]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Pi9d6-0005dg-AZ for kernel-team@lists.ubuntu.com; Wed, 26 Jan 2011 17:58:04 +0000 Received: from hutte.canonical.com ([91.189.90.181]) by adelie.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1Pi9d6-00011o-4Z for ; Wed, 26 Jan 2011 17:58:04 +0000 Received: from p5b2e464b.dip.t-dialin.net ([91.46.70.75] helo=[192.168.2.121]) by hutte.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1Pi9d5-0000v4-QZ for kernel-team@lists.ubuntu.com; Wed, 26 Jan 2011 17:58:04 +0000 Message-ID: <4D4060AA.6070404@canonical.com> Date: Wed, 26 Jan 2011 18:58:02 +0100 From: Stefan Bader User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Lightning/1.0b2 Thunderbird/3.1.7 MIME-Version: 1.0 To: kernel-team@lists.ubuntu.com Subject: Re: Hardy SRU, xen unified block-device I/O interface back end can orphan devices, CVE-2010-3699 References: <20110126141609.D5926F89F8@sepang.rtg.net> <4D404FAA.90202@canonical.com> In-Reply-To: <4D404FAA.90202@canonical.com> X-Enigmail-Version: 1.1.2 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com On 01/26/2011 05:45 PM, Stefan Bader wrote: > Scary, this sort of pulls in silently a few other changes to Xen, which makes it > even harder. > > On 01/26/2011 03:16 PM, Tim Gardner wrote: >> The following changes since commit 11e175fe9030a484684a34f0b13040bf93ef2290: >> Dan Rosenberg (1): >> V4L/DVB: ivtvfb: prevent reading uninitialized stack memory, CVE-2010-4079 >> >> are available in the git repository at: >> >> git://kernel.ubuntu.com/rtg/ubuntu-hardy.git CVE-2010-3699 >> >> Tim Gardner (1): >> UBUNTU: xen unified block-device I/O interface back end can orphan devices, CVE-2010-3699 >> >> .../xen/patchset/020-xen-CVE-2010-3699.patch | 152 ++++++++++++++++++++ >> 1 files changed, 152 insertions(+), 0 deletions(-) >> create mode 100644 debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch >> >> From a42d61170c261228e31c95b98e42ec80444638d8 Mon Sep 17 00:00:00 2001 >> From: Tim Gardner >> Date: Tue, 25 Jan 2011 14:54:33 -0700 >> Subject: [PATCH] UBUNTU: xen unified block-device I/O interface back end can orphan devices, CVE-2010-3699 >> >> Excerpted from http://rhn.redhat.com/errata/RHSA-2011-0004.html: >> >> "A flaw was found in the Xenbus code for the unified block-device I/O >> interface back end. A privileged guest user could use this flaw to cause a >> denial of service on the host system running the Xen hypervisor." >> >> Most of this patch was developed from http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/59f097ef181b. >> A careful comparision of the patch and the actual contents of http://xenbits.xensource.com/linux-2.6.18-xen.hg >> showed that part of the original patch introduced a regression as can be seen from the Mercurial log. >> I've not implemented that piece of the patch. >> >> Signed-off-by: Tim Gardner >> --- >> .../xen/patchset/020-xen-CVE-2010-3699.patch | 152 ++++++++++++++++++++ >> 1 files changed, 152 insertions(+), 0 deletions(-) >> create mode 100644 debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch >> >> diff --git a/debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch b/debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch >> new file mode 100644 >> index 0000000..57dea44 >> --- /dev/null >> +++ b/debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch >> @@ -0,0 +1,152 @@ >> +diff --git a/drivers/xen/blkback/xenbus.c b/drivers/xen/blkback/xenbus.c >> +index f9feb43..1611d7a 100644 >> +--- a/drivers/xen/blkback/xenbus.c >> ++++ b/drivers/xen/blkback/xenbus.c >> +@@ -362,6 +362,11 @@ static void frontend_changed(struct xenbus_device *dev, >> + if (dev->state == XenbusStateConnected) >> + break; >> + >> ++ /* Enforce precondition before potential leak point. >> ++ * blkif_disconnect() is idempotent. >> ++ */ >> ++ blkif_disconnect(be->blkif); >> ++ >> + err = connect_ring(be); >> + if (err) >> + break; >> +@@ -379,6 +384,7 @@ static void frontend_changed(struct xenbus_device *dev, >> + break; >> + /* fall through if not online */ >> + case XenbusStateUnknown: >> ++ /* implies blkif_disconnect() via blkback_remove() */ >> + device_unregister(&dev->dev); >> + break; >> + >> +diff --git a/drivers/xen/blktap/xenbus.c b/drivers/xen/blktap/xenbus.c >> +index f9e3159..d5db1b5 100644 >> +--- a/drivers/xen/blktap/xenbus.c >> ++++ b/drivers/xen/blktap/xenbus.c >> +@@ -326,6 +326,18 @@ static void tap_backend_changed(struct xenbus_watch *watch, >> + tap_update_blkif_status(be->blkif); >> + } >> + >> ++ >> ++static void tap_blkif_disconnect(blkif_t *blkif) >> ++{ >> ++ if (blkif->xenblkd) { >> ++ kthread_stop(blkif->xenblkd); >> ++ blkif->xenblkd = NULL; >> ++ } >> ++ >> ++ /* idempotent */ > >> ++ tap_blkif_free(blkif); > > Maybe this is giving grief. In our version tap_blkif_free() is also calling > kmem_cache_free, while the version in the repo has this factored out into a > separate function which is only called on blktap_remove() > >> ++} >> ++ >> + /** >> + * Callback received when the frontend's state changes. >> + */ >> +@@ -354,6 +366,11 @@ static void tap_frontend_changed(struct xenbus_device *dev, >> + if (dev->state == XenbusStateConnected) >> + break; >> + >> ++ /* Enforce precondition before potential leak point. >> ++ * tap_blkif_disconnect() is idempotent. >> ++ */ >> ++ tap_blkif_disconnect(be->blkif); >> ++ >> + err = connect_ring(be); >> + if (err) >> + break; >> +@@ -361,10 +378,7 @@ static void tap_frontend_changed(struct xenbus_device *dev, >> + break; >> + >> + case XenbusStateClosing: >> +- if (be->blkif->xenblkd) { >> +- kthread_stop(be->blkif->xenblkd); >> +- be->blkif->xenblkd = NULL; >> +- } >> ++ tap_blkif_disconnect(be->blkif); >> + xenbus_switch_state(dev, XenbusStateClosing); >> + break; >> + >> +@@ -374,6 +388,9 @@ static void tap_frontend_changed(struct xenbus_device *dev, >> + break; >> + /* fall through if not online */ >> + case XenbusStateUnknown: >> ++ /* Implies the effects of blkif_disconnect() via >> ++ * blktap_remove(). >> ++ */ >> + device_unregister(&dev->dev); >> + break; >> + >> +diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c >> +index b55005c..fb84b4b 100644 >> +--- a/drivers/xen/netback/xenbus.c >> ++++ b/drivers/xen/netback/xenbus.c >> +@@ -32,6 +32,7 @@ >> + static int connect_rings(struct backend_info *); >> + static void connect(struct backend_info *); >> + static void backend_create_netif(struct backend_info *be); >> ++static void netback_disconnect(struct device *); >> + >> + static int netback_remove(struct xenbus_device *dev) >> + { >> +@@ -39,15 +40,22 @@ static int netback_remove(struct xenbus_device *dev) >> + >> + netback_remove_accelerators(be, dev); >> + >> +- if (be->netif) { >> +- netif_disconnect(be->netif); >> +- be->netif = NULL; >> +- } >> ++ netback_disconnect(&dev->dev); >> + kfree(be); >> + dev->dev.driver_data = NULL; >> + return 0; >> + } >> + >> ++static void netback_disconnect(struct device *xbdev_dev) >> ++{ >> ++ struct backend_info *be = xbdev_dev->driver_data; >> ++ >> ++ if (be->netif) { >> ++ kobject_uevent(&xbdev_dev->kobj, KOBJ_OFFLINE); >> ++ netif_disconnect(be->netif); >> ++ be->netif = NULL; >> ++ } >> ++} >> + >> + /** >> + * Entry point to this code when a new device is created. Allocate the basic >> +@@ -225,16 +233,17 @@ static void frontend_changed(struct xenbus_device *dev, >> + break; >> + >> + case XenbusStateConnected: >> ++ if (dev->state == XenbusStateConnected) >> ++ break; >> ++ >> ++ /* backend_create_netif() is idempotent */ >> + backend_create_netif(be); >> + if (be->netif) >> + connect(be); >> + break; >> + >> + case XenbusStateClosing: >> +- if (be->netif) { >> +- netif_disconnect(be->netif); >> +- be->netif = NULL; >> +- } >> ++ netback_disconnect(&dev->dev); >> + xenbus_switch_state(dev, XenbusStateClosing); >> + break; >> + >> +@@ -244,8 +253,7 @@ static void frontend_changed(struct xenbus_device *dev, >> + break; >> + /* fall through if not online */ >> + case XenbusStateUnknown: >> +- if (be->netif != NULL) >> +- kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE); >> ++ /* implies netback_disconnect() via netback_remove() */ >> + device_unregister(&dev->dev); >> + break; >> + > > I still have the problem of running "xm create" but the same issue happens when I use the kernel from current proposed. So that needs more investigation. This is the patch I have come up so far. From 25a05d703385a09feb95cce60c63b7078b013892 Mon Sep 17 00:00:00 2001 From: Tim Gardner Date: Tue, 25 Jan 2011 14:54:33 -0700 Subject: [PATCH] UBUNTU: xen unified block-device I/O interface back end can orphan devices, CVE-2010-3699 CVE-2010-3699 BugLink: http://bugs.launchpad.net/bugs/??? Excerpted from http://rhn.redhat.com/errata/RHSA-2011-0004.html: "A flaw was found in the Xenbus code for the unified block-device I/O interface back end. A privileged guest user could use this flaw to cause a denial of service on the host system running the Xen hypervisor." Most of this patch was developed from http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/59f097ef181b. A careful comparision of the patch and the actual contents of http://xenbits.xensource.com/linux-2.6.18-xen.hg showed that part of the original patch introduced a regression as can be seen from the Mercurial log. I've not implemented that piece of the patch. Signed-off-by: Tim Gardner --- .../xen/patchset/020-xen-CVE-2010-3699.patch | 187 ++++++++++++++++++++ 1 files changed, 187 insertions(+), 0 deletions(-) create mode 100644 debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch diff --git a/debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch b/debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch new file mode 100644 index 0000000..3ada6fa --- /dev/null +++ b/debian/binary-custom.d/xen/patchset/020-xen-CVE-2010-3699.patch @@ -0,0 +1,187 @@ +diff -Nurp custom-source-xen.orig/drivers/xen/blkback/xenbus.c custom-source-xen/drivers/xen/blkback/xenbus.c +--- custom-source-xen.orig/drivers/xen/blkback/xenbus.c 2011-01-26 18:31:29.866234416 +0100 ++++ custom-source-xen/drivers/xen/blkback/xenbus.c 2011-01-26 18:29:26.690234371 +0100 +@@ -362,6 +362,11 @@ static void frontend_changed(struct xenb + if (dev->state == XenbusStateConnected) + break; + ++ /* Enforce precondition before potential leak point. ++ * blkif_disconnect() is idempotent. ++ */ ++ blkif_disconnect(be->blkif); ++ + err = connect_ring(be); + if (err) + break; +@@ -379,6 +384,7 @@ static void frontend_changed(struct xenb + break; + /* fall through if not online */ + case XenbusStateUnknown: ++ /* implies blkif_disconnect() via blkback_remove() */ + device_unregister(&dev->dev); + break; + +diff -Nurp custom-source-xen.orig/drivers/xen/blktap/common.h custom-source-xen/drivers/xen/blktap/common.h +--- custom-source-xen.orig/drivers/xen/blktap/common.h 2011-01-26 18:29:25.974232874 +0100 ++++ custom-source-xen/drivers/xen/blktap/common.h 2011-01-26 18:32:10.622236863 +0100 +@@ -89,6 +89,7 @@ typedef struct blkif_st { + + blkif_t *tap_alloc_blkif(domid_t domid); + void tap_blkif_free(blkif_t *blkif); ++void tap_blkif_kmem_cache_free(blkif_t *blkif); + int tap_blkif_map(blkif_t *blkif, unsigned long shared_page, + unsigned int evtchn); + void tap_blkif_unmap(blkif_t *blkif); +diff -Nurp custom-source-xen.orig/drivers/xen/blktap/interface.c custom-source-xen/drivers/xen/blktap/interface.c +--- custom-source-xen.orig/drivers/xen/blktap/interface.c 2011-01-26 18:31:29.882243505 +0100 ++++ custom-source-xen/drivers/xen/blktap/interface.c 2011-01-26 18:29:26.690234371 +0100 +@@ -162,8 +162,15 @@ void tap_blkif_free(blkif_t *blkif) + { + atomic_dec(&blkif->refcnt); + wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0); ++ atomic_inc(&blkif->refcnt); + + tap_blkif_unmap(blkif); ++} ++ ++void tap_blkif_kmem_cache_free(blkif_t *blkif) ++{ ++ if (!atomic_dec_and_test(&blkif->refcnt)) ++ BUG(); + kmem_cache_free(blkif_cachep, blkif); + } + +diff -Nurp custom-source-xen.orig/drivers/xen/blktap/xenbus.c custom-source-xen/drivers/xen/blktap/xenbus.c +--- custom-source-xen.orig/drivers/xen/blktap/xenbus.c 2011-01-26 18:31:29.882243505 +0100 ++++ custom-source-xen/drivers/xen/blktap/xenbus.c 2011-01-26 18:29:26.690234371 +0100 +@@ -183,6 +183,7 @@ static int blktap_remove(struct xenbus_d + kthread_stop(be->blkif->xenblkd); + signal_tapdisk(be->blkif->dev_num); + tap_blkif_free(be->blkif); ++ tap_blkif_kmem_cache_free(be->blkif); + be->blkif = NULL; + } + kfree(be); +@@ -326,6 +327,18 @@ static void tap_backend_changed(struct x + tap_update_blkif_status(be->blkif); + } + ++ ++static void tap_blkif_disconnect(blkif_t *blkif) ++{ ++ if (blkif->xenblkd) { ++ kthread_stop(blkif->xenblkd); ++ blkif->xenblkd = NULL; ++ } ++ ++ /* idempotent */ ++ tap_blkif_free(blkif); ++} ++ + /** + * Callback received when the frontend's state changes. + */ +@@ -354,6 +367,11 @@ static void tap_frontend_changed(struct + if (dev->state == XenbusStateConnected) + break; + ++ /* Enforce precondition before potential leak point. ++ * tap_blkif_disconnect() is idempotent. ++ */ ++ tap_blkif_disconnect(be->blkif); ++ + err = connect_ring(be); + if (err) + break; +@@ -361,10 +379,7 @@ static void tap_frontend_changed(struct + break; + + case XenbusStateClosing: +- if (be->blkif->xenblkd) { +- kthread_stop(be->blkif->xenblkd); +- be->blkif->xenblkd = NULL; +- } ++ tap_blkif_disconnect(be->blkif); + xenbus_switch_state(dev, XenbusStateClosing); + break; + +@@ -374,6 +389,9 @@ static void tap_frontend_changed(struct + break; + /* fall through if not online */ + case XenbusStateUnknown: ++ /* Implies the effects of blkif_disconnect() via ++ * blktap_remove(). ++ */ + device_unregister(&dev->dev); + break; + +diff -Nurp custom-source-xen.orig/drivers/xen/netback/xenbus.c custom-source-xen/drivers/xen/netback/xenbus.c +--- custom-source-xen.orig/drivers/xen/netback/xenbus.c 2011-01-26 18:31:29.882243505 +0100 ++++ custom-source-xen/drivers/xen/netback/xenbus.c 2011-01-26 18:29:26.690234371 +0100 +@@ -32,6 +32,7 @@ + static int connect_rings(struct backend_info *); + static void connect(struct backend_info *); + static void backend_create_netif(struct backend_info *be); ++static void netback_disconnect(struct device *); + + static int netback_remove(struct xenbus_device *dev) + { +@@ -39,15 +40,22 @@ static int netback_remove(struct xenbus_ + + netback_remove_accelerators(be, dev); + +- if (be->netif) { +- netif_disconnect(be->netif); +- be->netif = NULL; +- } ++ netback_disconnect(&dev->dev); + kfree(be); + dev->dev.driver_data = NULL; + return 0; + } + ++static void netback_disconnect(struct device *xbdev_dev) ++{ ++ struct backend_info *be = xbdev_dev->driver_data; ++ ++ if (be->netif) { ++ kobject_uevent(&xbdev_dev->kobj, KOBJ_OFFLINE); ++ netif_disconnect(be->netif); ++ be->netif = NULL; ++ } ++} + + /** + * Entry point to this code when a new device is created. Allocate the basic +@@ -225,16 +233,17 @@ static void frontend_changed(struct xenb + break; + + case XenbusStateConnected: ++ if (dev->state == XenbusStateConnected) ++ break; ++ ++ /* backend_create_netif() is idempotent */ + backend_create_netif(be); + if (be->netif) + connect(be); + break; + + case XenbusStateClosing: +- if (be->netif) { +- netif_disconnect(be->netif); +- be->netif = NULL; +- } ++ netback_disconnect(&dev->dev); + xenbus_switch_state(dev, XenbusStateClosing); + break; + +@@ -244,8 +253,7 @@ static void frontend_changed(struct xenb + break; + /* fall through if not online */ + case XenbusStateUnknown: +- if (be->netif != NULL) +- kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE); ++ /* implies netback_disconnect() via netback_remove() */ + device_unregister(&dev->dev); + break; + -- 1.7.0.4