[PATCH] drm/etnaviv: erase context from active_contexts on open failure

[PATCH] drm/etnaviv: erase context from active_contexts on open failure

[Changyul Lee]

etnaviv_open() allocates an ID in priv->active_contexts for the new
struct etnaviv_file_private. When etnaviv_iommu_context_init() fails,
the error path frees ctx but does not remove the entry from the
XArray, so the entry keeps a dangling pointer.

Erase the entry before freeing the context on that path.

Fixes: d306788b6e1b ("drm/etnaviv: allocate unique ID per drm_file")

Signed-off-by: Changyul Lee <lcy8047@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 08aca9035fc1..ceb8c00d66c0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -79,7 +79,7 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 					      priv->cmdbuf_suballoc);
 	if (!ctx->mmu) {
 		ret = -ENOMEM;
-		goto out_free;
+		goto out_free_id;
 	}
 
 	for (i = 0; i < ETNA_MAX_PIPES; i++) {
@@ -98,6 +98,8 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 
 	return 0;
 
+out_free_id:
+	xa_erase(&priv->active_contexts, ctx->id);
 out_free:
 	kfree(ctx);
 	return ret;
-- 
2.43.0

Re: [PATCH] drm/etnaviv: erase context from active_contexts on open failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of drm_sched_entity_init() is ignored in etnaviv_open(), which leads to a NULL pointer dereference in etnaviv_postclose() if the initialization fails.
--

commit 746a05a80357d93b28dcccb26bf278bdeb7aaf65
Author: Changyul Lee <lcy8047@gmail.com>

drm/etnaviv: erase context from active_contexts on open failure

This commit fixes a dangling pointer in the XArray active_contexts when
etnaviv_iommu_context_init() fails by properly erasing the entry.

> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 08aca9035fc1a..ceb8c00d66c06 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -79,7 +79,7 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
>  					      priv->cmdbuf_suballoc);
>  	if (!ctx->mmu) {
>  		ret = -ENOMEM;
> -		goto out_free;
> +		goto out_free_id;
>  	}
>  
>  	for (i = 0; i < ETNA_MAX_PIPES; i++) {

[ ... ]

>  		if (gpu) {
>  			sched = &gpu->sched;
>  			drm_sched_entity_init(&ctx->sched_entity[i],
>  					      DRM_SCHED_PRIORITY_NORMAL, &sched,
>  					      1, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for
drm_sched_entity_init() to fail here?

If drm_sched_entity_init() fails (for example, due to an ENOMEM from
drm_sched_entity_stats_new()), it leaves the entity zero-initialized.
Because the return value is ignored here, etnaviv_open() continues
and incorrectly returns success.

Later, when the file descriptor is closed, etnaviv_postclose() attempts
to unconditionally destroy the uninitialized entity:

drivers/gpu/drm/etnaviv/etnaviv_drv.c:etnaviv_postclose() {
    ...
    for (i = 0; i < ETNA_MAX_PIPES; i++) {
        if (priv->gpu[i])
            drm_sched_entity_destroy(&ctx->sched_entity[i]);
    }
    ...
}

This leads to a kernel panic in drm_sched_entity_flush() due to
dereferencing a NULL entity->rq:

drivers/gpu/drm/scheduler/sched_entity.c:drm_sched_entity_flush() {
    ...
    struct drm_gpu_scheduler *sched = entity->rq->sched;
    ...
}

Since this patch is already adding the out_free_id cleanup path to handle
MMU allocation failures, should we also check the return value of
drm_sched_entity_init() and properly clean up the initialized entities
and the context if it fails?

>  		}
>  	}
>  
>  	file->driver_priv = ctx;
>  
>  	return 0;
>  
> +out_free_id:
> +	xa_erase(&priv->active_contexts, ctx->id);
>  out_free:
>  	kfree(ctx);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907204619.645774-1-lcy8047@gmail.com?part=1