On 8/17/26 4:50 PM, Junrui Luo via B4 Relay wrote: > From: Junrui Luo <[email protected]> > > 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 <[email protected]> > Assisted-by: Claude:claude-opus-5
We are moving to Assisted-by: LLM > Cc: [email protected] > Signed-off-by: Junrui Luo <[email protected]> > --- > 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) > Just wondering how this was tested/caught? Thanks, Balbir
