On Thu, Nov 01, 2018 at 12:38:22AM -0600, William Kucharski wrote:
>
>
> > On Oct 25, 2018, at 12:15 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> >
> > The concern here is that "gup->size" is a u64 and "nr_pages" is unsigned
> > long. On 32 bit systems we could trick the kernel into allocating fewer
> > pages than expected.
> >
> > Fixes: 64c349f4ae78 ("mm: add infrastructure for get_user_pages_fast() benchmarking")
> > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > ---
> > mm/gup_benchmark.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
> > index debf11388a60..5b42d3d4b60a 100644
> > --- a/mm/gup_benchmark.c
> > +++ b/mm/gup_benchmark.c
> > @@ -27,6 +27,9 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
> > int nr;
> > struct page **pages;
> >
> > + if (gup->size > ULONG_MAX)
> > + return -EINVAL;
> > +
> > nr_pages = gup->size / PAGE_SIZE;
> > pages = kvcalloc(nr_pages, sizeof(void *), GFP_KERNEL);
> > if (!pages)
>
> Given gup->size is in bytes, if your goal is to avoid an overflow of nr_pages on 32-bit
> systems, shouldn't you be checking something like:
>
> if ((gup_size / PAGE_SIZE) > ULONG_MAX)
My patch lets people allocate 4MB. (U32_MAX / 4096 * sizeof(void *)).
Surely, that's enough? I liked my check because it avoids the divide so
it's faster and it is a no-op on 64bit systems.
regards,
dan carpenter
