Re: drm/amdkfd: implement counters for vm fault and migration
On 2021-07-06 9:08 p.m., Felix Kuehling
wrote:
Am 2021-07-06 um 11:36 a.m. schrieb Colin Ian King:
Hi,
Static analysis with Coverity on linux-next has found a potential null
pointer dereference in function svm_range_restore_pages in
drivers/gpu/drm/amd/amdkfd/kfd_svm.c from the following commit:
commit d4ebc2007040a0aff01bfe1b194085d3867328fd
Author: Philip Yang
Date: Tue Jun 22 00:12:32 2021 -0400
drm/amdkfd: implement counters for vm fault and migration
The analysis is as follows:
Thanks. Philip, see inline ...
2397 int
2398 svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
2399uint64_t addr)
2400{
2401struct mm_struct *mm = NULL;
2402struct svm_range_list *svms;
2403struct svm_range *prange;
2404struct kfd_process *p;
2405uint64_t timestamp;
2406int32_t best_loc;
2407int32_t gpuidx = MAX_GPU_INSTANCE;
2408bool write_locked = false;
2409int r = 0;
2410
1. Condition !(adev->kfd.dev->pgmap.type != 0), taking false branch.
2411if (!KFD_IS_SVM_API_SUPPORTED(adev->kfd.dev)) {
2412pr_debug("device does not support SVM\n");
2413return -EFAULT;
2414}
2415
2416p = kfd_lookup_process_by_pasid(pasid);
2. Condition !p, taking false branch.
2417if (!p) {
2418pr_debug("kfd process not founded pasid 0x%x\n", pasid);
2419return -ESRCH;
2420}
3. Condition !p->xnack_enabled, taking false branch.
2421if (!p->xnack_enabled) {
2422pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
2423return -EFAULT;
2424}
2425svms = &p->svms;
2426
4. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
5. Condition 1 /* __builtin_types_compatible_p() */, taking true branch.
6. Falling through to end of if statement.
7. Condition !!branch, taking false branch.
8. Condition ({...; !!branch;}), taking false branch.
2427pr_debug("restoring svms 0x%p fault address 0x%llx\n", svms,
addr);
2428
2429mm = get_task_mm(p->lead_thread);
9. Condition !mm, taking false branch.
2430if (!mm) {
2431pr_debug("svms 0x%p failed to get mm\n", svms);
2432r = -ESRCH;
2433goto out;
2434}
2435
2436mmap_read_lock(mm);
2437retry_write_locked:
2438mutex_lock(&svms->lock);
2439prange = svm_range_from_addr(svms, addr, NULL);
10. Condition !prange, taking true branch.
18. Condition !prange, taking true branch.
2440if (!prange) {
11. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
12. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
13. Falling through to end of if statement.
14. Condition !!branch, taking false branch.
15. Condition ({...; !!branch;}), taking false branch.
19. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
20. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
21. Falling through to end of if statement.
22. Condition !!branch, taking false branch.
23. Condition ({...; !!branch;}), taking false branch.
2441pr_debug("failed to find prange svms 0x%p address
[0x%llx]\n",
2442 svms, addr);
16. Condition !write_locked, taking true branch.
24. Condition !write_locked, taking false branch.
2443if (!write_locked) {
2444/* Need the write lock to create new range
with MMU notifier.
2445 * Also flush pending deferred work to make
sure the interval
2446 * tree is up to date before we add a new range
2447 */
2448mutex_unlock(&svms->lock);
2449mmap_read_unlock(mm);
2450mmap_write_lock(mm);
2451write_locked = true;
17. Jumping to label retry_write_locked.
2452goto retry_write_locked;
2453}
2454prange = svm_range_create_unregistered_range(adev,
p, mm, addr);
25. Condition !prange, taking true branch.
26. var_compare_op: Comparing prange to null implies that prange
might be null.
2455if (!prange) {
27. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
28. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
29. Falling through to end of if statement.
30. Condition !!branch, taking false branch.
31. Condition ({...; !!branch;}), taking false branch.
2456
Re: drm/amdkfd: implement counters for vm fault and migration
On 2021-07-06 11:36 a.m., Colin Ian
King wrote:
Hi,
Static analysis with Coverity on linux-next has found a potential null
pointer dereference in function svm_range_restore_pages in
drivers/gpu/drm/amd/amdkfd/kfd_svm.c from the following commit:
commit d4ebc2007040a0aff01bfe1b194085d3867328fd
Author: Philip Yang
Date: Tue Jun 22 00:12:32 2021 -0400
drm/amdkfd: implement counters for vm fault and migration
The analysis is as follows:
2397 int
2398 svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
2399uint64_t addr)
2400{
2401struct mm_struct *mm = NULL;
2402struct svm_range_list *svms;
2403struct svm_range *prange;
2404struct kfd_process *p;
2405uint64_t timestamp;
2406int32_t best_loc;
2407int32_t gpuidx = MAX_GPU_INSTANCE;
2408bool write_locked = false;
2409int r = 0;
2410
1. Condition !(adev->kfd.dev->pgmap.type != 0), taking false branch.
2411if (!KFD_IS_SVM_API_SUPPORTED(adev->kfd.dev)) {
2412pr_debug("device does not support SVM\n");
2413return -EFAULT;
2414}
2415
2416p = kfd_lookup_process_by_pasid(pasid);
2. Condition !p, taking false branch.
2417if (!p) {
2418pr_debug("kfd process not founded pasid 0x%x\n", pasid);
2419return -ESRCH;
2420}
3. Condition !p->xnack_enabled, taking false branch.
2421if (!p->xnack_enabled) {
2422pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
2423return -EFAULT;
2424}
2425svms = &p->svms;
2426
4. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
5. Condition 1 /* __builtin_types_compatible_p() */, taking true branch.
6. Falling through to end of if statement.
7. Condition !!branch, taking false branch.
8. Condition ({...; !!branch;}), taking false branch.
2427pr_debug("restoring svms 0x%p fault address 0x%llx\n", svms,
addr);
2428
2429mm = get_task_mm(p->lead_thread);
9. Condition !mm, taking false branch.
2430if (!mm) {
2431pr_debug("svms 0x%p failed to get mm\n", svms);
2432r = -ESRCH;
2433goto out;
2434}
2435
2436mmap_read_lock(mm);
2437retry_write_locked:
2438mutex_lock(&svms->lock);
2439prange = svm_range_from_addr(svms, addr, NULL);
10. Condition !prange, taking true branch.
18. Condition !prange, taking true branch.
2440if (!prange) {
11. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
12. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
13. Falling through to end of if statement.
14. Condition !!branch, taking false branch.
15. Condition ({...; !!branch;}), taking false branch.
19. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
20. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
21. Falling through to end of if statement.
22. Condition !!branch, taking false branch.
23. Condition ({...; !!branch;}), taking false branch.
2441pr_debug("failed to find prange svms 0x%p address
[0x%llx]\n",
2442 svms, addr);
16. Condition !write_locked, taking true branch.
24. Condition !write_locked, taking false branch.
2443if (!write_locked) {
2444/* Need the write lock to create new range
with MMU notifier.
2445 * Also flush pending deferred work to make
sure the interval
2446 * tree is up to date before we add a new range
2447 */
2448mutex_unlock(&svms->lock);
2449mmap_read_unlock(mm);
2450mmap_write_lock(mm);
2451write_locked = true;
17. Jumping to label retry_write_locked.
2452goto retry_write_locked;
2453}
2454prange = svm_range_create_unregistered_range(adev,
p, mm, addr);
25. Condition !prange, taking true branch.
26. var_compare_op: Comparing prange to null implies that prange
might be null.
2455if (!prange) {
27. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
28. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
29. Falling through to end of if statement.
30. Condition !!branch, taking false branch.
31. Condition ({...; !!branch;}), taking false branch.
2456pr_debug("failed to create unregistered
range svms 0x%p address [0x%llx]\n",
2457 svms, addr);
2458
Re: drm/amdkfd: implement counters for vm fault and migration
Am 2021-07-06 um 11:36 a.m. schrieb Colin Ian King:
> Hi,
>
> Static analysis with Coverity on linux-next has found a potential null
> pointer dereference in function svm_range_restore_pages in
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c from the following commit:
>
> commit d4ebc2007040a0aff01bfe1b194085d3867328fd
> Author: Philip Yang
> Date: Tue Jun 22 00:12:32 2021 -0400
>
> drm/amdkfd: implement counters for vm fault and migration
>
> The analysis is as follows:
Thanks. Philip, see inline ...
>
> 2397 int
> 2398 svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
> 2399uint64_t addr)
> 2400{
> 2401struct mm_struct *mm = NULL;
> 2402struct svm_range_list *svms;
> 2403struct svm_range *prange;
> 2404struct kfd_process *p;
> 2405uint64_t timestamp;
> 2406int32_t best_loc;
> 2407int32_t gpuidx = MAX_GPU_INSTANCE;
> 2408bool write_locked = false;
> 2409int r = 0;
> 2410
>
> 1. Condition !(adev->kfd.dev->pgmap.type != 0), taking false branch.
>
> 2411if (!KFD_IS_SVM_API_SUPPORTED(adev->kfd.dev)) {
> 2412pr_debug("device does not support SVM\n");
> 2413return -EFAULT;
> 2414}
> 2415
> 2416p = kfd_lookup_process_by_pasid(pasid);
>
> 2. Condition !p, taking false branch.
>
> 2417if (!p) {
> 2418pr_debug("kfd process not founded pasid 0x%x\n", pasid);
> 2419return -ESRCH;
> 2420}
>
> 3. Condition !p->xnack_enabled, taking false branch.
>
> 2421if (!p->xnack_enabled) {
> 2422pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
> 2423return -EFAULT;
> 2424}
> 2425svms = &p->svms;
> 2426
>
> 4. Condition 0 /* __builtin_types_compatible_p() */, taking false
> branch.
> 5. Condition 1 /* __builtin_types_compatible_p() */, taking true branch.
> 6. Falling through to end of if statement.
> 7. Condition !!branch, taking false branch.
> 8. Condition ({...; !!branch;}), taking false branch.
>
> 2427pr_debug("restoring svms 0x%p fault address 0x%llx\n", svms,
> addr);
> 2428
> 2429mm = get_task_mm(p->lead_thread);
>
> 9. Condition !mm, taking false branch.
>
> 2430if (!mm) {
> 2431pr_debug("svms 0x%p failed to get mm\n", svms);
> 2432r = -ESRCH;
> 2433goto out;
> 2434}
> 2435
> 2436mmap_read_lock(mm);
> 2437retry_write_locked:
> 2438mutex_lock(&svms->lock);
> 2439prange = svm_range_from_addr(svms, addr, NULL);
>
> 10. Condition !prange, taking true branch.
> 18. Condition !prange, taking true branch.
> 2440if (!prange) {
> 11. Condition 0 /* __builtin_types_compatible_p() */, taking false
> branch.
> 12. Condition 1 /* __builtin_types_compatible_p() */, taking true
> branch.
> 13. Falling through to end of if statement.
> 14. Condition !!branch, taking false branch.
> 15. Condition ({...; !!branch;}), taking false branch.
> 19. Condition 0 /* __builtin_types_compatible_p() */, taking false
> branch.
> 20. Condition 1 /* __builtin_types_compatible_p() */, taking true
> branch.
> 21. Falling through to end of if statement.
> 22. Condition !!branch, taking false branch.
> 23. Condition ({...; !!branch;}), taking false branch.
>
> 2441pr_debug("failed to find prange svms 0x%p address
> [0x%llx]\n",
> 2442 svms, addr);
>
> 16. Condition !write_locked, taking true branch.
> 24. Condition !write_locked, taking false branch.
>
> 2443if (!write_locked) {
> 2444/* Need the write lock to create new range
> with MMU notifier.
> 2445 * Also flush pending deferred work to make
> sure the interval
> 2446 * tree is up to date before we add a new range
> 2447 */
> 2448mutex_unlock(&svms->lock);
> 2449mmap_read_unlock(mm);
> 2450mmap_write_lock(mm);
> 2451write_locked = true;
>
> 17. Jumping to label retry_write_locked.
>
> 2452goto retry_write_locked;
> 2453}
> 2454prange = svm_range_create_unregistered_range(adev,
> p, mm, addr);
>
> 25. Condition !prange, taking true b
re: drm/amdkfd: implement counters for vm fault and migration
Hi,
Static analysis with Coverity on linux-next has found a potential null
pointer dereference in function svm_range_restore_pages in
drivers/gpu/drm/amd/amdkfd/kfd_svm.c from the following commit:
commit d4ebc2007040a0aff01bfe1b194085d3867328fd
Author: Philip Yang
Date: Tue Jun 22 00:12:32 2021 -0400
drm/amdkfd: implement counters for vm fault and migration
The analysis is as follows:
2397 int
2398 svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
2399uint64_t addr)
2400{
2401struct mm_struct *mm = NULL;
2402struct svm_range_list *svms;
2403struct svm_range *prange;
2404struct kfd_process *p;
2405uint64_t timestamp;
2406int32_t best_loc;
2407int32_t gpuidx = MAX_GPU_INSTANCE;
2408bool write_locked = false;
2409int r = 0;
2410
1. Condition !(adev->kfd.dev->pgmap.type != 0), taking false branch.
2411if (!KFD_IS_SVM_API_SUPPORTED(adev->kfd.dev)) {
2412pr_debug("device does not support SVM\n");
2413return -EFAULT;
2414}
2415
2416p = kfd_lookup_process_by_pasid(pasid);
2. Condition !p, taking false branch.
2417if (!p) {
2418pr_debug("kfd process not founded pasid 0x%x\n", pasid);
2419return -ESRCH;
2420}
3. Condition !p->xnack_enabled, taking false branch.
2421if (!p->xnack_enabled) {
2422pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
2423return -EFAULT;
2424}
2425svms = &p->svms;
2426
4. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
5. Condition 1 /* __builtin_types_compatible_p() */, taking true branch.
6. Falling through to end of if statement.
7. Condition !!branch, taking false branch.
8. Condition ({...; !!branch;}), taking false branch.
2427pr_debug("restoring svms 0x%p fault address 0x%llx\n", svms,
addr);
2428
2429mm = get_task_mm(p->lead_thread);
9. Condition !mm, taking false branch.
2430if (!mm) {
2431pr_debug("svms 0x%p failed to get mm\n", svms);
2432r = -ESRCH;
2433goto out;
2434}
2435
2436mmap_read_lock(mm);
2437retry_write_locked:
2438mutex_lock(&svms->lock);
2439prange = svm_range_from_addr(svms, addr, NULL);
10. Condition !prange, taking true branch.
18. Condition !prange, taking true branch.
2440if (!prange) {
11. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
12. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
13. Falling through to end of if statement.
14. Condition !!branch, taking false branch.
15. Condition ({...; !!branch;}), taking false branch.
19. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
20. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
21. Falling through to end of if statement.
22. Condition !!branch, taking false branch.
23. Condition ({...; !!branch;}), taking false branch.
2441pr_debug("failed to find prange svms 0x%p address
[0x%llx]\n",
2442 svms, addr);
16. Condition !write_locked, taking true branch.
24. Condition !write_locked, taking false branch.
2443if (!write_locked) {
2444/* Need the write lock to create new range
with MMU notifier.
2445 * Also flush pending deferred work to make
sure the interval
2446 * tree is up to date before we add a new range
2447 */
2448mutex_unlock(&svms->lock);
2449mmap_read_unlock(mm);
2450mmap_write_lock(mm);
2451write_locked = true;
17. Jumping to label retry_write_locked.
2452goto retry_write_locked;
2453}
2454prange = svm_range_create_unregistered_range(adev,
p, mm, addr);
25. Condition !prange, taking true branch.
26. var_compare_op: Comparing prange to null implies that prange
might be null.
2455if (!prange) {
27. Condition 0 /* __builtin_types_compatible_p() */, taking false
branch.
28. Condition 1 /* __builtin_types_compatible_p() */, taking true
branch.
29. Falling through to end of if statement.
30. Condition !!branch, taking false branch.
31. Condition ({...; !!branch;}), taking false branch.
2456pr_debug("failed to create unregistered
range svms 0x%p address [0x%llx]\n",
2457 svms, addr);
2458mmap_write_downgrade(mm);
2459r
