Re: [PATCH v2] lib/test_hmm: Check alloc_page_vma() return value

[Alistair Popple]

On 2026-05-18 at 13:19 +1000, liuqiangneo@xxxxxxx wrote...
> From: Qiang Liu <liuqiang@xxxxxxxxxx>
> 
> Return VM_FAULT_OOM if page allocation fails, which
> avoids a NULL pointer dereference when calling lock_page().
> 
> Signed-off-by: Qiang Liu <liuqiang@xxxxxxxxxx>
> ---
> v2:
> - Add unlock and free allocated pages before return.
> - https://lore.kernel.org/all/20260514032345.32256-1-liuqiangneo@xxxxxxx/
> ---
>  lib/test_hmm.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> index 213504915737..90aec4ca2e01 100644
> --- a/lib/test_hmm.c
> +++ b/lib/test_hmm.c
> @@ -1063,6 +1063,17 @@ static vm_fault_t dmirror_devmem_fault_alloc_and_copy(struct migrate_vma *args,
>  			/* Try with smaller pages if large allocation fails */
>  			if (!dpage && order) {
>  				dpage = alloc_page_vma(GFP_HIGHUSER_MOVABLE, args->vma, addr);
> +				if (!dpage) {
> +					/* Unlock and free pages already allocated. */
> +					while (i > 0) {
> +						struct page *fpage;

You will need to clear dst[i] as well to avoid subsequent calls to
migrate_vma_pages/finalize() trying to continue with the migration. But then the
whole error handling for this function already seems to be broken because not
all callers do this :-)

Looking at the two callers for this, one ignores the return code
(dmirror_migrate_to_system) and the other (dmirror_devmem_fault) gets the
error handling wrong because it skips calling migrate_vma_finalize() if
dmirror_devmem_fault_alloc_and_copy() failed.

What needs to happen is we propagate the error up to the test so the test fails,
but we still need to call migrate_vma_finalize() to remove the migration entries
created in the migrate_vma_setup() step.

 - Alistair

> +						fpage = migrate_pfn_to_page(dst[--i]);
> +						unlock_page(fpage);
> +						__free_page(fpage);
> +					}
> +					return VM_FAULT_OOM;
> +				}
>  				lock_page(dpage);
>  				dst[i] = migrate_pfn(page_to_pfn(dpage));
>  				dst_page = pfn_to_page(page_to_pfn(dpage));
> -- 
> 2.43.0
> 
>