Re: [PATCH v2 1/2] drm/etnaviv: Reference count struct etnaviv_file_private

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unchecked return value of drm_sched_entity_init leads to NULL pointer dereference on file close.
--

--- Patch [1]: [PATCH v2 1/2] drm/etnaviv: Reference count struct etnaviv_file_private ---
commit ac09092a81050d73fa591427824ad978bbce1e24
Author: Christian Gmeiner <cgmeiner@igalia.com>

drm/etnaviv: Reference count struct etnaviv_file_private

This commit introduces reference counting for etnaviv_file_private to ensure
the context remains alive while the GPU reset path is still running. The
scheduler timeout worker now holds a reference, avoiding a race with closing
the DRM file.

> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 08aca9035fc1a..a27ed014fb4e7 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
[ ... ]
> @@ -70,6 +83,8 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
>  	if (!ctx)
>  		return -ENOMEM;
>  
> +	kref_init(&ctx->refcount);
> +
>  	ret = xa_alloc_cyclic(&priv->active_contexts, &ctx->id, ctx,
>  			      xa_limit_32b, &priv->next_context_id, GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but does etnaviv_open() need to check the
return value of drm_sched_entity_init()?

Looking at etnaviv_open() just below this block, the driver initializes a
scheduler entity for each valid GPU without checking for failure:

etnaviv_open() {
        ...
        drm_sched_entity_init(&ctx->sched_entity[i],
                              DRM_SCHED_PRIORITY_NORMAL, &sched,
                              1, NULL);
        ...
}

If drm_sched_entity_init() fails (e.g., due to memory exhaustion),
entity->stats remains NULL. 

Later, etnaviv_postclose() unconditionally tears down all entities:

etnaviv_postclose() {
        ...
        if (gpu)
                drm_sched_entity_destroy(&ctx->sched_entity[i]);
        ...
}

This eventually invokes drm_sched_entity_fini() in
drivers/gpu/drm/scheduler/sched_entity.c, which calls:

        drm_sched_entity_stats_put(entity->stats);

Could this cause a NULL pointer dereference in kref_put() when it attempts
to operate on the NULL entity->stats pointer during file close?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-etnaviv-reset-notification-v2-0-90cfebcf22a2@igalia.com?part=1