Re: [PATCH] drm/amdkfd: fix NULL pointer dereference

2024-04-15 Thread Felix Kuehling
This patch does not apply to amd-staging-drm-next. This is against a 
DKMS branch and should be reviewed on our internal mailing list.


However, I suspect that part of the problem is, that the DKMS branch has 
diverged quite a bit in this area, and is missing at least one patch 
from me that was reverted, probably because of an improper port. The 
proper solution should involve getting the DKMS branch back in sync with 
upstream. I'll look into that.


Regards,
  Felix

On 2024-04-13 14:07, vitaly.pros...@amd.com wrote:

From: Vitaly Prosyak 

[  +0.006038] BUG: kernel NULL pointer dereference, address: 0028
[  +0.006969] #PF: supervisor read access in kernel mode
[  +0.005139] #PF: error_code(0x) - not-present page
[  +0.005139] PGD 0 P4D 0
[  +0.002530] Oops:  [#1] PREEMPT SMP NOPTI
[  +0.004356] CPU: 11 PID: 12625 Comm: kworker/11:0 Tainted: GW 
 6.7.0+ #2
[  +0.008097] Hardware name: ASUS System Product Name/Pro WS WRX80E-SAGE SE 
WIFI II, BIOS 1302 12/08/2023
[  +0.009398] Workqueue: events evict_process_worker [amdgpu]
[  +0.005750] RIP: 0010:evict_process_worker+0x2f/0x460 [amdgpu]
[  +0.005991] Code: 55 48 89 e5 41 57 41 56 4c 8d b7 a8 fc ff ff 41 55 41 54 53 48 89 
fb 48 83 ec 10 0f 1f 44 00 00 48 8b 43 f8 8b 93 b0 00 00 00 <48> 3b 50 28 0f 85 
50 03 00 00 48 8d 7b 58 e8 ee be cb bf 48 8b 05
[  +0.018791] RSP: 0018:c90009a2be10 EFLAGS: 00010282
[  +0.005226] RAX:  RBX: 888197ffc358 RCX: 
[  +0.007140] RDX: 0a1b RSI:  RDI: 888197ffc358
[  +0.007139] RBP: c90009a2be48 R08:  R09: 
[  +0.007139] R10:  R11:  R12: 888197ffc358
[  +0.007139] R13: 888100153a00 R14: 888197ffc000 R15: 888100153a05
[  +0.007137] FS:  () GS:889facac() 
knlGS:
[  +0.008094] CS:  0010 DS:  ES:  CR0: 80050033
[  +0.005747] CR2: 0028 CR3: 00010d1fc001 CR4: 00770ef0
[  +0.007138] PKRU: 5554
[  +0.002702] Call Trace:
[  +0.002443]  
[  +0.002096]  ? show_regs+0x72/0x90
[  +0.003402]  ? __die+0x25/0x80
[  +0.003052]  ? page_fault_oops+0x154/0x4c0
[  +0.004099]  ? do_user_addr_fault+0x30e/0x6e0
[  +0.004357]  ? psi_group_change+0x237/0x520
[  +0.004185]  ? exc_page_fault+0x84/0x1b0
[  +0.003926]  ? asm_exc_page_fault+0x27/0x30
[  +0.004187]  ? evict_process_worker+0x2f/0x460 [amdgpu]
[  +0.005377]  process_one_work+0x17b/0x360
[  +0.004011]  ? __pfx_worker_thread+0x10/0x10
[  +0.004269]  worker_thread+0x307/0x430
[  +0.003748]  ? __pfx_worker_thread+0x10/0x10
[  +0.004268]  kthread+0xf7/0x130
[  +0.003142]  ? __pfx_kthread+0x10/0x10
[  +0.003749]  ret_from_fork+0x46/0x70
[  +0.003573]  ? __pfx_kthread+0x10/0x10
[  +0.003747]  ret_from_fork_asm+0x1b/0x30
[  +0.003924]  

When we run stressful tests, the eviction fence could be zero and not match
to last_eviction_seqno.

Avoid calling dma_fence_signal and dma_fence_put with zero fences to rely
on checking parameters in DMA API.

Cc: Alex Deucher 
Cc: Christian Koenig 
Cc: Xiaogang Chen 
Cc: Felix Kuehling 
Signed-off-by: Vitaly Prosyak 
---
  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index eb380296017d..a15fae1c398a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -2118,7 +2118,7 @@ static void evict_process_worker(struct work_struct *work)
 */
p = container_of(dwork, struct kfd_process, eviction_work);
trace_kfd_evict_process_worker_start(p);
-   WARN_ONCE(p->last_eviction_seqno != p->ef->seqno,
+   WARN_ONCE(p->ef && p->last_eviction_seqno != p->ef->seqno,
  "Eviction fence mismatch\n");
  
  	/* Narrow window of overlap between restore and evict work

@@ -2134,9 +2134,11 @@ static void evict_process_worker(struct work_struct 
*work)
pr_debug("Started evicting pasid 0x%x\n", p->pasid);
ret = kfd_process_evict_queues(p, false, 
KFD_QUEUE_EVICTION_TRIGGER_TTM);
if (!ret) {
-   dma_fence_signal(p->ef);
-   dma_fence_put(p->ef);
-   p->ef = NULL;
+   if (p->ef) {
+   dma_fence_signal(p->ef);
+   dma_fence_put(p->ef);
+   p->ef = NULL;
+   }
  
  		if (!kfd_process_unmap_doorbells_if_idle(p))

kfd_process_schedule_restore(p);


[PATCH] drm/amdkfd: fix NULL pointer dereference

2024-04-13 Thread vitaly.prosyak
From: Vitaly Prosyak 

[  +0.006038] BUG: kernel NULL pointer dereference, address: 0028
[  +0.006969] #PF: supervisor read access in kernel mode
[  +0.005139] #PF: error_code(0x) - not-present page
[  +0.005139] PGD 0 P4D 0
[  +0.002530] Oops:  [#1] PREEMPT SMP NOPTI
[  +0.004356] CPU: 11 PID: 12625 Comm: kworker/11:0 Tainted: GW 
 6.7.0+ #2
[  +0.008097] Hardware name: ASUS System Product Name/Pro WS WRX80E-SAGE SE 
WIFI II, BIOS 1302 12/08/2023
[  +0.009398] Workqueue: events evict_process_worker [amdgpu]
[  +0.005750] RIP: 0010:evict_process_worker+0x2f/0x460 [amdgpu]
[  +0.005991] Code: 55 48 89 e5 41 57 41 56 4c 8d b7 a8 fc ff ff 41 55 41 54 53 
48 89 fb 48 83 ec 10 0f 1f 44 00 00 48 8b 43 f8 8b 93 b0 00 00 00 <48> 3b 50 28 
0f 85 50 03 00 00 48 8d 7b 58 e8 ee be cb bf 48 8b 05
[  +0.018791] RSP: 0018:c90009a2be10 EFLAGS: 00010282
[  +0.005226] RAX:  RBX: 888197ffc358 RCX: 
[  +0.007140] RDX: 0a1b RSI:  RDI: 888197ffc358
[  +0.007139] RBP: c90009a2be48 R08:  R09: 
[  +0.007139] R10:  R11:  R12: 888197ffc358
[  +0.007139] R13: 888100153a00 R14: 888197ffc000 R15: 888100153a05
[  +0.007137] FS:  () GS:889facac() 
knlGS:
[  +0.008094] CS:  0010 DS:  ES:  CR0: 80050033
[  +0.005747] CR2: 0028 CR3: 00010d1fc001 CR4: 00770ef0
[  +0.007138] PKRU: 5554
[  +0.002702] Call Trace:
[  +0.002443]  
[  +0.002096]  ? show_regs+0x72/0x90
[  +0.003402]  ? __die+0x25/0x80
[  +0.003052]  ? page_fault_oops+0x154/0x4c0
[  +0.004099]  ? do_user_addr_fault+0x30e/0x6e0
[  +0.004357]  ? psi_group_change+0x237/0x520
[  +0.004185]  ? exc_page_fault+0x84/0x1b0
[  +0.003926]  ? asm_exc_page_fault+0x27/0x30
[  +0.004187]  ? evict_process_worker+0x2f/0x460 [amdgpu]
[  +0.005377]  process_one_work+0x17b/0x360
[  +0.004011]  ? __pfx_worker_thread+0x10/0x10
[  +0.004269]  worker_thread+0x307/0x430
[  +0.003748]  ? __pfx_worker_thread+0x10/0x10
[  +0.004268]  kthread+0xf7/0x130
[  +0.003142]  ? __pfx_kthread+0x10/0x10
[  +0.003749]  ret_from_fork+0x46/0x70
[  +0.003573]  ? __pfx_kthread+0x10/0x10
[  +0.003747]  ret_from_fork_asm+0x1b/0x30
[  +0.003924]  

When we run stressful tests, the eviction fence could be zero and not match
to last_eviction_seqno.

Avoid calling dma_fence_signal and dma_fence_put with zero fences to rely
on checking parameters in DMA API.

Cc: Alex Deucher 
Cc: Christian Koenig 
Cc: Xiaogang Chen 
Cc: Felix Kuehling 
Signed-off-by: Vitaly Prosyak 
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index eb380296017d..a15fae1c398a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -2118,7 +2118,7 @@ static void evict_process_worker(struct work_struct *work)
 */
p = container_of(dwork, struct kfd_process, eviction_work);
trace_kfd_evict_process_worker_start(p);
-   WARN_ONCE(p->last_eviction_seqno != p->ef->seqno,
+   WARN_ONCE(p->ef && p->last_eviction_seqno != p->ef->seqno,
  "Eviction fence mismatch\n");
 
/* Narrow window of overlap between restore and evict work
@@ -2134,9 +2134,11 @@ static void evict_process_worker(struct work_struct 
*work)
pr_debug("Started evicting pasid 0x%x\n", p->pasid);
ret = kfd_process_evict_queues(p, false, 
KFD_QUEUE_EVICTION_TRIGGER_TTM);
if (!ret) {
-   dma_fence_signal(p->ef);
-   dma_fence_put(p->ef);
-   p->ef = NULL;
+   if (p->ef) {
+   dma_fence_signal(p->ef);
+   dma_fence_put(p->ef);
+   p->ef = NULL;
+   }
 
if (!kfd_process_unmap_doorbells_if_idle(p))
kfd_process_schedule_restore(p);
-- 
2.25.1