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 f0995acbdc992330a7496429e04bd0fb92227f1d..cbbb8fc8f858289fa16a4986d36f29b358f8c468 100644 (file)
--- 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 e0df8954557b6e700fabf4d0de3774205d193b45..746882e30fed23b2870d3bfba86a84f25e903220 100644 (file)
--- 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 44ac8be5384734ed54c312adac9cebd993673db1..ba169b72f9c1e2d037e82b50de1abfb4843914af 100644 (file)
--- 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;