accel/amdxdna: Remove drv_cmd tracing from job free callback

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

Posted by Lizhi Hou 1 week, 2 days ago

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);
 	if (!job->job_done)
 		up(&hwctx->priv->job_sem);
 
-- 
2.34.1

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

Posted by Mario Limonciello 1 week ago


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?

>   	if (!job->job_done)
>   		up(&hwctx->priv->job_sem);
>

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

Posted by Lizhi Hou 6 days, 18 hours ago

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.

Lizhi

>
>>       if (!job->job_done)
>>           up(&hwctx->priv->job_sem);
>

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

Posted by Mario Limonciello 4 days, 15 hours ago


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);
>>

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

Posted by Lizhi Hou 4 days, 13 hours ago

Applied to drm-misc-next-fixes

On 6/3/26 11:34, Mario Limonciello wrote:
>
>
> 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);
>>>
>