Hardy SRU, xen unified block-device I/O interface back end can orphan devices, CVE-2010-3699
Stefan Bader
stefan.bader at canonical.com
Wed Jan 26 17:58:02 UTC 2011
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 <tim.gardner at 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 at 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;
>> +
>
>
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-UBUNTU-xen-unified-block-device-I-O-interface-back-e.patch
Type: text/x-diff
Size: 7636 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20110126/0f536e0b/attachment.patch>
More information about the kernel-team
mailing list