Messages in this thread |  | | Date | Tue, 19 May 2026 09:39:32 +1000 | | From | Alistair Popple <> | | Subject | Re: [PATCH v2] lib/test_hmm: Check alloc_page_vma() return value |
| |
On 2026-05-18 at 13:19 +1000, liuqiangneo@163.com wrote... > From: Qiang Liu <liuqiang@kylinos.cn> > > Return VM_FAULT_OOM if page allocation fails, which > avoids a NULL pointer dereference when calling lock_page(). > > Signed-off-by: Qiang Liu <liuqiang@kylinos.cn> > --- > v2: > - Add unlock and free allocated pages before return. > - https://lore.kernel.org/all/20260514032345.32256-1-liuqiangneo@163.com/ > --- > 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 > >
|  |