Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Unhandled error return from drm_sched_entity_init() leads to a NULL pointer dereference in the scheduler. -- --- Patch [1]: [PATCH 1/2] drm/etnaviv: Reference count struct etnaviv_file_private --- commit f3b8b988f63ea7f9c0ef8d910f216fe724269fff Author: Christian Gmeiner <[email protected]> drm/etnaviv: Reference count struct etnaviv_file_private This commit adds reference counting to the etnaviv_file_private structure. It ensures that per-context data remains valid during GPU resets handled by the scheduler timeout worker, preventing races with DRM file closures. > 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); > if (ret < 0) [Severity: High] This is a pre-existing issue, but I noticed an unhandled error return further down in etnaviv_open() that could lead to a NULL pointer dereference. In etnaviv_open(), the driver calls drm_sched_entity_init() but ignores its return value: for (i = 0; i < ETNA_MAX_PIPES; i++) { struct etnaviv_gpu *gpu = priv->gpu[i]; struct drm_gpu_scheduler *sched; if (gpu) { sched = &gpu->sched; drm_sched_entity_init(&ctx->sched_entity[i], DRM_SCHED_PRIORITY_NORMAL, &sched, 1, NULL); } } If drm_sched_entity_init() fails (for example, due to -ENOMEM when allocating entity->stats), it zeroes the drm_sched_entity structure, setting entity->rq to NULL, and returns an error. Because etnaviv_open() ignores this error, it proceeds to return success to the VFS. Later, when a user space application submits a job to this pipe via etnaviv_ioctl_gem_submit(), it passes this uninitialized entity to drm_sched_job_init(). The DRM scheduler's drm_sched_job_init() detects that entity->rq is NULL and attempts to log an error using dev_err(job->sched->dev, ...). However, because the job object was just allocated with kzalloc() in submit_create(), job->sched is NULL. Could this lead to an immediate NULL pointer dereference when accessing job->sched->dev? Should etnaviv_open() be updated to check the return value of drm_sched_entity_init() and handle the cleanup on failure? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
