diff mbox

[Hardy] CVE-2010-4247: XEN: Add yield points to blktap and blkback

Message ID 1306937205-11184-1-git-send-email-stefan.bader@canonical.com
State New
Headers show

Commit Message

Stefan Bader June 1, 2011, 2:06 p.m. UTC
As far as I can see this only affects Hardy as that is the only place
that creates a dom0 kernel. The code itself seems to be present in the
lucid-ec2 tree, but as we do not support dom0 and those are drivers
for that. So I set the Lucid-ec2 status to not-affected, but I am thinking
of adding the changes anyway, just in case...

-Stefan

From 7610b848ef18bd8db8471b450f09bc24f7c5cf7e Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Wed, 1 Jun 2011 11:54:40 +0200
Subject: [PATCH] UBUNTU: XEN: Add yield points to blktap and blkback

CVE-2010-4247
BugLink: http://bugs.launchpad.net/bugs/791212

This adds a combined patch that consists of

http://xenbits.xensource.com/hg/linux-2.6.18-xen.hg/rev/77f831cbb91d

blkback: Request-processing loop is unbounded and hence requires a
yield point. Also, bad request type is a good cause to sleep for a
short while as the frontend has probably gone mad.

Patch by Steven Smith <steven.smith@eu.citrix.com>

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

and

http://xenbits.xensource.com/hg/linux-2.6.18-xen.hg/rev/7070d34f251c

blkback/blktap: Check for kthread_should_stop() in inner loop,
mdelaay() should be msleep(), and these changes belong in blktap as
well as blkback.
Based on comments and patches from Jan Beulich and Steven Smith.
Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 .../xen/patchset/021-xen-CVE-2010-4247.patch       |  127 ++++++++++++++++++++
 1 files changed, 127 insertions(+), 0 deletions(-)
 create mode 100644 debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch

Comments

Stefan Bader June 1, 2011, 2:27 p.m. UTC | #1
On 01.06.2011 16:06, Stefan Bader wrote:
> As far as I can see this only affects Hardy as that is the only place
> that creates a dom0 kernel. The code itself seems to be present in the
> lucid-ec2 tree, but as we do not support dom0 and those are drivers
> for that. So I set the Lucid-ec2 status to not-affected, but I am thinking
> of adding the changes anyway, just in case...

Looking again, I am not sure what I was looking at before. In fact this change 
is already present in lucid-ec2 (but still not used).

>
> -Stefan
>
>  From 7610b848ef18bd8db8471b450f09bc24f7c5cf7e Mon Sep 17 00:00:00 2001
> From: Stefan Bader<stefan.bader@canonical.com>
> Date: Wed, 1 Jun 2011 11:54:40 +0200
> Subject: [PATCH] UBUNTU: XEN: Add yield points to blktap and blkback
>
> CVE-2010-4247
> BugLink: http://bugs.launchpad.net/bugs/791212
>
> This adds a combined patch that consists of
>
> http://xenbits.xensource.com/hg/linux-2.6.18-xen.hg/rev/77f831cbb91d
>
> blkback: Request-processing loop is unbounded and hence requires a
> yield point. Also, bad request type is a good cause to sleep for a
> short while as the frontend has probably gone mad.
>
> Patch by Steven Smith<steven.smith@eu.citrix.com>
>
> Signed-off-by: Keir Fraser<keir.fraser@citrix.com>
>
> and
>
> http://xenbits.xensource.com/hg/linux-2.6.18-xen.hg/rev/7070d34f251c
>
> blkback/blktap: Check for kthread_should_stop() in inner loop,
> mdelaay() should be msleep(), and these changes belong in blktap as
> well as blkback.
> Based on comments and patches from Jan Beulich and Steven Smith.
> Signed-off-by: Keir Fraser<keir.fraser@citrix.com>
>
> Signed-off-by: Stefan Bader<stefan.bader@canonical.com>
> ---
>   .../xen/patchset/021-xen-CVE-2010-4247.patch       |  127 ++++++++++++++++++++
>   1 files changed, 127 insertions(+), 0 deletions(-)
>   create mode 100644 debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
>
> diff --git a/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch b/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
> new file mode 100644
> index 0000000..95a24b7
> --- /dev/null
> +++ b/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
> @@ -0,0 +1,127 @@
> +Fixes for CVE-2010-4247
> +
> +blkback: Request-processing loop is unbounded and hence requires a
> +yield point. Also, bad request type is a good cause to sleep for a
> +short while as the frontend has probably gone mad.
> +
> +Patch by Steven Smith<steven.smith@eu.citrix.com>
> +
> +Signed-off-by: Keir Fraser<keir.fraser@citrix.com>
> +
> +blkback/blktap: Check for kthread_should_stop() in inner loop,
> +mdelaay() should be msleep(), and these changes belong in blktap as
> +well as blkback.
> +Based on comments and patches from Jan Beulich and Steven Smith.
> +Signed-off-by: Keir Fraser<keir.fraser@citrix.com>
> +
> +Signed-off-by: Stefan Bader<stefan.bader@canonical.com>
> +
> +diff -Nurp custom-source-xen.orig/drivers/xen/blkback/blkback.c custom-source-xen/drivers/xen/blkback/blkback.c
> +--- custom-source-xen.orig/drivers/xen/blkback/blkback.c	2011-06-01 09:35:13.180006000 +0000
> ++++ custom-source-xen/drivers/xen/blkback/blkback.c	2011-06-01 09:46:55.470006001 +0000
> +@@ -309,7 +309,7 @@ static int do_block_io_op(blkif_t *blkif
> + 	rp = blk_rings->common.sring->req_prod;
> + 	rmb(); /* Ensure we see queued requests up to 'rp'. */
> +
> +-	while ((rc != rp)) {
> ++	while (rc != rp) {
> +
> + 		if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc))
> + 			break;
> +@@ -321,6 +321,11 @@ static int do_block_io_op(blkif_t *blkif
> + 			break;
> + 		}
> +
> ++		if (kthread_should_stop()) {
> ++			more_to_do = 1;
> ++			break;
> ++		}
> ++
> + 		switch (blkif->blk_protocol) {
> + 		case BLKIF_PROTOCOL_NATIVE:
> + 			memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req));
> +@@ -349,6 +354,9 @@ static int do_block_io_op(blkif_t *blkif
> + 			dispatch_rw_block_io(blkif,&req, pending_req);
> + 			break;
> + 		default:
> ++			/* A good sign something is wrong: sleep for a while to
> ++			 * avoid excessive CPU consumption by a bad guest. */
> ++			msleep(1);
> + 			DPRINTK("error: unknown block io operation [%d]\n",
> + 				req.operation);
> + 			make_response(blkif, req.id, req.operation,
> +@@ -356,7 +364,11 @@ static int do_block_io_op(blkif_t *blkif
> + 			free_req(pending_req);
> + 			break;
> + 		}
> ++
> ++		/* Yield point for this unbounded loop. */
> ++		cond_resched();
> + 	}
> ++
> + 	return more_to_do;
> + }
> +
> +@@ -507,7 +519,8 @@ static void dispatch_rw_block_io(blkif_t
> +  fail_response:
> + 	make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR);
> + 	free_req(pending_req);
> +-}
> ++	msleep(1); /* back off a bit */
> ++}
> +
> +
> +
> +diff -Nurp custom-source-xen.orig/drivers/xen/blktap/blktap.c custom-source-xen/drivers/xen/blktap/blktap.c
> +--- custom-source-xen.orig/drivers/xen/blktap/blktap.c	2011-06-01 09:35:13.190006000 +0000
> ++++ custom-source-xen/drivers/xen/blktap/blktap.c	2011-06-01 09:45:50.870006001 +0000
> +@@ -53,6 +53,7 @@
> + #include<linux/major.h>
> + #include<linux/gfp.h>
> + #include<linux/poll.h>
> ++#include<linux/delay.h>
> + #include<asm/tlbflush.h>
> +
> + #define MAX_TAP_DEV 256     /*the maximum number of tapdisk ring devices    */
> +@@ -1243,6 +1244,11 @@ static int do_block_io_op(blkif_t *blkif
> + 			break;
> + 		}
> +
> ++		if (kthread_should_stop()) {
> ++			more_to_do = 1;
> ++			break;
> ++		}
> ++
> + 		switch (blkif->blk_protocol) {
> + 		case BLKIF_PROTOCOL_NATIVE:
> + 			memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc),
> +@@ -1271,6 +1277,9 @@ static int do_block_io_op(blkif_t *blkif
> + 			break;
> +
> + 		default:
> ++			/* A good sign something is wrong: sleep for a while to
> ++			 * avoid excessive CPU consumption by a bad guest. */
> ++			msleep(1);
> + 			WPRINTK("unknown operation [%d]\n",
> + 				req.operation);
> + 			make_response(blkif, req.id, req.operation,
> +@@ -1278,6 +1287,9 @@ static int do_block_io_op(blkif_t *blkif
> + 			free_req(pending_req);
> + 			break;
> + 		}
> ++
> ++		/* Yield point for this unbounded loop. */
> ++		cond_resched();
> + 	}
> + 		
> + 	blktap_kick_user(blkif->dev_num);
> +@@ -1504,7 +1516,8 @@ static void dispatch_rw_block_io(blkif_t
> +  fail_response:
> + 	make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR);
> + 	free_req(pending_req);
> +-}
> ++	msleep(1); /* back off a bit */
> ++}
> +
> +
> +
Stefan Bader June 7, 2011, 2:52 p.m. UTC | #2
On 01.06.2011 16:06, Stefan Bader wrote:
> As far as I can see this only affects Hardy as that is the only place
> that creates a dom0 kernel. The code itself seems to be present in the
> lucid-ec2 tree, but as we do not support dom0 and those are drivers
> for that. So I set the Lucid-ec2 status to not-affected, but I am thinking
> of adding the changes anyway, just in case...
> 
> -Stefan
> 
> From 7610b848ef18bd8db8471b450f09bc24f7c5cf7e Mon Sep 17 00:00:00 2001
> From: Stefan Bader <stefan.bader@canonical.com>
> Date: Wed, 1 Jun 2011 11:54:40 +0200
> Subject: [PATCH] UBUNTU: XEN: Add yield points to blktap and blkback
> 
> CVE-2010-4247
> BugLink: http://bugs.launchpad.net/bugs/791212
> 
> This adds a combined patch that consists of
> 
> http://xenbits.xensource.com/hg/linux-2.6.18-xen.hg/rev/77f831cbb91d
> 
> blkback: Request-processing loop is unbounded and hence requires a
> yield point. Also, bad request type is a good cause to sleep for a
> short while as the frontend has probably gone mad.
> 
> Patch by Steven Smith <steven.smith@eu.citrix.com>
> 
> Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
> 
> and
> 
> http://xenbits.xensource.com/hg/linux-2.6.18-xen.hg/rev/7070d34f251c
> 
> blkback/blktap: Check for kthread_should_stop() in inner loop,
> mdelaay() should be msleep(), and these changes belong in blktap as
> well as blkback.
> Based on comments and patches from Jan Beulich and Steven Smith.
> Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
> 
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  .../xen/patchset/021-xen-CVE-2010-4247.patch       |  127 ++++++++++++++++++++
>  1 files changed, 127 insertions(+), 0 deletions(-)
>  create mode 100644 debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
> 
> diff --git a/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch b/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
> new file mode 100644
> index 0000000..95a24b7
> --- /dev/null
> +++ b/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
> @@ -0,0 +1,127 @@
> +Fixes for CVE-2010-4247
> +
> +blkback: Request-processing loop is unbounded and hence requires a
> +yield point. Also, bad request type is a good cause to sleep for a
> +short while as the frontend has probably gone mad.
> +
> +Patch by Steven Smith <steven.smith@eu.citrix.com>
> +
> +Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
> +
> +blkback/blktap: Check for kthread_should_stop() in inner loop,
> +mdelaay() should be msleep(), and these changes belong in blktap as
> +well as blkback.
> +Based on comments and patches from Jan Beulich and Steven Smith.
> +Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
> +
> +Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> +
> +diff -Nurp custom-source-xen.orig/drivers/xen/blkback/blkback.c custom-source-xen/drivers/xen/blkback/blkback.c
> +--- custom-source-xen.orig/drivers/xen/blkback/blkback.c	2011-06-01 09:35:13.180006000 +0000
> ++++ custom-source-xen/drivers/xen/blkback/blkback.c	2011-06-01 09:46:55.470006001 +0000
> +@@ -309,7 +309,7 @@ static int do_block_io_op(blkif_t *blkif
> + 	rp = blk_rings->common.sring->req_prod;
> + 	rmb(); /* Ensure we see queued requests up to 'rp'. */
> + 
> +-	while ((rc != rp)) {
> ++	while (rc != rp) {
> + 
> + 		if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc))
> + 			break;
> +@@ -321,6 +321,11 @@ static int do_block_io_op(blkif_t *blkif
> + 			break;
> + 		}
> + 
> ++		if (kthread_should_stop()) {
> ++			more_to_do = 1;
> ++			break;
> ++		}
> ++
> + 		switch (blkif->blk_protocol) {
> + 		case BLKIF_PROTOCOL_NATIVE:
> + 			memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req));
> +@@ -349,6 +354,9 @@ static int do_block_io_op(blkif_t *blkif
> + 			dispatch_rw_block_io(blkif, &req, pending_req);
> + 			break;
> + 		default:
> ++			/* A good sign something is wrong: sleep for a while to
> ++			 * avoid excessive CPU consumption by a bad guest. */
> ++			msleep(1);
> + 			DPRINTK("error: unknown block io operation [%d]\n",
> + 				req.operation);
> + 			make_response(blkif, req.id, req.operation,
> +@@ -356,7 +364,11 @@ static int do_block_io_op(blkif_t *blkif
> + 			free_req(pending_req);
> + 			break;
> + 		}
> ++
> ++		/* Yield point for this unbounded loop. */
> ++		cond_resched();
> + 	}
> ++
> + 	return more_to_do;
> + }
> + 
> +@@ -507,7 +519,8 @@ static void dispatch_rw_block_io(blkif_t
> +  fail_response:
> + 	make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR);
> + 	free_req(pending_req);
> +-} 
> ++	msleep(1); /* back off a bit */
> ++}
> + 
> + 
> + 
> +diff -Nurp custom-source-xen.orig/drivers/xen/blktap/blktap.c custom-source-xen/drivers/xen/blktap/blktap.c
> +--- custom-source-xen.orig/drivers/xen/blktap/blktap.c	2011-06-01 09:35:13.190006000 +0000
> ++++ custom-source-xen/drivers/xen/blktap/blktap.c	2011-06-01 09:45:50.870006001 +0000
> +@@ -53,6 +53,7 @@
> + #include <linux/major.h>
> + #include <linux/gfp.h>
> + #include <linux/poll.h>
> ++#include <linux/delay.h>
> + #include <asm/tlbflush.h>
> + 
> + #define MAX_TAP_DEV 256     /*the maximum number of tapdisk ring devices    */
> +@@ -1243,6 +1244,11 @@ static int do_block_io_op(blkif_t *blkif
> + 			break;
> + 		}
> + 
> ++		if (kthread_should_stop()) {
> ++			more_to_do = 1;
> ++			break;
> ++		}
> ++
> + 		switch (blkif->blk_protocol) {
> + 		case BLKIF_PROTOCOL_NATIVE:
> + 			memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc),
> +@@ -1271,6 +1277,9 @@ static int do_block_io_op(blkif_t *blkif
> + 			break;
> + 
> + 		default:
> ++			/* A good sign something is wrong: sleep for a while to
> ++			 * avoid excessive CPU consumption by a bad guest. */
> ++			msleep(1);
> + 			WPRINTK("unknown operation [%d]\n",
> + 				req.operation);
> + 			make_response(blkif, req.id, req.operation,
> +@@ -1278,6 +1287,9 @@ static int do_block_io_op(blkif_t *blkif
> + 			free_req(pending_req);
> + 			break;
> + 		}
> ++
> ++		/* Yield point for this unbounded loop. */
> ++		cond_resched();
> + 	}
> + 		
> + 	blktap_kick_user(blkif->dev_num);
> +@@ -1504,7 +1516,8 @@ static void dispatch_rw_block_io(blkif_t
> +  fail_response:
> + 	make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR);
> + 	free_req(pending_req);
> +-} 
> ++	msleep(1); /* back off a bit */
> ++}
> + 
> + 
> + 

Pushing this one as I did not see any replies, yet.

-Stefan
Tim Gardner June 7, 2011, 4:19 p.m. UTC | #3
On 06/07/2011 08:52 AM, Stefan Bader wrote:
> On 01.06.2011 16:06, Stefan Bader wrote:
>> As far as I can see this only affects Hardy as that is the only
>> place that creates a dom0 kernel. The code itself seems to be
>> present in the lucid-ec2 tree, but as we do not support dom0 and
>> those are drivers for that. So I set the Lucid-ec2 status to
>> not-affected, but I am thinking of adding the changes anyway, just
>> in case...
>>
>> -Stefan
<snip>
>
> Pushing this one as I did not see any replies, yet.
>
> -Stefan
>

It didn't get reviewed because I couldn't figure out what you wanted. As 
far as I can tell you're _thinking_ of adding changes to drivers that 
are not in fact even used, nor will they _ever_ be used since we won't 
have a Lucid DOM0 host unless we provide one via an LTS backported 
kernel. In which case the point is moot.

rtg
John Johansen June 8, 2011, 12:28 a.m. UTC | #4
On 06/07/2011 09:19 AM, Tim Gardner wrote:
> On 06/07/2011 08:52 AM, Stefan Bader wrote:
>> On 01.06.2011 16:06, Stefan Bader wrote:
>>> As far as I can see this only affects Hardy as that is the only
>>> place that creates a dom0 kernel. The code itself seems to be
>>> present in the lucid-ec2 tree, but as we do not support dom0 and
>>> those are drivers for that. So I set the Lucid-ec2 status to
>>> not-affected, but I am thinking of adding the changes anyway, just
>>> in case...
>>>
>>> -Stefan
> <snip>
>>
>> Pushing this one as I did not see any replies, yet.
>>
>> -Stefan
>>
> 
> It didn't get reviewed because I couldn't figure out what you wanted. As far as I can tell you're _thinking_ of adding changes to drivers that are not in fact even used, nor will they _ever_ be used since we won't have a Lucid DOM0 host unless we provide one via an LTS backported kernel. In which case the point is moot.
> 
As I read it, this is for Hardy where we do have dom0, and not Lucid because we don't support dom0 there.

My big question when looking at the patch was why combine the two patches its based off?  I found it hard to look at and compare the patch that way.
Stefan Bader June 8, 2011, 7:14 a.m. UTC | #5
On 07.06.2011 18:19, Tim Gardner wrote:
> On 06/07/2011 08:52 AM, Stefan Bader wrote:
>> On 01.06.2011 16:06, Stefan Bader wrote:
>>> As far as I can see this only affects Hardy as that is the only
>>> place that creates a dom0 kernel. The code itself seems to be
>>> present in the lucid-ec2 tree, but as we do not support dom0 and
>>> those are drivers for that. So I set the Lucid-ec2 status to
>>> not-affected, but I am thinking of adding the changes anyway, just
>>> in case...
>>>
>>> -Stefan
> <snip>
>>
>> Pushing this one as I did not see any replies, yet.
>>
>> -Stefan
>>
> 
> It didn't get reviewed because I couldn't figure out what you wanted. As far as
> I can tell you're _thinking_ of adding changes to drivers that are not in fact
> even used, nor will they _ever_ be used since we won't have a Lucid DOM0 host
> unless we provide one via an LTS backported kernel. In which case the point is
> moot.
> 
> rtg

Ok, to clarify, for Hardy we need it. For Lucid I was thinking of pulling them
in just in case but found out that actually they are there, so nothing to do there.

Stefan
Stefan Bader June 8, 2011, 7:18 a.m. UTC | #6
On 08.06.2011 02:28, John Johansen wrote:
> On 06/07/2011 09:19 AM, Tim Gardner wrote:
>> On 06/07/2011 08:52 AM, Stefan Bader wrote:
>>> On 01.06.2011 16:06, Stefan Bader wrote:
>>>> As far as I can see this only affects Hardy as that is the only
>>>> place that creates a dom0 kernel. The code itself seems to be
>>>> present in the lucid-ec2 tree, but as we do not support dom0 and
>>>> those are drivers for that. So I set the Lucid-ec2 status to
>>>> not-affected, but I am thinking of adding the changes anyway, just
>>>> in case...
>>>>
>>>> -Stefan
>> <snip>
>>>
>>> Pushing this one as I did not see any replies, yet.
>>>
>>> -Stefan
>>>
>>
>> It didn't get reviewed because I couldn't figure out what you wanted. As far as I can tell you're _thinking_ of adding changes to drivers that are not in fact even used, nor will they _ever_ be used since we won't have a Lucid DOM0 host unless we provide one via an LTS backported kernel. In which case the point is moot.
>>
> As I read it, this is for Hardy where we do have dom0, and not Lucid because we don't support dom0 there.
> 
> My big question when looking at the patch was why combine the two patches its based off?  I found it hard to look at and compare the patch that way.
> 

Mostly because generating new patches for the custom binary build is a pita. And
then one patch added some blkback parts which the second partially corrected and
additionally added them to blktap. So I was slightly lazy there with the slight
(imo) benefit of having a patch that adds the same code patterns to two files.

-Stefan
Tim Gardner June 8, 2011, 12:56 p.m. UTC | #7
On 06/07/2011 06:28 PM, John Johansen wrote:
> On 06/07/2011 09:19 AM, Tim Gardner wrote:
>> On 06/07/2011 08:52 AM, Stefan Bader wrote:
>>> On 01.06.2011 16:06, Stefan Bader wrote:
>>>> As far as I can see this only affects Hardy as that is the
>>>> only place that creates a dom0 kernel. The code itself seems to
>>>> be present in the lucid-ec2 tree, but as we do not support dom0
>>>> and those are drivers for that. So I set the Lucid-ec2 status
>>>> to not-affected, but I am thinking of adding the changes
>>>> anyway, just in case...
>>>>
>>>> -Stefan
>> <snip>
>>>
>>> Pushing this one as I did not see any replies, yet.
>>>
>>> -Stefan
>>>
>>
>> It didn't get reviewed because I couldn't figure out what you
>> wanted. As far as I can tell you're _thinking_ of adding changes to
>> drivers that are not in fact even used, nor will they _ever_ be
>> used since we won't have a Lucid DOM0 host unless we provide one
>> via an LTS backported kernel. In which case the point is moot.
>>
> As I read it, this is for Hardy where we do have dom0, and not Lucid
> because we don't support dom0 there.
>
> My big question when looking at the patch was why combine the two
> patches its based off?  I found it hard to look at and compare the
> patch that way.
>

Ah, I must not have had my coffee yet. I had lucid-ec2 on the brain and 
couldn't figure out why you'd want to apply Dom0 patches therein.

rtg
Stefan Bader June 9, 2011, 1:01 p.m. UTC | #8
Hm, I might need more coffee but I cannot really tell whether I got ACKs or
"needs rewrite" for the Hardy patch...

-Stefan
Tim Gardner June 9, 2011, 1:09 p.m. UTC | #9
On 06/09/2011 07:01 AM, Stefan Bader wrote:
> Hm, I might need more coffee but I cannot really tell whether I got ACKs or
> "needs rewrite" for the Hardy patch...
>
> -Stefan
>

Presumably there are Dom0 test cases to exercise this code? It otherwise 
looks fine.

Acked-by: Tim Gardner <tim.gardner@canonical.com>
John Johansen June 9, 2011, 4:25 p.m. UTC | #10
On 06/09/2011 06:01 AM, Stefan Bader wrote:
> Hm, I might need more coffee but I cannot really tell whether I got ACKs or
> "needs rewrite" for the Hardy patch...
> 
oops, sorry I meant to include

Acked-by: John Johansen <john.johansen@canonical.com>
Stefan Bader June 9, 2011, 5:09 p.m. UTC | #11
Applied and pushed to hardy master-next
diff mbox

Patch

diff --git a/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch b/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
new file mode 100644
index 0000000..95a24b7
--- /dev/null
+++ b/debian/binary-custom.d/xen/patchset/021-xen-CVE-2010-4247.patch
@@ -0,0 +1,127 @@ 
+Fixes for CVE-2010-4247
+
+blkback: Request-processing loop is unbounded and hence requires a
+yield point. Also, bad request type is a good cause to sleep for a
+short while as the frontend has probably gone mad.
+
+Patch by Steven Smith <steven.smith@eu.citrix.com>
+
+Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
+
+blkback/blktap: Check for kthread_should_stop() in inner loop,
+mdelaay() should be msleep(), and these changes belong in blktap as
+well as blkback.
+Based on comments and patches from Jan Beulich and Steven Smith.
+Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
+
+Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
+
+diff -Nurp custom-source-xen.orig/drivers/xen/blkback/blkback.c custom-source-xen/drivers/xen/blkback/blkback.c
+--- custom-source-xen.orig/drivers/xen/blkback/blkback.c	2011-06-01 09:35:13.180006000 +0000
++++ custom-source-xen/drivers/xen/blkback/blkback.c	2011-06-01 09:46:55.470006001 +0000
+@@ -309,7 +309,7 @@ static int do_block_io_op(blkif_t *blkif
+ 	rp = blk_rings->common.sring->req_prod;
+ 	rmb(); /* Ensure we see queued requests up to 'rp'. */
+ 
+-	while ((rc != rp)) {
++	while (rc != rp) {
+ 
+ 		if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc))
+ 			break;
+@@ -321,6 +321,11 @@ static int do_block_io_op(blkif_t *blkif
+ 			break;
+ 		}
+ 
++		if (kthread_should_stop()) {
++			more_to_do = 1;
++			break;
++		}
++
+ 		switch (blkif->blk_protocol) {
+ 		case BLKIF_PROTOCOL_NATIVE:
+ 			memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req));
+@@ -349,6 +354,9 @@ static int do_block_io_op(blkif_t *blkif
+ 			dispatch_rw_block_io(blkif, &req, pending_req);
+ 			break;
+ 		default:
++			/* A good sign something is wrong: sleep for a while to
++			 * avoid excessive CPU consumption by a bad guest. */
++			msleep(1);
+ 			DPRINTK("error: unknown block io operation [%d]\n",
+ 				req.operation);
+ 			make_response(blkif, req.id, req.operation,
+@@ -356,7 +364,11 @@ static int do_block_io_op(blkif_t *blkif
+ 			free_req(pending_req);
+ 			break;
+ 		}
++
++		/* Yield point for this unbounded loop. */
++		cond_resched();
+ 	}
++
+ 	return more_to_do;
+ }
+ 
+@@ -507,7 +519,8 @@ static void dispatch_rw_block_io(blkif_t
+  fail_response:
+ 	make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR);
+ 	free_req(pending_req);
+-} 
++	msleep(1); /* back off a bit */
++}
+ 
+ 
+ 
+diff -Nurp custom-source-xen.orig/drivers/xen/blktap/blktap.c custom-source-xen/drivers/xen/blktap/blktap.c
+--- custom-source-xen.orig/drivers/xen/blktap/blktap.c	2011-06-01 09:35:13.190006000 +0000
++++ custom-source-xen/drivers/xen/blktap/blktap.c	2011-06-01 09:45:50.870006001 +0000
+@@ -53,6 +53,7 @@
+ #include <linux/major.h>
+ #include <linux/gfp.h>
+ #include <linux/poll.h>
++#include <linux/delay.h>
+ #include <asm/tlbflush.h>
+ 
+ #define MAX_TAP_DEV 256     /*the maximum number of tapdisk ring devices    */
+@@ -1243,6 +1244,11 @@ static int do_block_io_op(blkif_t *blkif
+ 			break;
+ 		}
+ 
++		if (kthread_should_stop()) {
++			more_to_do = 1;
++			break;
++		}
++
+ 		switch (blkif->blk_protocol) {
+ 		case BLKIF_PROTOCOL_NATIVE:
+ 			memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc),
+@@ -1271,6 +1277,9 @@ static int do_block_io_op(blkif_t *blkif
+ 			break;
+ 
+ 		default:
++			/* A good sign something is wrong: sleep for a while to
++			 * avoid excessive CPU consumption by a bad guest. */
++			msleep(1);
+ 			WPRINTK("unknown operation [%d]\n",
+ 				req.operation);
+ 			make_response(blkif, req.id, req.operation,
+@@ -1278,6 +1287,9 @@ static int do_block_io_op(blkif_t *blkif
+ 			free_req(pending_req);
+ 			break;
+ 		}
++
++		/* Yield point for this unbounded loop. */
++		cond_resched();
+ 	}
+ 		
+ 	blktap_kick_user(blkif->dev_num);
+@@ -1504,7 +1516,8 @@ static void dispatch_rw_block_io(blkif_t
+  fail_response:
+ 	make_response(blkif, req->id, req->operation, BLKIF_RSP_ERROR);
+ 	free_req(pending_req);
+-} 
++	msleep(1); /* back off a bit */
++}
+ 
+ 
+