Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] Memory leak in xe_bo_init_locked() on drm_gem_private_object_init() failure. - [Medium] Memory leak in vmw_bo_create() when vmw_bo_init() fails at GEM initialization. - [Medium] Heap allocation leak in amdxdna_drm_create_dev_bo() when drm_gem_private_object_init() fails. --
commit 31f8fb9f49fa7d6352b54fca867dc5132c27a261 Author: Christian König <[email protected]> drm/gem: Convert drm_gem_private_object_init to return error code This commit changes drm_gem_private_object_init() to return an error code instead of void, allowing it to gracefully fail rather than crashing the kernel with BUG_ON. Call sites across various DRM and accelerator drivers have been updated to check the return value and handle errors appropriately. [ ... ] > diff --git a/drivers/accel/amdxdna/amdxdna_gem.c > b/drivers/accel/amdxdna/amdxdna_gem.c > index b01f5d95fac83..96490aa0af1be 100644 > --- a/drivers/accel/amdxdna/amdxdna_gem.c > +++ b/drivers/accel/amdxdna/amdxdna_gem.c > @@ -1050,7 +1050,11 @@ amdxdna_drm_create_dev_bo(struct drm_device *dev, > return ERR_PTR(ret); > } > > - drm_gem_private_object_init(dev, gobj, aligned_sz); > + ret = drm_gem_private_object_init(dev, gobj, aligned_sz); > + if (ret) { > + amdxdna_gem_destroy_obj(abo); > + return ERR_PTR(ret); > + } [Severity: Medium] Does this code leak the heap allocation? In amdxdna_drm_create_dev_bo(), there is an earlier successful call to amdxdna_gem_heap_alloc(). If drm_gem_private_object_init() fails, the error handler only calls amdxdna_gem_destroy_obj() and skips calling amdxdna_gem_heap_free(), which leaves a persistent resource leak in the client's heap tracking. [ ... ] > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 9c7a73c0b0dc3..e4e4b87e738d3 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -426,7 +426,9 @@ static int vmw_bo_init(struct vmw_private *dev_priv, > atomic_set(&vmw_bo->map_count, 0); > > params->size = ALIGN(params->size, PAGE_SIZE); > - drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, params->size); > + ret = drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, > params->size); > + if (ret) > + return ret; > > vmw_bo_placement_set(vmw_bo, params->domain, params->busy_domain); > ret = ttm_bo_init_reserved(bdev, &vmw_bo->tbo, params->bo_type, [Severity: Medium] Could this early return cause the vmw_bo structure to be leaked? When vmw_bo_create() allocates the object, it passes it to vmw_bo_init() with a destruction callback. If drm_gem_private_object_init() fails and returns early here, it bypasses the ttm_bo_init_reserved() setup. Because TTM never receives the destruction callback, the caller receives the error and nullifies the pointer without actually freeing it, permanently leaking the struct. [ ... ] > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c > index dde309821237b..85e49b48f9d3a 100644 > --- a/drivers/gpu/drm/xe/xe_bo.c > +++ b/drivers/gpu/drm/xe/xe_bo.c > @@ -2400,7 +2400,9 @@ struct xe_bo *xe_bo_init_locked(struct xe_device *xe, > struct xe_bo *bo, > /* Initialize purge advisory state */ > bo->purgeable.state = XE_MADV_PURGEABLE_WILLNEED; > > - drm_gem_private_object_init(&xe->drm, &bo->ttm.base, size); > + err = drm_gem_private_object_init(&xe->drm, &bo->ttm.base, size); > + if (err) > + return ERR_PTR(err); > > if (resv) { > ctx.allow_res_evict = !(flags & XE_BO_FLAG_NO_RESV_EVICT); [Severity: Medium] Can this error path leak the xe_bo struct? If xe_bo_init_locked() is called with a NULL bo pointer, such as from xe_bo_create_locked(), it allocates a new xe_bo structure internally. If drm_gem_private_object_init() then fails, returning ERR_PTR(err) immediately means the newly allocated bo reference is lost before it can be freed with xe_bo_free(). -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
