Re: [PATCH 04/10] drm/gem: Convert drm_gem_private_object_init to return error code

[sashiko-bot@kernel.org]

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 <ckoenig.leichtzumerken@gmail.com>

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/20260827124910.2245-1-christian.koenig@amd.com?part=4