[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b9f7a493-5611-4450-a26a-10b03d1dc313@igalia.com>
Date: Tue, 2 Sep 2025 08:59:15 +0100
From: Tvrtko Ursulin <tvrtko.ursulin@...lia.com>
To: phasta@...nel.org,
Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>,
Matthew Brost <matthew.brost@...el.com>, Danilo Krummrich <dakr@...nel.org>,
Christian König <ckoenig.leichtzumerken@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/sched: Fix racy access to
drm_sched_entity.dependency
On 02/09/2025 08:27, Philipp Stanner wrote:
> On Mon, 2025-09-01 at 14:40 +0200, Pierre-Eric Pelloux-Prayer wrote:
>> The drm_sched_job_unschedulable trace point can access
>> entity->dependency after it was cleared by the callback
>> installed in drm_sched_entity_add_dependency_cb, causing:
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000020
>> [...]
>> Workqueue: comp_1.1.0 drm_sched_run_job_work [gpu_sched]
>> RIP: 0010:trace_event_raw_event_drm_sched_job_unschedulable+0x70/0xd0 [gpu_sched]
>>
>> To fix this we either need to keep a reference to the fence before
>> setting up the callbacks, or move the trace_drm_sched_job_unschedulable
>> calls into drm_sched_entity_add_dependency_cb where they can be
>> done earlier.
>>
>> Fixes: 76d97c870f29 ("drm/sched: Trace dependencies for GPU jobs")
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@...lia.com>
>
> Applied to drm-misc-next
Shouldn't it have been drm-misc-fixes?
Regards,
Tvrtko
>> ---
>> drivers/gpu/drm/scheduler/sched_entity.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
>> index 8867b95ab089..3d06f72531ba 100644
>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>> @@ -391,7 +391,8 @@ EXPORT_SYMBOL(drm_sched_entity_set_priority);
>> * Add a callback to the current dependency of the entity to wake up the
>> * scheduler when the entity becomes available.
>> */
>> -static bool drm_sched_entity_add_dependency_cb(struct drm_sched_entity *entity)
>> +static bool drm_sched_entity_add_dependency_cb(struct drm_sched_entity *entity,
>> + struct drm_sched_job *sched_job)
>> {
>> struct drm_gpu_scheduler *sched = entity->rq->sched;
>> struct dma_fence *fence = entity->dependency;
>> @@ -421,6 +422,10 @@ static bool drm_sched_entity_add_dependency_cb(struct drm_sched_entity *entity)
>> entity->dependency = fence;
>> }
>>
>> + if (trace_drm_sched_job_unschedulable_enabled() &&
>> + !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &entity->dependency->flags))
>> + trace_drm_sched_job_unschedulable(sched_job, entity->dependency);
>> +
>> if (!dma_fence_add_callback(entity->dependency, &entity->cb,
>> drm_sched_entity_wakeup))
>> return true;
>> @@ -461,10 +466,8 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity)
>>
>> while ((entity->dependency =
>> drm_sched_job_dependency(sched_job, entity))) {
>> - if (drm_sched_entity_add_dependency_cb(entity)) {
>> - trace_drm_sched_job_unschedulable(sched_job, entity->dependency);
>> + if (drm_sched_entity_add_dependency_cb(entity, sched_job))
>> return NULL;
>> - }
>> }
>>
>> /* skip jobs from entity that marked guilty */
>
Powered by blists - more mailing lists