[bug report] fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

Hello Muhammad Usama Anjum,

Commit 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and
optionally clear info about PTEs") from Aug 21, 2023 (linux-next),
leads to the following Smatch static checker warning:

	fs/proc/task_mmu.c:2664 pagemap_scan_get_args()
	warn: potential user controlled sizeof overflow 'arg->vec_len * 24' '0-u64max * 24' type='ullong'

fs/proc/task_mmu.c
    2637 static int pagemap_scan_get_args(struct pm_scan_arg *arg,
    2638                                  unsigned long uarg)
    2639 {
    2640         if (copy_from_user(arg, (void __user *)uarg, sizeof(*arg)))

arg comes from the user

    2641                 return -EFAULT;
    2642 
    2643         if (arg->size != sizeof(struct pm_scan_arg))
    2644                 return -EINVAL;
    2645 
    2646         /* Validate requested features */
    2647         if (arg->flags & ~PM_SCAN_FLAGS)
    2648                 return -EINVAL;
    2649         if ((arg->category_inverted | arg->category_mask |
    2650              arg->category_anyof_mask | arg->return_mask) & ~PM_SCAN_CATEGORIES)
    2651                 return -EINVAL;
    2652 
    2653         arg->start = untagged_addr((unsigned long)arg->start);
    2654         arg->end = untagged_addr((unsigned long)arg->end);
    2655         arg->vec = untagged_addr((unsigned long)arg->vec);
    2656 
    2657         /* Validate memory pointers */
    2658         if (!IS_ALIGNED(arg->start, PAGE_SIZE))
    2659                 return -EINVAL;

We should probably check ->end here as well.

    2660         if (!access_ok((void __user *)(long)arg->start, arg->end - arg->start))

Otherwise we're checking access_ok() and then making ->end larger.  Maybe move
the arg->end = ALIGN(arg->end, PAGE_SIZE) before the access_ok() check?

    2661                 return -EFAULT;
    2662         if (!arg->vec && arg->vec_len)
    2663                 return -EINVAL;
--> 2664         if (arg->vec && !access_ok((void __user *)(long)arg->vec,
    2665                               arg->vec_len * sizeof(struct page_region)))

This "arg->vec_len * sizeof(struct page_region)" multiply could have an integer
overflow.

arg->vec_len is a u64 so size_add() won't work on a 32bit system.  I wonder if
size_add() should check for sizes larger than SIZE_MAX?

    2666                 return -EFAULT;
    2667 
    2668         /* Fixup default values */
    2669         arg->end = ALIGN(arg->end, PAGE_SIZE);
    2670         arg->walk_end = 0;
    2671         if (!arg->max_pages)
    2672                 arg->max_pages = ULONG_MAX;
    2673 
    2674         return 0;
    2675 }

regards,
dan carpenter