mbox

Hardy SRU, xen unified block-device I/O interface back end can orphan devices, CVE-2010-3699

Message ID 20110126141609.D5926F89F8@sepang.rtg.net
State Accepted
Delegated to: Stefan Bader
Headers show

Pull-request

git://kernel.ubuntu.com/rtg/ubuntu-hardy.git CVE-2010-3699

Message

Tim Gardner Jan. 26, 2011, 2:16 p.m. UTC 
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 <tim.gardner@canonical.com>
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 <tim.gardner@canonical.com>
---
 .../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

Comments

Tim Gardner Jan. 26, 2011, 2:23 p.m. UTC | #1 
Re-pushed with a proper BugLink in the commit message.
Stefan Bader Jan. 26, 2011, 4:45 p.m. UTC | #2 
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 <tim.gardner@canonical.com>
> 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 <tim.gardner@canonical.com>
> ---
>  .../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;
> +
Tim Gardner Jan. 27, 2011, 6:34 p.m. UTC | #3 
applied and pushed