[PATCH 0/2] drm/nouveau: fix out-of-bounds VRAM access and VM_BIND page-size mismatch

[PATCH 0/2] drm/nouveau: fix out-of-bounds VRAM access and VM_BIND page-size mismatch

[Junrui Luo via B4 Relay]

Two unrelated fixes; they touch different files and can be applied
independently.

Patch 1 sizes the VRAM buffer object backing a device-private region to
the region it actually backs. The region grew to DMEM_CHUNK_SIZE *
NR_CHUNKS while the buffer object stayed at DMEM_CHUNK_SIZE, so
nouveau_dmem_page_addr() resolves every page past the first chunk to VRAM
outside the object.

Patch 2 rejects a VM_BIND that replaces a mapping with one using a
different page size. A new mapping takes over the page tables of the
mappings it replaces, which only holds while they all use the same page
size. Since select_page_shift() started deriving one per mapping it no
longer does, and the new mapping ends up mapping at a size its page
tables were never built for.

Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
Junrui Luo (2):
      drm/nouveau/dmem: pin VRAM for the whole registered range
      drm/nouveau/uvmm: reject replace across page sizes

 drivers/gpu/drm/nouveau/nouveau_dmem.c |  4 ++--
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 3 deletions(-)
---
base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
change-id: 20260817-nouveau-fixes-23877845c3ab

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

[PATCH 0/2] drm/nouveau: fix out-of-bounds VRAM access and VM_BIND page-size mismatch

[Junrui Luo]

Two unrelated fixes; they touch different files and can be applied
independently.

Patch 1 sizes the VRAM buffer object backing a device-private region to
the region it actually backs. The region grew to DMEM_CHUNK_SIZE *
NR_CHUNKS while the buffer object stayed at DMEM_CHUNK_SIZE, so
nouveau_dmem_page_addr() resolves every page past the first chunk to VRAM
outside the object.

Patch 2 rejects a VM_BIND that replaces a mapping with one using a
different page size. A new mapping takes over the page tables of the
mappings it replaces, which only holds while they all use the same page
size. Since select_page_shift() started deriving one per mapping it no
longer does, and the new mapping ends up mapping at a size its page
tables were never built for.

Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
Junrui Luo (2):
      drm/nouveau/dmem: pin VRAM for the whole registered range
      drm/nouveau/uvmm: reject replace across page sizes

 drivers/gpu/drm/nouveau/nouveau_dmem.c |  4 ++--
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 3 deletions(-)
---
base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
change-id: 20260817-nouveau-fixes-23877845c3ab

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

[PATCH 1/2] drm/nouveau/dmem: pin VRAM for the whole registered range

[Junrui Luo via B4 Relay]

From: Junrui Luo <moonafterrain@outlook.com>

Commit c32287471077 ("gpu/drm/nouveau: enable THP support for GPU memory
migration") grew the device-private region that
nouveau_dmem_chunk_alloc() registers from DMEM_CHUNK_SIZE to
DMEM_CHUNK_SIZE * NR_CHUNKS, but left the VRAM buffer object backing that
region at DMEM_CHUNK_SIZE.

nouveau_dmem_page_addr() returns chunk->bo->offset plus the page's offset
within the registered region, so every page past the first chunk resolves
to VRAM outside the buffer object.

Size the buffer object to the region it backs.

Fixes: c32287471077 ("gpu/drm/nouveau: enable THP support for GPU memory migration")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/nouveau/nouveau_dmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 9442ec6e1f6c..356ff8f3c1b8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -325,8 +325,8 @@ nouveau_dmem_chunk_alloc(struct nouveau_drm *drm, struct page **ppage,
 	chunk->pagemap.ops = &nouveau_dmem_pagemap_ops;
 	chunk->pagemap.owner = drm->dev;
 
-	ret = nouveau_bo_new_pin(&drm->client, NOUVEAU_GEM_DOMAIN_VRAM, DMEM_CHUNK_SIZE,
-				 &chunk->bo);
+	ret = nouveau_bo_new_pin(&drm->client, NOUVEAU_GEM_DOMAIN_VRAM,
+				 DMEM_CHUNK_SIZE * NR_CHUNKS, &chunk->bo);
 	if (ret)
 		goto out_release;
 

-- 
2.51.2

[PATCH 1/2] drm/nouveau/dmem: pin VRAM for the whole registered range

[Junrui Luo]

Commit c32287471077 ("gpu/drm/nouveau: enable THP support for GPU memory
migration") grew the device-private region that
nouveau_dmem_chunk_alloc() registers from DMEM_CHUNK_SIZE to
DMEM_CHUNK_SIZE * NR_CHUNKS, but left the VRAM buffer object backing that
region at DMEM_CHUNK_SIZE.

nouveau_dmem_page_addr() returns chunk->bo->offset plus the page's offset
within the registered region, so every page past the first chunk resolves
to VRAM outside the buffer object.

Size the buffer object to the region it backs.

Fixes: c32287471077 ("gpu/drm/nouveau: enable THP support for GPU memory migration")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/nouveau/nouveau_dmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 9442ec6e1f6c..356ff8f3c1b8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -325,8 +325,8 @@ nouveau_dmem_chunk_alloc(struct nouveau_drm *drm, struct page **ppage,
 	chunk->pagemap.ops = &nouveau_dmem_pagemap_ops;
 	chunk->pagemap.owner = drm->dev;
 
-	ret = nouveau_bo_new_pin(&drm->client, NOUVEAU_GEM_DOMAIN_VRAM, DMEM_CHUNK_SIZE,
-				 &chunk->bo);
+	ret = nouveau_bo_new_pin(&drm->client, NOUVEAU_GEM_DOMAIN_VRAM,
+				 DMEM_CHUNK_SIZE * NR_CHUNKS, &chunk->bo);
 	if (ret)
 		goto out_release;
 

-- 
2.51.2

Re: [PATCH 1/2] drm/nouveau/dmem: pin VRAM for the whole registered range

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] Double unmap of DMA pages in nouveau_dmem_evict_chunk error path.
- [High] DMA mapping leak in device memory migration paths for THP folios.
- [High] Device memory migration completely broken due to invalid page_shift in nouveau_pfns_map().
- [High] Splitting a THP folio will cause chunk->callocated to underflow.
- [High] NULL pointer dereference occurs in nouveau_dmem_chunk_alloc() when migrating large folios if CONFIG_TRANSPARENT_HUGEPAGE is disabled.
- [Medium] Device memory leak of 508KB per chunk in nouveau_dmem_chunk_alloc() for non-THP allocations.
--

--- Patch [1]: [PATCH 1/2] drm/nouveau/dmem: pin VRAM for the whole registered range ---
commit d125a5f88d0941299dc186384f431b1921ef50ed
Author: Junrui Luo <moonafterrain@outlook.com>

drm/nouveau/dmem: pin VRAM for the whole registered range

Commit c32287471077 ("gpu/drm/nouveau: enable THP support for GPU memory
migration") grew the device-private region that
nouveau_dmem_chunk_alloc() registers from DMEM_CHUNK_SIZE to
DMEM_CHUNK_SIZE * NR_CHUNKS, but left the VRAM buffer object backing that
region at DMEM_CHUNK_SIZE. This commit sizes the buffer object to the
region it backs.

Fixes: c32287471077 ("gpu/drm/nouveau: enable THP support for GPU memory migration")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>

> diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> index 9442ec6e1f6c1..356ff8f3c1b84 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> @@ -325,8 +325,8 @@ nouveau_dmem_chunk_alloc(struct nouveau_drm *drm, struct page **ppage,
>  	chunk->pagemap.ops = &nouveau_dmem_pagemap_ops;
>  	chunk->pagemap.owner = drm->dev;
>  
> -	ret = nouveau_bo_new_pin(&drm->client, NOUVEAU_GEM_DOMAIN_VRAM, DMEM_CHUNK_SIZE,
> -				 &chunk->bo);
> +	ret = nouveau_bo_new_pin(&drm->client, NOUVEAU_GEM_DOMAIN_VRAM,
> +				 DMEM_CHUNK_SIZE * NR_CHUNKS, &chunk->bo);
>  	if (ret)
>  		goto out_release;

[ ... ]

> 	pfn = pfn_first;
> 	for (i = 0; i < NR_CHUNKS; i++) {
> 		int j;
> 
> 		if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) || !is_large) {
> 			for (j = 0; j < DMEM_CHUNK_NPAGES - 1; j++, pfn++) {

[Severity: Medium]
This isn't a bug introduced by this patch, but does this loop skip and
leak pages?

The loop initialization runs NR_CHUNKS times, and its inner loop runs
DMEM_CHUNK_NPAGES - 1 times, continually incrementing pfn. This populates
exactly NR_CHUNKS * (DMEM_CHUNK_NPAGES - 1) pages into free_pages.

After the loop, 1 page is popped and returned to the caller, leaving the
remaining pages in free_pages. The rest of the pages at the end of the
allocated physical and VRAM region appear to be uninitialized and
permanently leaked.

[ ... ]

> 		} else {
> 			page = pfn_to_page(pfn);
> 			page->zone_device_data = drm->dmem->free_folios;
> 			drm->dmem->free_folios = page_folio(page);
> 			pfn += DMEM_CHUNK_NPAGES;
> 		}
> 	}
> 
> 	/* Move to next page */
> 	if (is_large) {
> 		*ppage = &drm->dmem->free_folios->page;

[Severity: High]
This is a pre-existing issue, but can this dereference a NULL pointer?

If migrate_vma_setup() sets MIGRATE_PFN_COMPOUND for large page cache
folios, is_large evaluates to true. If CONFIG_TRANSPARENT_HUGEPAGE is
disabled, the earlier condition evaluates to true and populates free_pages
instead of free_folios.

Immediately after, the code checks if (is_large) and attempts to
dereference drm->dmem->free_folios->page, which would still be NULL.


drivers/gpu/drm/nouveau/nouveau_dmem.c:nouveau_dmem_evict_chunk() {
    ...
        nouveau_dmem_copy_folio(chunk->drm,
            page_folio(migrate_pfn_to_page(src_pfns[i])),
            page_folio(dpage),
            &dma_info[i]);
    ...
        dma_unmap_page(chunk->drm->dev->dev, dma_info[i].dma_addr,
                dma_info[i].size, DMA_BIDIRECTIONAL);
    ...
}

[Severity: High]
This is a pre-existing issue, but does this lead to a double unmap of DMA
pages on error?

In nouveau_dmem_copy_folio(), if the internal copy fails, it calls
dma_unmap_page() but returns -EIO without clearing dma_info->dma_addr or
size.

The loop in nouveau_dmem_evict_chunk() ignores the return value from
nouveau_dmem_copy_folio() and proceeds. It then unconditionally loops over
all npages and calls dma_unmap_page() using dma_info[i].

For pages that failed to copy, wouldn't this unmap the same DMA address a
second time, potentially corrupting DMA API state?


drivers/gpu/drm/nouveau/nouveau_dmem.c:nouveau_dmem_migrate_to_ram() {
    ...
        dma_unmap_page(drm->dev->dev, dma_info.dma_addr, PAGE_SIZE,
                        DMA_BIDIRECTIONAL);
    ...
}

[Severity: High]
This is a pre-existing issue, but does this hardcoded PAGE_SIZE leak DMA
mappings for THP folios?

nouveau_dmem_copy_folio() maps destination pages using page_size(dpage),
which evaluates to the full size for THP folios, and stores this in
dma_info.size.

However, the cleanup paths in nouveau_dmem_migrate_to_ram() on success,
and nouveau_dmem_migrate_copy_one() on error, hardcode PAGE_SIZE when
calling dma_unmap_page().

Does this leave the remaining pages of the folio permanently mapped in the
IOMMU?


drivers/gpu/drm/nouveau/nouveau_dmem.c:nouveau_dmem_migrate_chunk() {
    ...
        nouveau_pfns_map(svmm, args->vma->vm_mm, args->start, pfns, i, order);
    ...
}

drivers/gpu/drm/nouveau/nouveau_svm.c:nouveau_pfns_map() {
    ...
    args->p.size = npages << page_shift;
    args->p.page = page_shift;
    ...
}

[Severity: High]
This is a pre-existing issue, but is the page_shift parameter incorrect
here?

In nouveau_dmem_migrate_chunk(), nouveau_pfns_map() is called with order
(which is 0 or 9) instead of the actual shift like PAGE_SHIFT (12 or 21).

In nouveau_pfns_map(), this parameter is used to populate args->p.size and
args->p.page. Because npages is the number of 4KB pages processed, if
order = 0 is passed, size evaluates to exactly the number of 4KB pages in
bytes, instead of the correct mapping size.

Does this cause the VMM IOCTL backend to reject the mapping payload due
to invalid sizes and shifts?


drivers/gpu/drm/nouveau/nouveau_dmem.c:nouveau_dmem_folio_split() {
    ...
static void nouveau_dmem_folio_split(struct folio *head, struct folio *tail)
    ...
}

drivers/gpu/drm/nouveau/nouveau_dmem.c:nouveau_dmem_folio_free() {
    ...
    WARN_ON(!chunk->callocated);
    chunk->callocated--;
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does splitting a THP folio
cause an underflow of chunk->callocated?

When a large THP folio is allocated, it increments chunk->callocated by 1.
If the kernel splits this folio, nouveau_dmem_folio_split() is called for
each tail page.

However, nouveau_dmem_folio_split() does not appear to adjust
chunk->callocated to account for the newly split discrete pages.

When these split pages are subsequently freed, nouveau_dmem_folio_free()
will decrement chunk->callocated multiple times for what was originally
a single increment. Will this underflow chunk->callocated and trigger
the WARN_ON() checks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-nouveau-fixes-v1-0-f518d0c735f3@outlook.com?part=1

[PATCH 2/2] drm/nouveau/uvmm: reject replace across page sizes

[Junrui Luo via B4 Relay]

From: Junrui Luo <moonafterrain@outlook.com>

A new mapping takes over the page tables of the mappings it replaces.
nouveau_uvmm_sm_prepare() only acquires page tables for the range no
existing mapping covers, and the map path frees the replaced mappings
without putting their references. That is only valid while all of them
use the same page size, which select_page_shift() no longer guarantees.

Rebinding a GART BO over a 2MiB VRAM BO therefore leaves the new mapping
owning page tables built for a different page size, and it then maps at a
size that was never referenced over that range. Since raw map does not
allocate, nvkm_vmm_iter() can walk down to a NULL leaf and dereference
it. The remainders of a split have the same problem: op_map_prepare()
recomputes a page size with select_page_shift() while the remainder keeps
the parent's page tables, so a parent that was itself downgraded can leave
a remainder that re-aligns to a larger size. This happens on the unmap
path too.

Reject the bind, and make split remainders inherit the page size of the
mapping they are split from.

Fixes: c488a94e7e14 ("drm/nouveau/uvmm: Allow larger pages")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index f5e4756b4de4..6404c54d097c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -85,6 +85,8 @@ struct uvmm_map_args {
 	u64 addr;
 	u64 range;
 	u8 kind;
+	/* Page size to give the new mapping, or 0 to derive it from the op. */
+	u8 page_shift;
 };
 
 static int
@@ -655,7 +657,8 @@ op_map_prepare(struct nouveau_uvmm *uvmm,
 
 	uvma->region = args->region;
 	uvma->kind = args->kind;
-	uvma->page_shift = select_page_shift(uvmm, op);
+	uvma->page_shift = args->page_shift ? args->page_shift :
+			   select_page_shift(uvmm, op);
 
 	drm_gpuva_map(&uvmm->base, &uvma->va, op);
 
@@ -684,8 +687,20 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
 	struct drm_gpuva_op *op;
 	u64 vmm_get_start = args ? args->addr : 0;
 	u64 vmm_get_end = args ? args->addr + args->range : 0;
+	u8 map_page_shift = 0;
 	int ret;
 
+	/* A new mapping takes over the page tables of the mappings it replaces,
+	 * so every one of them has to be using its page size. The new mapping
+	 * is the last op drm_gpuvm_sm_map_ops_create() emits.
+	 */
+	if (args) {
+		struct drm_gpuva_op *last = drm_gpuva_last_op(ops);
+
+		if (last->op == DRM_GPUVA_OP_MAP)
+			map_page_shift = select_page_shift(uvmm, &last->map);
+	}
+
 	drm_gpuva_for_each_op(op, ops) {
 		switch (op->op) {
 		case DRM_GPUVA_OP_MAP: {
@@ -713,11 +728,22 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
 			struct uvmm_map_args remap_args = {
 				.kind = uvma_from_va(va)->kind,
 				.region = uvma_from_va(va)->region,
+				/* The remainders of the split keep the page
+				 * tables of the mapping they are split from,
+				 * so they must keep its page size too.
+				 */
+				.page_shift = uvma_from_va(va)->page_shift,
 			};
 			u64 ustart = va->va.addr;
 			u64 urange = va->va.range;
 			u64 uend = ustart + urange;
 
+			if (map_page_shift &&
+			    uvma_from_va(va)->page_shift != map_page_shift) {
+				ret = -EINVAL;
+				goto unwind;
+			}
+
 			op_unmap_prepare(r->unmap);
 
 			if (r->prev) {
@@ -756,6 +782,11 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
 			u64 uend = ustart + urange;
 			u8 page_shift = uvma_from_va(va)->page_shift;
 
+			if (map_page_shift && page_shift != map_page_shift) {
+				ret = -EINVAL;
+				goto unwind;
+			}
+
 			op_unmap_prepare(u);
 
 			if (!args)

-- 
2.51.2

[PATCH 2/2] drm/nouveau/uvmm: reject replace across page sizes

[Junrui Luo]

A new mapping takes over the page tables of the mappings it replaces.
nouveau_uvmm_sm_prepare() only acquires page tables for the range no
existing mapping covers, and the map path frees the replaced mappings
without putting their references. That is only valid while all of them
use the same page size, which select_page_shift() no longer guarantees.

Rebinding a GART BO over a 2MiB VRAM BO therefore leaves the new mapping
owning page tables built for a different page size, and it then maps at a
size that was never referenced over that range. Since raw map does not
allocate, nvkm_vmm_iter() can walk down to a NULL leaf and dereference
it. The remainders of a split have the same problem: op_map_prepare()
recomputes a page size with select_page_shift() while the remainder keeps
the parent's page tables, so a parent that was itself downgraded can leave
a remainder that re-aligns to a larger size. This happens on the unmap
path too.

Reject the bind, and make split remainders inherit the page size of the
mapping they are split from.

Fixes: c488a94e7e14 ("drm/nouveau/uvmm: Allow larger pages")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index f5e4756b4de4..6404c54d097c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -85,6 +85,8 @@ struct uvmm_map_args {
 	u64 addr;
 	u64 range;
 	u8 kind;
+	/* Page size to give the new mapping, or 0 to derive it from the op. */
+	u8 page_shift;
 };
 
 static int
@@ -655,7 +657,8 @@ op_map_prepare(struct nouveau_uvmm *uvmm,
 
 	uvma->region = args->region;
 	uvma->kind = args->kind;
-	uvma->page_shift = select_page_shift(uvmm, op);
+	uvma->page_shift = args->page_shift ? args->page_shift :
+			   select_page_shift(uvmm, op);
 
 	drm_gpuva_map(&uvmm->base, &uvma->va, op);
 
@@ -684,8 +687,20 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
 	struct drm_gpuva_op *op;
 	u64 vmm_get_start = args ? args->addr : 0;
 	u64 vmm_get_end = args ? args->addr + args->range : 0;
+	u8 map_page_shift = 0;
 	int ret;
 
+	/* A new mapping takes over the page tables of the mappings it replaces,
+	 * so every one of them has to be using its page size. The new mapping
+	 * is the last op drm_gpuvm_sm_map_ops_create() emits.
+	 */
+	if (args) {
+		struct drm_gpuva_op *last = drm_gpuva_last_op(ops);
+
+		if (last->op == DRM_GPUVA_OP_MAP)
+			map_page_shift = select_page_shift(uvmm, &last->map);
+	}
+
 	drm_gpuva_for_each_op(op, ops) {
 		switch (op->op) {
 		case DRM_GPUVA_OP_MAP: {
@@ -713,11 +728,22 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
 			struct uvmm_map_args remap_args = {
 				.kind = uvma_from_va(va)->kind,
 				.region = uvma_from_va(va)->region,
+				/* The remainders of the split keep the page
+				 * tables of the mapping they are split from,
+				 * so they must keep its page size too.
+				 */
+				.page_shift = uvma_from_va(va)->page_shift,
 			};
 			u64 ustart = va->va.addr;
 			u64 urange = va->va.range;
 			u64 uend = ustart + urange;
 
+			if (map_page_shift &&
+			    uvma_from_va(va)->page_shift != map_page_shift) {
+				ret = -EINVAL;
+				goto unwind;
+			}
+
 			op_unmap_prepare(r->unmap);
 
 			if (r->prev) {
@@ -756,6 +782,11 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
 			u64 uend = ustart + urange;
 			u8 page_shift = uvma_from_va(va)->page_shift;
 
+			if (map_page_shift && page_shift != map_page_shift) {
+				ret = -EINVAL;
+				goto unwind;
+			}
+
 			op_unmap_prepare(u);
 
 			if (!args)

-- 
2.51.2

Re: [PATCH 2/2] drm/nouveau/uvmm: reject replace across page sizes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Unaligned split remainders inherit a large page shift without alignment validation, causing silent hardware unmap failures and VMM state mismatch.

Pre-existing issues:
- [High] Missing rollback for `op_unmap_prepare` in `DRM_GPUVA_OP_REMAP` error paths leads to permanent loss of GPUVM mappings.
--

--- Patch [2]: [PATCH 2/2] drm/nouveau/uvmm: reject replace across page sizes ---
commit 90a0db35cc87b7654c52294b9abdb178f1a05f0b
Author: Junrui Luo <moonafterrain@outlook.com>

drm/nouveau/uvmm: reject replace across page sizes

A new mapping takes over the page tables of the mappings it replaces.
nouveau_uvmm_sm_prepare() only acquires page tables for the range no
existing mapping covers, and the map path frees the replaced mappings
without putting their references. That is only valid while all of them
use the same page size, which select_page_shift() no longer guarantees.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> index f5e4756b4de4a..6404c54d097c1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c

[ ... ]

> @@ -713,11 +728,22 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm *uvmm,
>  			struct uvmm_map_args remap_args = {
>  				.kind = uvma_from_va(va)->kind,
>  				.region = uvma_from_va(va)->region,
> +				/* The remainders of the split keep the page
> +				 * tables of the mapping they are split from,
> +				 * so they must keep its page size too.
> +				 */
> +				.page_shift = uvma_from_va(va)->page_shift,

[Severity: Critical]
Does this inheritance of the parent's page shift bypass alignment checks for
unaligned split remainders?

If a process maps a BO with a 2MB-aligned address and size (creating a 2MB
page shift mapping), and then unmaps a 4K sub-region, the DRM_GPUVA_OP_REMAP
has args == NULL. The remainder mapping inherits the 2MB page shift via
remap_args.page_shift.

When passed to op_map_prepare():

	uvma->page_shift = args->page_shift ? args->page_shift :
			   select_page_shift(uvmm, op);

The logic skips select_page_shift() because args->page_shift is non-zero,
assigning the 2MB page size to the remainder mapping. Could this cause the
hardware unmap operations to silently fail (e.g., nvkm_vmm_iter computing a
0 iteration count for sizes smaller than the page size)? This seems like it
could leave the memory still mapped in hardware while the kernel considers
the hole empty.

>  			};
>  			u64 ustart = va->va.addr;
>  			u64 urange = va->va.range;
>  			u64 uend = ustart + urange;
>  
> +			if (map_page_shift &&
> +			    uvma_from_va(va)->page_shift != map_page_shift) {
> +				ret = -EINVAL;
> +				goto unwind;
> +			}
> +
>  			op_unmap_prepare(r->unmap);
>  
>  			if (r->prev) {
>  				ret = op_map_prepare(uvmm, &new->prev, r->prev,
>  						     &remap_args);
>  				if (ret)
>  					goto unwind;

[Severity: High]
This isn't a bug introduced by this patch, but does this error path permanently
lose the mapping from the interval tree?

If op_map_prepare() fails (for example, due to an ENOMEM from
nouveau_uvma_alloc), the code jumps to unwind. However,
nouveau_uvmm_sm_prepare_unwind() starts unwinding from drm_gpuva_prev_op(op),
which skips the current operation.

This means op_unmap_prepare_unwind(r->unmap) is never called to roll back
the op_unmap_prepare(r->unmap) that executed just before this. Will this leave
the mapping permanently removed from the tracking tree while returning failure
to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-nouveau-fixes-v1-0-f518d0c735f3@outlook.com?part=2