From: Keir Fraser <keir@xen.org>
Date: Tue, 23 Nov 2010 13:58:38 +0000 (+0000)
Subject: blkback/blktap/netback: Fix CVE-2010-3699
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=ef9e715c419c25f9103d706abb103e503551a6df;p=people%2Froyger%2Flinux-2.6.18-xen.git

blkback/blktap/netback: Fix CVE-2010-3699

A guest can cause the backend driver to leak a kernel
thread. Such leaked threads hold references to the device, whichmakes
the device impossible to tear down. If shut down, the guest remains a
zombie domain, the xenwatch process hangs, and most xm commands will
stop working.

This patch tries to do the following, for all of netback, blkback,
blktap:
    - identify/extract idempotent teardown operations,
    - add/move the invocation of said teardown operation
      right before we're about to allocate new resources in the
      Connected states.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

diff --git a/drivers/xen/blkback/xenbus.c b/drivers/xen/blkback/xenbus.c
index f0995acb..cbbb8fc8 100644
--- a/drivers/xen/blkback/xenbus.c
+++ b/drivers/xen/blkback/xenbus.c
@@ -380,6 +380,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;
@@ -397,6 +402,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 e0df8954..746882e3 100644
--- a/drivers/xen/blktap/xenbus.c
+++ b/drivers/xen/blktap/xenbus.c
@@ -339,6 +339,18 @@ static void tap_backend_changed(struct xenbus_watch *watch,
 	tap_update_blkif_status(be->blkif);
 }
 
+
+static void 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.
  */
@@ -367,6 +379,11 @@ static void tap_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;
@@ -374,11 +391,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_free(be->blkif);
+		blkif_disconnect(be->blkif);
 		xenbus_switch_state(dev, XenbusStateClosing);
 		break;
 
@@ -388,6 +401,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 44ac8be5..ba169b72 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,16 +40,22 @@ static int netback_remove(struct xenbus_device *dev)
 
 	netback_remove_accelerators(be, dev);
 
-	if (be->netif) {
-		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
-		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
@@ -234,17 +241,19 @@ static void frontend_changed(struct xenbus_device *dev,
 	case XenbusStateConnected:
 		if (dev->state == XenbusStateConnected)
 			break;
+
+		/* Enforce precondition before potential leak point.
+		 * netback_disconnect() is idempotent.
+		 */
+		netback_disconnect(&dev->dev);
+
 		backend_create_netif(be);
 		if (be->netif)
 			connect(be);
 		break;
 
 	case XenbusStateClosing:
-		if (be->netif) {
-			kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
-			netif_disconnect(be->netif);
-			be->netif = NULL;
-		}
+		netback_disconnect(&dev->dev);
 		xenbus_switch_state(dev, XenbusStateClosing);
 		break;
 
@@ -254,6 +263,7 @@ static void frontend_changed(struct xenbus_device *dev,
 			break;
 		/* fall through if not online */
 	case XenbusStateUnknown:
+		/* implies netback_disconnect() via netback_remove() */
 		device_unregister(&dev->dev);
 		break;