nouveau/gem: reserve the bo in the info ioctl around the vma lookup
Subsystems (3)
Dave Airlie
maintainerPATCH 1/1 nouveau/gem: reserve the bo in the info ioctl around the vma lookup
From: Dave Airlie <[email protected]>
In the non-uvmm path, there could be a race between the info lookup finding the vma, and the gem close path closing the vma leading to a use-after-free.
Spotted with the help of Opus 4.6.
--- drivers/gpu/drm/nouveau/nouveau_gem.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/gpu/drm/nouveau/nouveau_gem.c +10 -1
@@ -313,11 +313,20 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem, rep->offset = nvbo->offset;if (vmm->vmm.object.oclass >=NVIF_CLASS_VMM_NV50 && !nouveau_cli_uvmm (cli)) { +int ret; + + ret =ttm_bo_reserve (&nvbo->bo, false, false,NULL ); +if (ret) +return ret; + vma =nouveau_vma_find (nvbo, vmm); -if (!vma) +if (!vma) { +ttm_bo_unreserve (&nvbo->bo);return -EINVAL ; + } rep->offset = vma->addr; +ttm_bo_unreserve (&nvbo->bo); }else rep->offset =0 ;
Signature · 1 line
2.54.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Accessing nvbo->bo.resource->mem_type and nvbo->offset without holding the BO reservation lock can lead to race conditions and a NULL pointer dereference. --
commit 764469ad454388a56166a4dfcafbe464a44fc502
Author: Dave Airlie <[email protected]>
nouveau/gem: reserve the bo in the info ioctl around the vma lookup
This commit adds a buffer object reservation lock around the vma lookup in the non-uvmm path to prevent a use-after-free against the gem close path.
drivers/gpu/drm/nouveau/nouveau_gem.c nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem, · line 313
Quoted 6 lines
drivers/gpu/drm/nouveau/nouveau_gem.c +0
-0@@ -313,11 +313,20 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem, rep->offset = nvbo->offset;
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to access the buffer object resource right before this section?
In nouveau_gem_info(), when checking the domains, the code dereferences nvbo->bo.resource:
else if (nvbo->bo.resource->mem_type == TTM_PL_TT)
rep->domain = NOUVEAU_GEM_DOMAIN_GART;
else
...
rep->offset = nvbo->offset;Because this happens before the newly added ttm_bo_reserve() lock is acquired, could a concurrent buffer object move or pipeline gutting free the resource and set bo.resource to NULL?
If so, accessing the memory type without the lock and without a NULL check could lead to a NULL pointer dereference. Should the reservation be moved higher up to protect these accesses as well?
Quoted 7 lines
drivers/gpu/drm/nouveau/nouveau_gem.c +5
-0if (vmm->vmm.object.oclass >=NVIF_CLASS_VMM_NV50 && !nouveau_cli_uvmm (cli)) { +int ret; + + ret =ttm_bo_reserve (&nvbo->bo, false, false,NULL ); +if (ret) +return ret;
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Quoted 1 line · Dave Airlie
[PATCH] nouveau/gem: reserve the bo in the info ioctl around the vma lookup
Applied, thanks!
Branch: drm-misc-next
Tree: https://gitlab.freedesktop.org/drm/misc/kernel.git
[1/1] nouveau/gem: reserve the bo in the info ioctl around the vma lookup
commit: 5e17160d41d9
The patch will appear in the next linux-next integration (typically within 24 hours on weekdays).
The patch is queued up for the upcoming merge window for the next major kernel release.