[PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[Alex Deucher]

Avoid a possible UAF in GPU recovery due to a race between
the sched timeout callback and the tdr work queue.

The gpu recovery function calls drm_sched_stop() and
later drm_sched_start().  drm_sched_start() restarts
the tdr queue which will eventually free the job.  If
the tdr queue frees the job before time out callback
completes, the job will be freed and we'll get a UAF
when accessing the pasid.  Cache it early to avoid the
UAF.

Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
Cc: SRINIVASAN.SHANMUGAM@amd.com
Cc: vitaly.prosyak@amd.com
Cc: christian.koenig@amd.com
Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---

v2: Check the pasid rather than job (Lijo)
    Add fixes tag (Christian)

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 8a851d7548c00..c6b1dd95c401d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 	struct amdgpu_hive_info *hive = NULL;
 	int r = 0;
 	bool need_emergency_restart = false;
+	/* save the pasid here as the job may be freed before the end of the reset */
+	int pasid = job ? job->pasid : -EINVAL;
 
 	/*
 	 * If it reaches here because of hang/timeout and a RAS error is
@@ -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 	if (!r) {
 		struct amdgpu_task_info *ti = NULL;
 
-		if (job)
-			ti = amdgpu_vm_get_task_info_pasid(adev, job->pasid);
+		/*
+		 * The job may already be freed at this point via the sched tdr workqueue so
+		 * use the cached pasid.
+		 */
+		if (pasid >= 0)
+			ti = amdgpu_vm_get_task_info_pasid(adev, pasid);
 
 		drm_dev_wedged_event(adev_to_drm(adev), DRM_WEDGE_RECOVERY_NONE,
 				     ti ? &ti->task : NULL);
-- 
2.52.0

RE: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[SHANMUGAM, SRINIVASAN]

[Public]

> -----Original Message-----
> From: Deucher, Alexander <Alexander.Deucher@amd.com>
> Sent: Thursday, December 11, 2025 1:54 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; SHANMUGAM,
> SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>; Prosyak, Vitaly
> <Vitaly.Prosyak@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>;
> Matthew Brost <matthew.brost@intel.com>
> Subject: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery
>
> Avoid a possible UAF in GPU recovery due to a race between the sched timeout
> callback and the tdr work queue.
>
> The gpu recovery function calls drm_sched_stop() and later drm_sched_start().
> drm_sched_start() restarts the tdr queue which will eventually free the job.  If the tdr
> queue frees the job before time out callback completes, the job will be freed and
> we'll get a UAF when accessing the pasid.  Cache it early to avoid the UAF.
>
> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
> Cc: SRINIVASAN.SHANMUGAM@amd.com
> Cc: vitaly.prosyak@amd.com
> Cc: christian.koenig@amd.com
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>
> v2: Check the pasid rather than job (Lijo)
>     Add fixes tag (Christian)

Hi Alex,

if possible, could you pls add this signature to the commit message for references:

[  493.058141] BUG: KASAN: slab-use-after-free in amdgpu_device_gpu_recover+0x968/0x990 [amdgpu]
[  493.067530] Read of size 4 at addr ffff88b0ce3f794c by task kworker/u128:1/323
[  493.074892]
[  493.076485] CPU: 9 UID: 0 PID: 323 Comm: kworker/u128:1 Tainted: G            E       6.16.0-1289896.2.zuul.bf4f11df81c1410bbe901c4373305a31 #1 PREEMPT(voluntary)
[  493.076493] Tainted: [E]=UNSIGNED_MODULE
[  493.076495] Hardware name: TYAN B8021G88V2HR-2T/S8021GM2NR-2T, BIOS V1.03.B10 04/01/2019
[  493.076500] Workqueue: amdgpu-reset-dev drm_sched_job_timedout [gpu_sched]
[  493.076512] Call Trace:
[  493.076515]  <TASK>
[  493.076518]  dump_stack_lvl+0x64/0x80
[  493.076529]  print_report+0xce/0x630
[  493.076536]  ? _raw_spin_lock_irqsave+0x86/0xd0
[  493.076541]  ? __pfx__raw_spin_lock_irqsave+0x10/0x10
[  493.076545]  ? amdgpu_device_gpu_recover+0x968/0x990 [amdgpu]
[  493.077253]  kasan_report+0xb8/0xf0
[  493.077258]  ? amdgpu_device_gpu_recover+0x968/0x990 [amdgpu]
[  493.077965]  amdgpu_device_gpu_recover+0x968/0x990 [amdgpu]
[  493.078672]  ? __pfx_amdgpu_device_gpu_recover+0x10/0x10 [amdgpu]
[  493.079378]  ? amdgpu_coredump+0x1fd/0x4c0 [amdgpu]
[  493.080111]  amdgpu_job_timedout+0x642/0x1400 [amdgpu]
[  493.080903]  ? pick_task_fair+0x24e/0x330
[  493.080910]  ? __pfx_amdgpu_job_timedout+0x10/0x10 [amdgpu]
[  493.081702]  ? _raw_spin_lock+0x75/0xc0
[  493.081708]  ? __pfx__raw_spin_lock+0x10/0x10
[  493.081712]  drm_sched_job_timedout+0x1b0/0x4b0 [gpu_sched]
[  493.081721]  ? __pfx__raw_spin_lock_irq+0x10/0x10
[  493.081725]  process_one_work+0x679/0xff0
[  493.081732]  worker_thread+0x6ce/0xfd0
[  493.081736]  ? __pfx_worker_thread+0x10/0x10
[  493.081739]  kthread+0x376/0x730
[  493.081744]  ? __pfx_kthread+0x10/0x10
[  493.081748]  ? __pfx__raw_spin_lock_irq+0x10/0x10
[  493.081751]  ? __pfx_kthread+0x10/0x10
[  493.081755]  ret_from_fork+0x247/0x330
[  493.081761]  ? __pfx_kthread+0x10/0x10
[  493.081764]  ret_from_fork_asm+0x1a/0x30
[  493.081771]  </TASK>
[  493.081773]

This fix matches what we saw in the KASAN report. The issue happens
because gpu_recover() reads job->pasid after the scheduler restarts.
At that point, the TDR worker may already have freed the job, which
leads to the use-after-free.

By saving the PASID at the start of gpu_recover() and using that saved
value later, we avoid touching the job after it may have been freed.
All other job accesses happen before the scheduler restart, so they are
still safe.

So even if the job gets freed halfway through GPU reset,
we still have its PASID safely stored.

This completely avoids reading the freed job.

From my side this looks correct and robust for this UAF

Reviewed-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 8a851d7548c00..c6b1dd95c401d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
> *adev,
>       struct amdgpu_hive_info *hive = NULL;
>       int r = 0;
>       bool need_emergency_restart = false;
> +     /* save the pasid here as the job may be freed before the end of the reset */
> +     int pasid = job ? job->pasid : -EINVAL;
>
>       /*
>        * If it reaches here because of hang/timeout and a RAS error is @@ -
> 6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
> *adev,
>       if (!r) {
>               struct amdgpu_task_info *ti = NULL;
>
> -             if (job)
> -                     ti = amdgpu_vm_get_task_info_pasid(adev, job->pasid);
> +             /*
> +              * The job may already be freed at this point via the sched tdr
> workqueue so
> +              * use the cached pasid.
> +              */
> +             if (pasid >= 0)
> +                     ti = amdgpu_vm_get_task_info_pasid(adev, pasid);
>
>               drm_dev_wedged_event(adev_to_drm(adev),
> DRM_WEDGE_RECOVERY_NONE,
>                                    ti ? &ti->task : NULL);
> --
> 2.52.0

Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[Lazar, Lijo]

On 12/11/2025 1:53 AM, Alex Deucher wrote:
> Avoid a possible UAF in GPU recovery due to a race between
> the sched timeout callback and the tdr work queue.
> 
> The gpu recovery function calls drm_sched_stop() and
> later drm_sched_start().  drm_sched_start() restarts
> the tdr queue which will eventually free the job.  If
> the tdr queue frees the job before time out callback
> completes, the job will be freed and we'll get a UAF
> when accessing the pasid.  Cache it early to avoid the
> UAF.
> 
> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
> Cc: SRINIVASAN.SHANMUGAM@amd.com
> Cc: vitaly.prosyak@amd.com
> Cc: christian.koenig@amd.com
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> 
> v2: Check the pasid rather than job (Lijo)
>      Add fixes tag (Christian)
> 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 8a851d7548c00..c6b1dd95c401d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>   	struct amdgpu_hive_info *hive = NULL;
>   	int r = 0;
>   	bool need_emergency_restart = false;
> +	/* save the pasid here as the job may be freed before the end of the reset */
> +	int pasid = job ? job->pasid : -EINVAL;
>   
>   	/*
>   	 * If it reaches here because of hang/timeout and a RAS error is
> @@ -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>   	if (!r) {
>   		struct amdgpu_task_info *ti = NULL;
>   
> -		if (job)
> -			ti = amdgpu_vm_get_task_info_pasid(adev, job->pasid);
> +		/*
> +		 * The job may already be freed at this point via the sched tdr workqueue so
> +		 * use the cached pasid.
> +		 */

amdgpu_device_gpu_recover() is run in tdr workqueue.

Now if this is the case, someone has to explain the logic  -

Timeout is triggered here -
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/scheduler/sched_main.c#L559

This calls amdgpu_job_timedout() -> amdgpu_device_gpu_recover()

After that, there is this access to the job -

https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/scheduler/sched_main.c#L566

At least, in some condition, job is not expected to be freed. Then I'm 
not sure if this is the right fix.

Thanks,
Lijo

> +		if (pasid >= 0)
> +			ti = amdgpu_vm_get_task_info_pasid(adev, pasid);
>   
>   		drm_dev_wedged_event(adev_to_drm(adev), DRM_WEDGE_RECOVERY_NONE,
>   				     ti ? &ti->task : NULL);

RE: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[SHANMUGAM, SRINIVASAN]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, December 11, 2025 10:34 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> Prosyak, Vitaly <Vitaly.Prosyak@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Matthew Brost <matthew.brost@intel.com>
> Subject: Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu
> recovery
>
>
>
> On 12/11/2025 1:53 AM, Alex Deucher wrote:
> > Avoid a possible UAF in GPU recovery due to a race between the sched
> > timeout callback and the tdr work queue.
> >
> > The gpu recovery function calls drm_sched_stop() and later
> > drm_sched_start().  drm_sched_start() restarts the tdr queue which
> > will eventually free the job.  If the tdr queue frees the job before
> > time out callback completes, the job will be freed and we'll get a UAF
> > when accessing the pasid.  Cache it early to avoid the UAF.
> >
> > Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
> > Cc: SRINIVASAN.SHANMUGAM@amd.com
> > Cc: vitaly.prosyak@amd.com
> > Cc: christian.koenig@amd.com
> > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >
> > v2: Check the pasid rather than job (Lijo)
> >      Add fixes tag (Christian)
> >
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 8a851d7548c00..c6b1dd95c401d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct
> amdgpu_device *adev,
> >     struct amdgpu_hive_info *hive = NULL;
> >     int r = 0;
> >     bool need_emergency_restart = false;
> > +   /* save the pasid here as the job may be freed before the end of the reset */
> > +   int pasid = job ? job->pasid : -EINVAL;
> >
> >     /*
> >      * If it reaches here because of hang/timeout and a RAS error is @@
> > -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
> *adev,
> >     if (!r) {
> >             struct amdgpu_task_info *ti = NULL;
> >
> > -           if (job)
> > -                   ti = amdgpu_vm_get_task_info_pasid(adev, job->pasid);
> > +           /*
> > +            * The job may already be freed at this point via the sched tdr
> workqueue so
> > +            * use the cached pasid.
> > +            */
>
> amdgpu_device_gpu_recover() is run in tdr workqueue.
>
> Now if this is the case, someone has to explain the logic  -
>
> Timeout is triggered here -
> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/scheduler/sched_main
> .c#L559
>
> This calls amdgpu_job_timedout() -> amdgpu_device_gpu_recover()
>
> After that, there is this access to the job -
>
> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/scheduler/sched_main
> .c#L566
>
> At least, in some condition, job is not expected to be freed. Then I'm not sure if this
> is the right fix.

What is that "someone", "some condition" you feel like? Its better to bring proper justification, and take up this as separate refactoring task

Best,
Srini

Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[Lazar, Lijo]

On 12/11/2025 10:52 AM, SHANMUGAM, SRINIVASAN wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Thursday, December 11, 2025 10:34 AM
>> To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
>> gfx@lists.freedesktop.org
>> Cc: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
>> Prosyak, Vitaly <Vitaly.Prosyak@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>; Matthew Brost <matthew.brost@intel.com>
>> Subject: Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu
>> recovery
>>
>>
>>
>> On 12/11/2025 1:53 AM, Alex Deucher wrote:
>>> Avoid a possible UAF in GPU recovery due to a race between the sched
>>> timeout callback and the tdr work queue.
>>>
>>> The gpu recovery function calls drm_sched_stop() and later
>>> drm_sched_start().  drm_sched_start() restarts the tdr queue which
>>> will eventually free the job.  If the tdr queue frees the job before
>>> time out callback completes, the job will be freed and we'll get a UAF
>>> when accessing the pasid.  Cache it early to avoid the UAF.
>>>
>>> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
>>> Cc: SRINIVASAN.SHANMUGAM@amd.com
>>> Cc: vitaly.prosyak@amd.com
>>> Cc: christian.koenig@amd.com
>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>
>>> v2: Check the pasid rather than job (Lijo)
>>>       Add fixes tag (Christian)
>>>
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 8a851d7548c00..c6b1dd95c401d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct
>> amdgpu_device *adev,
>>>      struct amdgpu_hive_info *hive = NULL;
>>>      int r = 0;
>>>      bool need_emergency_restart = false;
>>> +   /* save the pasid here as the job may be freed before the end of the reset */
>>> +   int pasid = job ? job->pasid : -EINVAL;
>>>
>>>      /*
>>>       * If it reaches here because of hang/timeout and a RAS error is @@
>>> -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
>> *adev,
>>>      if (!r) {
>>>              struct amdgpu_task_info *ti = NULL;
>>>
>>> -           if (job)
>>> -                   ti = amdgpu_vm_get_task_info_pasid(adev, job->pasid);
>>> +           /*
>>> +            * The job may already be freed at this point via the sched tdr
>> workqueue so
>>> +            * use the cached pasid.
>>> +            */
>>
>> amdgpu_device_gpu_recover() is run in tdr workqueue.
>>
>> Now if this is the case, someone has to explain the logic  -
>>
>> Timeout is triggered here -
>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/scheduler/sched_main
>> .c#L559
>>
>> This calls amdgpu_job_timedout() -> amdgpu_device_gpu_recover()
>>
>> After that, there is this access to the job -
>>
>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/scheduler/sched_main
>> .c#L566
>>
>> At least, in some condition, job is not expected to be freed. Then I'm not sure if this
>> is the right fix.
> 
> What is that "someone", "some condition" you feel like? Its better to bring proper justification, and take up this as separate refactoring task
> 

Basically, if scheduler code itself is not expecting job to be not freed 
after timedout callback, then why callback handler needs to assume the same?

Now if callback handler does something else which in turn frees the job, 
the fix needs to be there instead of having this kind of fix.

Thanks,
Lijo


> Best,
> Srini

Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[Lazar, Lijo]

On 12/11/2025 11:14 AM, Lazar, Lijo wrote:
> 
> 
> On 12/11/2025 10:52 AM, SHANMUGAM, SRINIVASAN wrote:
>> [AMD Official Use Only - AMD Internal Distribution Only]
>>
>>> -----Original Message-----
>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>> Sent: Thursday, December 11, 2025 10:34 AM
>>> To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
>>> gfx@lists.freedesktop.org
>>> Cc: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
>>> Prosyak, Vitaly <Vitaly.Prosyak@amd.com>; Koenig, Christian
>>> <Christian.Koenig@amd.com>; Matthew Brost <matthew.brost@intel.com>
>>> Subject: Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu
>>> recovery
>>>
>>>
>>>
>>> On 12/11/2025 1:53 AM, Alex Deucher wrote:
>>>> Avoid a possible UAF in GPU recovery due to a race between the sched
>>>> timeout callback and the tdr work queue.
>>>>
>>>> The gpu recovery function calls drm_sched_stop() and later
>>>> drm_sched_start().  drm_sched_start() restarts the tdr queue which
>>>> will eventually free the job.  If the tdr queue frees the job before
>>>> time out callback completes, the job will be freed and we'll get a UAF
>>>> when accessing the pasid.  Cache it early to avoid the UAF.
>>>>
>>>> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
>>>> Cc: SRINIVASAN.SHANMUGAM@amd.com
>>>> Cc: vitaly.prosyak@amd.com
>>>> Cc: christian.koenig@amd.com
>>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>>
>>>> v2: Check the pasid rather than job (Lijo)
>>>>       Add fixes tag (Christian)
>>>>
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
>>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index 8a851d7548c00..c6b1dd95c401d 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct
>>> amdgpu_device *adev,
>>>>      struct amdgpu_hive_info *hive = NULL;
>>>>      int r = 0;
>>>>      bool need_emergency_restart = false;
>>>> +   /* save the pasid here as the job may be freed before the end of 
>>>> the reset */
>>>> +   int pasid = job ? job->pasid : -EINVAL;
>>>>
>>>>      /*
>>>>       * If it reaches here because of hang/timeout and a RAS error 
>>>> is @@
>>>> -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
>>> *adev,
>>>>      if (!r) {
>>>>              struct amdgpu_task_info *ti = NULL;
>>>>
>>>> -           if (job)
>>>> -                   ti = amdgpu_vm_get_task_info_pasid(adev, job- 
>>>> >pasid);
>>>> +           /*
>>>> +            * The job may already be freed at this point via the 
>>>> sched tdr
>>> workqueue so
>>>> +            * use the cached pasid.
>>>> +            */
>>>
>>> amdgpu_device_gpu_recover() is run in tdr workqueue.
>>>
>>> Now if this is the case, someone has to explain the logic  -
>>>
>>> Timeout is triggered here -
>>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/ 
>>> scheduler/sched_main
>>> .c#L559
>>>
>>> This calls amdgpu_job_timedout() -> amdgpu_device_gpu_recover()
>>>
>>> After that, there is this access to the job -
>>>
>>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/ 
>>> scheduler/sched_main
>>> .c#L566
>>>
>>> At least, in some condition, job is not expected to be freed. Then 
>>> I'm not sure if this
>>> is the right fix.
>>
>> What is that "someone", "some condition" you feel like? Its better to 
>> bring proper justification, and take up this as separate refactoring task
>>
> 
> Basically, if scheduler code itself is not expecting job to be not freed 
> after timedout callback, then why callback handler needs to assume the 
> same?
> 

Taking out double 'not', following is what I meant -

'if scheduler code itself is expecting job to be not freed'

Thanks,
Lijo

> Now if callback handler does something else which in turn frees the job, 
> the fix needs to be there instead of having this kind of fix.
> 
> Thanks,
> Lijo
> 
> 
>> Best,
>> Srini
> 

Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[Matthew Brost]

On Thu, Dec 11, 2025 at 11:37:58AM +0530, Lazar, Lijo wrote:
> 
> 
> On 12/11/2025 11:14 AM, Lazar, Lijo wrote:
> > 
> > 
> > On 12/11/2025 10:52 AM, SHANMUGAM, SRINIVASAN wrote:
> > > [AMD Official Use Only - AMD Internal Distribution Only]
> > > 
> > > > -----Original Message-----
> > > > From: Lazar, Lijo <Lijo.Lazar@amd.com>
> > > > Sent: Thursday, December 11, 2025 10:34 AM
> > > > To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> > > > gfx@lists.freedesktop.org
> > > > Cc: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
> > > > Prosyak, Vitaly <Vitaly.Prosyak@amd.com>; Koenig, Christian
> > > > <Christian.Koenig@amd.com>; Matthew Brost <matthew.brost@intel.com>
> > > > Subject: Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu
> > > > recovery
> > > > 
> > > > 
> > > > 
> > > > On 12/11/2025 1:53 AM, Alex Deucher wrote:
> > > > > Avoid a possible UAF in GPU recovery due to a race between the sched
> > > > > timeout callback and the tdr work queue.
> > > > > 
> > > > > The gpu recovery function calls drm_sched_stop() and later
> > > > > drm_sched_start().  drm_sched_start() restarts the tdr queue which
> > > > > will eventually free the job.  If the tdr queue frees the job before
> > > > > time out callback completes, the job will be freed and we'll get a UAF
> > > > > when accessing the pasid.  Cache it early to avoid the UAF.
> > > > > 
> > > > > Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
> > > > > Cc: SRINIVASAN.SHANMUGAM@amd.com
> > > > > Cc: vitaly.prosyak@amd.com
> > > > > Cc: christian.koenig@amd.com
> > > > > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> > > > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > > > > ---
> > > > > 
> > > > > v2: Check the pasid rather than job (Lijo)
> > > > >       Add fixes tag (Christian)
> > > > > 
> > > > >    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
> > > > >    1 file changed, 8 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > index 8a851d7548c00..c6b1dd95c401d 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct
> > > > amdgpu_device *adev,
> > > > >      struct amdgpu_hive_info *hive = NULL;
> > > > >      int r = 0;
> > > > >      bool need_emergency_restart = false;
> > > > > +   /* save the pasid here as the job may be freed before
> > > > > the end of the reset */
> > > > > +   int pasid = job ? job->pasid : -EINVAL;
> > > > > 
> > > > >      /*
> > > > >       * If it reaches here because of hang/timeout and a RAS
> > > > > error is @@
> > > > > -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
> > > > *adev,
> > > > >      if (!r) {
> > > > >              struct amdgpu_task_info *ti = NULL;
> > > > > 
> > > > > -           if (job)
> > > > > -                   ti = amdgpu_vm_get_task_info_pasid(adev,
> > > > > job- >pasid);
> > > > > +           /*
> > > > > +            * The job may already be freed at this point
> > > > > via the sched tdr
> > > > workqueue so
> > > > > +            * use the cached pasid.
> > > > > +            */
> > > > 
> > > > amdgpu_device_gpu_recover() is run in tdr workqueue.
> > > > 
> > > > Now if this is the case, someone has to explain the logic  -
> > > > 
> > > > Timeout is triggered here -
> > > > https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/
> > > > scheduler/sched_main
> > > > .c#L559
> > > > 
> > > > This calls amdgpu_job_timedout() -> amdgpu_device_gpu_recover()
> > > > 
> > > > After that, there is this access to the job -
> > > > 
> > > > https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/
> > > > scheduler/sched_main
> > > > .c#L566

Yes, the DRM scheduler is broken. Luckily, if free_guilty is set, the
job isn’t in the pending list, so it won’t disappear. It is actually
safe to touch this in the above code example.

Complete nonsense, yes. Is it safe, barely.

I feel like there is concept in Linux which solves this whole thing.

> > > > 
> > > > At least, in some condition, job is not expected to be freed.
> > > > Then I'm not sure if this
> > > > is the right fix.

It isn’t. Fixing DRM scheduler is. But until then, I’m fairly certain
your driver won’t explode with this patch.

Matt

> > > 
> > > What is that "someone", "some condition" you feel like? Its better
> > > to bring proper justification, and take up this as separate
> > > refactoring task
> > > 
> > 
> > Basically, if scheduler code itself is not expecting job to be not freed
> > after timedout callback, then why callback handler needs to assume the
> > same?
> > 
> 
> Taking out double 'not', following is what I meant -
> 
> 'if scheduler code itself is expecting job to be not freed'
> 
> Thanks,
> Lijo
> 
> > Now if callback handler does something else which in turn frees the job,
> > the fix needs to be there instead of having this kind of fix.
> > 
> > Thanks,
> > Lijo
> > 
> > 
> > > Best,
> > > Srini
> > 
> 

Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[Lazar, Lijo]

On 12/11/2025 12:09 PM, Matthew Brost wrote:
> On Thu, Dec 11, 2025 at 11:37:58AM +0530, Lazar, Lijo wrote:
>>
>>
>> On 12/11/2025 11:14 AM, Lazar, Lijo wrote:
>>>
>>>
>>> On 12/11/2025 10:52 AM, SHANMUGAM, SRINIVASAN wrote:
>>>> [AMD Official Use Only - AMD Internal Distribution Only]
>>>>
>>>>> -----Original Message-----
>>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>>> Sent: Thursday, December 11, 2025 10:34 AM
>>>>> To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
>>>>> gfx@lists.freedesktop.org
>>>>> Cc: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>;
>>>>> Prosyak, Vitaly <Vitaly.Prosyak@amd.com>; Koenig, Christian
>>>>> <Christian.Koenig@amd.com>; Matthew Brost <matthew.brost@intel.com>
>>>>> Subject: Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu
>>>>> recovery
>>>>>
>>>>>
>>>>>
>>>>> On 12/11/2025 1:53 AM, Alex Deucher wrote:
>>>>>> Avoid a possible UAF in GPU recovery due to a race between the sched
>>>>>> timeout callback and the tdr work queue.
>>>>>>
>>>>>> The gpu recovery function calls drm_sched_stop() and later
>>>>>> drm_sched_start().  drm_sched_start() restarts the tdr queue which
>>>>>> will eventually free the job.  If the tdr queue frees the job before
>>>>>> time out callback completes, the job will be freed and we'll get a UAF
>>>>>> when accessing the pasid.  Cache it early to avoid the UAF.
>>>>>>
>>>>>> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")
>>>>>> Cc: SRINIVASAN.SHANMUGAM@amd.com
>>>>>> Cc: vitaly.prosyak@amd.com
>>>>>> Cc: christian.koenig@amd.com
>>>>>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>>>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>>>> ---
>>>>>>
>>>>>> v2: Check the pasid rather than job (Lijo)
>>>>>>        Add fixes tag (Christian)
>>>>>>
>>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
>>>>>>     1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> index 8a851d7548c00..c6b1dd95c401d 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>> @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct
>>>>> amdgpu_device *adev,
>>>>>>       struct amdgpu_hive_info *hive = NULL;
>>>>>>       int r = 0;
>>>>>>       bool need_emergency_restart = false;
>>>>>> +   /* save the pasid here as the job may be freed before
>>>>>> the end of the reset */
>>>>>> +   int pasid = job ? job->pasid : -EINVAL;
>>>>>>
>>>>>>       /*
>>>>>>        * If it reaches here because of hang/timeout and a RAS
>>>>>> error is @@
>>>>>> -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
>>>>> *adev,
>>>>>>       if (!r) {
>>>>>>               struct amdgpu_task_info *ti = NULL;
>>>>>>
>>>>>> -           if (job)
>>>>>> -                   ti = amdgpu_vm_get_task_info_pasid(adev,
>>>>>> job- >pasid);
>>>>>> +           /*
>>>>>> +            * The job may already be freed at this point
>>>>>> via the sched tdr
>>>>> workqueue so
>>>>>> +            * use the cached pasid.
>>>>>> +            */
>>>>>
>>>>> amdgpu_device_gpu_recover() is run in tdr workqueue.
>>>>>
>>>>> Now if this is the case, someone has to explain the logic  -
>>>>>
>>>>> Timeout is triggered here -
>>>>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/
>>>>> scheduler/sched_main
>>>>> .c#L559
>>>>>
>>>>> This calls amdgpu_job_timedout() -> amdgpu_device_gpu_recover()
>>>>>
>>>>> After that, there is this access to the job -
>>>>>
>>>>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/
>>>>> scheduler/sched_main
>>>>> .c#L566
> 
> Yes, the DRM scheduler is broken. Luckily, if free_guilty is set, the
> job isn’t in the pending list, so it won’t disappear. It is actually
> safe to touch this in the above code example.
> 
> Complete nonsense, yes. Is it safe, barely.
> 
> I feel like there is concept in Linux which solves this whole thing.
> 
>>>>>
>>>>> At least, in some condition, job is not expected to be freed.
>>>>> Then I'm not sure if this
>>>>> is the right fix.
> 
> It isn’t. Fixing DRM scheduler is. But until then, I’m fairly certain
> your driver won’t explode with this patch.
> 

Thanks for the details. In that case, I think this description - "via 
the sched tdr workqueue" - may not be accurate (assuming the real path 
is yet to be traced?) and be dropped. Instead, it could say something 
like under 'certain conditions' drm scheduler could drop the job, hence 
avoid accesses.

Apart from that, don't know if the "Fixes" is really required. That 
patch assumed sanity from scheduler. At this point, this seems more like 
a workaround for drm scheduler issue unless amdgpu timeout handler is 
also doing something unexpected.

Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

Thanks,
Lijo

> Matt
> 
>>>>
>>>> What is that "someone", "some condition" you feel like? Its better
>>>> to bring proper justification, and take up this as separate
>>>> refactoring task
>>>>
>>>
>>> Basically, if scheduler code itself is not expecting job to be not freed
>>> after timedout callback, then why callback handler needs to assume the
>>> same?
>>>
>>
>> Taking out double 'not', following is what I meant -
>>
>> 'if scheduler code itself is expecting job to be not freed'
>>
>> Thanks,
>> Lijo
>>
>>> Now if callback handler does something else which in turn frees the job,
>>> the fix needs to be there instead of having this kind of fix.
>>>
>>> Thanks,
>>> Lijo
>>>
>>>
>>>> Best,
>>>> Srini
>>>
>>

Re: [PATCH V2] drm/amdgpu: fix a job->pasid access race in gpu recovery

[Christian König]

On 12/10/25 21:23, Alex Deucher wrote:
> Avoid a possible UAF in GPU recovery due to a race between
> the sched timeout callback and the tdr work queue.
> 
> The gpu recovery function calls drm_sched_stop() and
> later drm_sched_start().  drm_sched_start() restarts
> the tdr queue which will eventually free the job.  If
> the tdr queue frees the job before time out callback
> completes, the job will be freed and we'll get a UAF
> when accessing the pasid.  Cache it early to avoid the
> UAF.
> 
> Fixes: a72002cb181f ("drm/amdgpu: Make use of drm_wedge_task_info")

Philip and I briefly remember that there was also a bug report about it a while ago.

But I can't find the link of hand, if anybody can find please speak up.

> Cc: SRINIVASAN.SHANMUGAM@amd.com
> Cc: vitaly.prosyak@amd.com
> Cc: christian.koenig@amd.com
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
> 
> v2: Check the pasid rather than job (Lijo)
>     Add fixes tag (Christian)
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 8a851d7548c00..c6b1dd95c401d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6634,6 +6634,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>  	struct amdgpu_hive_info *hive = NULL;
>  	int r = 0;
>  	bool need_emergency_restart = false;
> +	/* save the pasid here as the job may be freed before the end of the reset */
> +	int pasid = job ? job->pasid : -EINVAL;
>  
>  	/*
>  	 * If it reaches here because of hang/timeout and a RAS error is
> @@ -6734,8 +6736,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>  	if (!r) {
>  		struct amdgpu_task_info *ti = NULL;
>  
> -		if (job)
> -			ti = amdgpu_vm_get_task_info_pasid(adev, job->pasid);
> +		/*
> +		 * The job may already be freed at this point via the sched tdr workqueue so
> +		 * use the cached pasid.
> +		 */
> +		if (pasid >= 0)
> +			ti = amdgpu_vm_get_task_info_pasid(adev, pasid);
>  
>  		drm_dev_wedged_event(adev_to_drm(adev), DRM_WEDGE_RECOVERY_NONE,
>  				     ti ? &ti->task : NULL);