lkml.org 
[lkml]   [2011]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [stable] [74/74] net: fix rds_iovec page count overflow
On 11-04-13 11:51 AM, Greg KH wrote:
> 2.6.32-longterm review patch. If anyone has any objections, please let us know.
>
> ------------------
>
> From: Linus Torvalds <torvalds@linux-foundation.org>
>
> commit 1b1f693d7ad6d193862dcb1118540a030c5e761f upstream.
>
> 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>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>
> ---
> net/rds/rdma.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> --- a/net/rds/rdma.c
> +++ b/net/rds/rdma.c
> @@ -473,6 +473,14 @@ static struct rds_rdma_op *rds_rdma_prep
>
> max_pages = max(nr, max_pages);
> nr_pages += nr;
> +
> + /*
> + * nr_pages for one entry is limited to (UINT_MAX>>PAGE_SHIFT)+1,
> + * so tot_pages cannot overflow without first going negative.
> + */
> + if ((int)nr_pages < 0)

Sorry if this doesn't make the review cutoff; just noticed it now.

A cosmetic note -- I think the comment no longer matches the code for
the backport, in that it is now misleading, and should instead be:

* nr for one entry is limited to (UINT_MAX>>PAGE_SHIFT)+1,
* so nr_pages cannot overflow without first going negative.

For context, the original upstream was:

tot_pages += nr_pages;
+
+ /*
+ * nr_pages for one entry is limited to (UINT_MAX>>PAGE_SHIFT)+1,
+ * so tot_pages cannot overflow without first going negative.
+ */
+ if ((int)tot_pages < 0)
+ return -EINVAL;

Paul.


\
 
 \ /
  Last update: 2011-04-15 18:57    [W:0.223 / U:3.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site