[PATCH] drm/nouveau/dmem: free migration arrays on early fault returns

[PATCH] drm/nouveau/dmem: free migration arrays on early fault returns

[Gregory Price (Meta)]

nouveau_dmem_migrate_to_ram() leaks its source and destination PFN
arrays when migrate_vma_setup() fails or collects no migratable pages.

The arrays used to be local variables, but THP support changed them to
kcalloc() allocations without updating these returns.

Route both exits through the existing array cleanup. Preserve
VM_FAULT_SIGBUS for a setup error and zero for an empty migration.

Fixes: c32287471077 ("gpu/drm/nouveau: enable THP support for GPU memory migration")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 drivers/gpu/drm/nouveau/nouveau_dmem.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index e74d7bb975a8..4cfc7b16d445 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -233,10 +233,12 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
 	 * than just one page on CPU fault. When such fault happens it is very
 	 * likely that more surrounding page will CPU fault too.
 	 */
-	if (migrate_vma_setup(&args) < 0)
-		return VM_FAULT_SIGBUS;
+	if (migrate_vma_setup(&args) < 0) {
+		ret = VM_FAULT_SIGBUS;
+		goto err;
+	}
 	if (!args.cpages)
-		return 0;
+		goto err;
 
 	if (order)
 		dpage = folio_page(vma_alloc_folio(GFP_HIGHUSER | __GFP_ZERO,
-- 
2.56.0

Re: [PATCH] drm/nouveau/dmem: free migration arrays on early fault returns

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] drm/nouveau/dmem: uncopied folio mapped on error in
  nouveau_dmem_migrate_to_ram() [1]
--

commit 33edd5aee5de0eaebe288aeb01245d89558426c2
Author: Gregory Price (Meta) <gourry@gourry.net>

drm/nouveau/dmem: free migration arrays on early fault returns

This commit fixes memory leaks of the source and destination PFN arrays in
nouveau_dmem_migrate_to_ram() when migrate_vma_setup() fails or finds no
pages to migrate.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> index eacac80558344..ae98e177a240b 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> @@ -233,10 +233,12 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
>  	 * than just one page on CPU fault. When such fault happens it is very
>  	 * likely that more surrounding page will CPU fault too.
>  	 */
> -	if (migrate_vma_setup(&args) < 0)
> -		return VM_FAULT_SIGBUS;
> +	if (migrate_vma_setup(&args) < 0) {
> +		ret = VM_FAULT_SIGBUS;
> +		goto err;
> +	}
>  	if (!args.cpages)
> -		return 0;
> +		goto err;
>  
>  	if (order)
>  		dpage = folio_page(vma_alloc_folio(GFP_HIGHUSER | __GFP_ZERO,

[Severity: Critical]
This problem wasn't introduced by this patch, but if
nouveau_dmem_copy_folio() fails later in nouveau_dmem_migrate_to_ram()
[1], does jumping to done cause migrate_vma_finalize() to map the
uncopied destination folio into the process page tables?

drivers/gpu/drm/nouveau/nouveau_dmem.c:nouveau_dmem_migrate_to_ram() {
	...
	args.dst[0] = migrate_pfn(page_to_pfn(dpage));
	if (order)
		args.dst[0] |= MIGRATE_PFN_COMPOUND;
	dfolio = page_folio(dpage);

	svmm = folio_zone_device_data(sfolio);
	mutex_lock(&svmm->mutex);
	nouveau_svmm_invalidate(svmm, args.start, args.end);
	err = nouveau_dmem_copy_folio(drm, sfolio, dfolio, &dma_info);
	mutex_unlock(&svmm->mutex);
	if (err) {
		ret = VM_FAULT_SIGBUS;
		goto done;
	}

	nouveau_fence_new(&fence, dmem->migrate.chan);
	migrate_vma_pages(&args);
	...
done:
	migrate_vma_finalize(&args);
}

Because args.src[0] still has MIGRATE_PFN_MIGRATE set and args.dst[0]
holds a valid PFN, migrate_vma_finalize() -> __migrate_device_finalize()
treats the aborted migration as successful even though migrate_vma_pages()
never ran.  It installs the zeroed destination folio without initialized
mapping or rmap state and frees the source VRAM folio, corrupting
userspace memory and mm invariants when the folio is later unmapped or
reclaimed.

[1] https://sashiko.dev/#/bug/linux-1d44e729-3bbd-4dee-aa29-0b3e8aaa45ea

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113195436.2659094-1-gourry@gourry.net?part=1