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

[Tim Gardner]

On 01/28/2011 08:12 AM, Stefan Bader wrote:
> On 01/28/2011 04:01 PM, Tim Gardner wrote:
>> 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
>
> Likely better. Just made it>INT_MAX (though that should not make that much
> difference).
>

This change behaves as expected. See attached sample program.

Acked-by: Tim Gardner <tim.gardner at canonical.com>

rtg

-- 
Tim Gardner tim.gardner at canonical.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: c.c
Type: text/x-csrc
Size: 403 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20110128/eb100e80/attachment.c>