Karmic, Lucid, Maverick: SRU: CVE-2010-3865

Tim Gardner tim.gardner at canonical.com
Fri Jan 28 15:01:34 UTC 2011


On 01/28/2011 07:36 AM, Stefan Bader wrote:
> Code has changed reasonable since Maverick but same patch applies to
> all older affected releases.
>
>  From 2a8309b4f615072025743fae22147be5aa8e86cd Mon Sep 17 00:00:00 2001
> From: Linus Torvalds<torvalds at linux-foundation.org>
> Date: Thu, 28 Oct 2010 15:40:55 +0000
> Subject: [PATCH] net: fix rds_iovec page count overflow
>
> BugLink: http://bugs.launchpad.net/bugs/709153
> CVE-2010-3865
>
> As reported by Thomas Pollet, the rdma page counting can overflow.  We
> get the rdma sizes in 64-bit unsigned entities, but then limit it to
> UINT_MAX bytes and shift them down to pages (so with a possible "+1" for
> an unaligned address).
>
> So each individual page count fits comfortably in an 'unsigned int' (not
> even close to overflowing into signed), but as they are added up, they
> might end up resulting in a signed return value. Which would be wrong.
>
> Catch the case of tot_pages turning negative, and return the appropriate
> error code.
>
> Reported-by: Thomas Pollet<thomas.pollet at gmail.com>
> Signed-off-by: Linus Torvalds<torvalds at linux-foundation.org>
> Signed-off-by: Andy Grover<andy.grover at oracle.com>
> Signed-off-by: David S. Miller<davem at davemloft.net>
> (backported from commit 1b1f693d7ad6d193862dcb1118540a030c5e761f upstream)
> Signed-off-by: Stefan Bader<stefan.bader at canonical.com>
> ---
>   net/rds/rdma.c |   10 ++++++++++
>   1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/net/rds/rdma.c b/net/rds/rdma.c
> index 3998967..0a403a7 100644
> --- a/net/rds/rdma.c
> +++ b/net/rds/rdma.c
> @@ -500,6 +500,16 @@ static struct rds_rdma_op *rds_rdma_prepare(struct rds_sock *rs,
>
>   		max_pages = max(nr, max_pages);
>   		nr_pages += nr;
> +
> +		/*
> +		 * nr for one entry in limited to (UINT_MAX>>PAGE_SHIFT)+1
> +		 * so nr_pages cannot overflow without first going negative.
> +		 * If nr cannot overflow then max_pages should be ok.
> +		 */
> +		if (nr_pages<  0) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>   	}
>
>   	pages = kcalloc(max_pages, sizeof(struct page *), GFP_KERNEL);

I'm kind of uncomfortable comparing an 'unsigned int' against 0. IIRC 
the results are somewhat compiler dependent. Wouldn't it be clearer if 
it was 'if (nr_pages >= INT_MAX)' ?

rtg
-- 
Tim Gardner tim.gardner at canonical.com




More information about the kernel-team mailing list