Re: [PATCH V1] accel/amdxdna: Remove drv_cmd tracing from job free callback

[Mario Limonciello <superm1@kernel.org>]

On 6/1/26 10:18, Lizhi Hou wrote:
> 
> On 5/31/26 07:18, Mario Limonciello wrote:
>>
>>
>> On 5/29/26 17:28, Lizhi Hou wrote:
>>> aie2_sched_job_free() accesses job->drv_cmd for tracing purposes. 
>>> However,
>>> job->drv_cmd is owned by the caller and may already have been freed when
>>> the job free callback runs, leading to a potential use-after-free.
>>>
>>> Remove the job->drv_cmd access from aie2_sched_job_free().
>>>
>>> Fixes: 8711eb2dde2e ("accel/amdxdna: Improve tracing for job 
>>> lifecycle and mailbox RX worker")
>>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>>> ---
>>>   drivers/accel/amdxdna/aie2_ctx.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/ 
>>> amdxdna/aie2_ctx.c
>>> index 658a5fb1fda6..2ad343728782 100644
>>> --- a/drivers/accel/amdxdna/aie2_ctx.c
>>> +++ b/drivers/accel/amdxdna/aie2_ctx.c
>>> @@ -437,8 +437,9 @@ static void aie2_sched_job_free(struct 
>>> drm_sched_job *sched_job)
>>>       struct amdxdna_sched_job *job = drm_job_to_xdna_job(sched_job);
>>>       struct amdxdna_hwctx *hwctx = job->hwctx;
>>>   +    /* job->drv_cmd could be freed, so use DEFAULT_IO */
>>>       trace_xdna_job(sched_job, hwctx->name, "job free",
>>> -               job->seq, job->drv_cmd ? job->drv_cmd->opcode : 
>>> DEFAULT_IO);
>>> +               job->seq, DEFAULT_IO);
>>
>> Could this still be a race with dov->drv_cmd being valid when the 
>> first part of the expression is evaluated (job->drv_cmd) but invalid 
>> when job->drv_cmd->opcode is accessed?
> 
> When aie2_sched_job_free() is called, the job->drv_cmd could already be 
> freed. So it should never access job->drv_cmd at all. The entire 
> expression "job->drv_cmd ? job->drv_cmd->opcode :  DEFAULT_IO" is removed.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> 
> Lizhi
> 
>>
>>>       if (!job->job_done)
>>>           up(&hwctx->priv->job_sem);
>>