Re: [PATCH] mm/hmm/test: Reject overflowing page counts

[Alistair Popple]

On 2026-09-16 at 02:37 +1000, Dan Carpenter <error27@xxxxxxxxx> wrote...
> The number of pages comes directly from userspace.  Shifting a value
> larger than ULONG_MAX >> PAGE_SHIFT discards its high bits before the
> existing end-address check.  A request for a huge number of pages can
> therefore be accepted and processed as a much smaller request.
> 
> Reject page counts that cannot be represented as a byte size before
> performing the shift.
> 
> So far as ChatGPT and I can tell this doesn't cause an issue in
> practice but preventing this integer overflow is the correct thing to
> do.

Agree on both counts. So far as I can tell every subsequent function using cmd
just uses npages to calculate a size to calculate an end address. Eg:

	unsigned long size = cmd->npages << PAGE_SHIFT;

	start = cmd->addr;
	end = start + size;

And this is all just for a specific userspace directed test program, which
currently never asks for a huge number of pages. This makes the fix easy because
it won't have been causing any test failures nor will it introduce any.

> Fixes: b2ef9f5a5cb3 ("mm/hmm/test: add selftest driver for HMM")
> Assisted-by: ChatGPT:gpt-5

Minor nit: I think the style changed recently to:

Assisted-by: LLM [TOOL1] [TOOL2]

Where TOOL is optional.

> Signed-off-by: Dan Carpenter <error27@xxxxxxxxx>
> ---
>  lib/test_hmm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> index 6911daa9f854..b409c42bfe6f 100644
> --- a/lib/test_hmm.c
> +++ b/lib/test_hmm.c
> @@ -1624,6 +1624,8 @@ static long dmirror_fops_unlocked_ioctl(struct file *filp,
>  
>  	if (cmd.addr & ~PAGE_MASK)
>  		return -EINVAL;
> +	if (cmd.npages > ULONG_MAX >> PAGE_SHIFT)
> +		return -EINVAL;
>  	if (cmd.addr >= (cmd.addr + (cmd.npages << PAGE_SHIFT)))
>  		return -EINVAL;

Most functions (eg. dmirror_read) have a similar sequence as well. I don't think
we need to add this check to all of them though, and arguably we should just
remove the redundant overflow check in each function and just validate once on
entry from user-space.

But this looks good to me:

Reviewed-by: Alistair Popple <apopple@xxxxxxxxxx>

 - Alistair

>
> -- 
> 2.53.0
> 
>