| Message ID | 1296225409-21189-1-git-send-email-stefan.bader@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Stefan Bader |
| Headers | show |
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@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@gmail.com> > Signed-off-by: Linus Torvalds<torvalds@linux-foundation.org> > Signed-off-by: Andy Grover<andy.grover@oracle.com> > Signed-off-by: David S. Miller<davem@davemloft.net> > (backported from commit 1b1f693d7ad6d193862dcb1118540a030c5e761f upstream) > Signed-off-by: Stefan Bader<stefan.bader@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
applied and pushed
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);