Re: [PATCH] mm/gup_benchmark: prevent integer overflow in ioctl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> 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)

instead?

    William Kucharski




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux