Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] A racing userspace thread can trigger a use-after-free by guessing the GEM handle during creation, leading to premature object destruction. -- --- Patch [1]: [PATCH] drm/tegra: Finalize GEM object on initialization failure --- commit dfc541dc5f8719918460cc54c0dfa389b27272fd Author: Guangshuo Li <[email protected]> drm/tegra: Finalize GEM object on initialization failure The commit introduces a call to drm_gem_private_object_fini() on the failure path of drm_gem_object_init() inside tegra_bo_alloc_object() before freeing the buffer object, ensuring proper cleanup of the partially initialized GEM object. > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > index ace3e5a805cf7..9ce8ecf1a2335 100644 > --- a/drivers/gpu/drm/tegra/gem.c > +++ b/drivers/gpu/drm/tegra/gem.c > @@ -312,8 +312,10 @@ static struct tegra_bo *tegra_bo_alloc_object(struct > drm_device *drm, > size = round_up(size, PAGE_SIZE); > > err = drm_gem_object_init(drm, &bo->gem, size); > - if (err < 0) > + if (err < 0) { > + drm_gem_private_object_fini(&bo->gem); > goto free; > + } > > err = drm_gem_create_mmap_offset(&bo->gem); > if (err < 0) [Severity: High] This is a pre-existing issue, but while reviewing this area of the code I noticed a potential use-after-free in the related tegra_bo_create_with_handle() function. If drm_gem_handle_create() fails, the error path unconditionally calls tegra_bo_free_object(): drivers/gpu/drm/tegra/gem.c:tegra_bo_create_with_handle() { ... err = drm_gem_handle_create(file, &bo->gem, handle); if (err) { tegra_bo_free_object(&bo->gem); return ERR_PTR(err); } ... } Inside drm_gem_handle_create(), the handle is inserted into the IDR before secondary operations like drm_vma_node_allow() are checked. If that check fails, the handle is removed. During the brief window where the handle is in the IDR, could a racing userspace thread guess the handle and issue an ioctl (like DRM_IOCTL_GEM_CLOSE) to look up the object and increment its refcount? If that happens and tegra_bo_create_with_handle() receives the error, it appears to ignore the elevated refcount and unconditionally free the object by calling tegra_bo_free_object(). When the racing thread later calls drm_gem_object_put(), does this result in dereferencing freed memory? Would it be safer to use drm_gem_object_put() instead of directly calling tegra_bo_free_object() here so that the object is only freed when the racing thread drops its reference? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
