On 2025-10-22 10:25, Srinivasan Shanmugam wrote:
The function svm_range_validate_and_map() was freeing `range` when
amdgpu_hmm_range_get_pages() failed. But later, the code still used the
same `range` pointer and freed it again. This could cause a
use-after-free and double-free issue.
The fix sets `range = NULL` right after it is freed and checks for
`range` before using or freeing it again.
v2: Removed duplicate !r check in the condition for clarity.
Fixes: c5e357c924e5 ("drm/amdgpu: update the functions to use amdgpu version of
hmm")
Reported by: Dan Carpenter <[email protected]>
Cc: Sunil Khatri <[email protected]>
Cc: Christian König <[email protected]>
Cc: Alex Deucher <[email protected]>
Signed-off-by: Srinivasan Shanmugam <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index f041643308ca..c057d892dea6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1744,13 +1744,14 @@ static int svm_range_validate_and_map(struct mm_struct
*mm,
WRITE_ONCE(p->svms.faulting_task, NULL);
if (r) {
amdgpu_hmm_range_free(range);
Guess v1, v2 patch sent out by accident?
another double free range->hmm_range.hmm_pfns, inside
amdgpu_hmm_range_get_pages, if hmm_range_fault return failed,
out_free_pfns should set hmm_range->hmm_pfns = NULL
+ range = NULL;
pr_debug("failed %d to get svm range pages\n",
r);
}
} else {
r = -EFAULT;
}
- if (!r) {
+ if (!r && range) {
the range check is redundant, !r already means range is not NULL.
offset = (addr >> PAGE_SHIFT) - prange->start;
r = svm_range_dma_map(prange, ctx->bitmap, offset,
npages,
range->hmm_range.hmm_pfns);
@@ -1764,12 +1765,13 @@ static int svm_range_validate_and_map(struct mm_struct
*mm,
* Overrride return value to TRY AGAIN only if prior returns
* were successful
*/
- if (range && !amdgpu_hmm_range_valid(range) && !r) {
+ if (!r && range && !amdgpu_hmm_range_valid(range)) {
Is the check reorder necessary?
Regards,
Philip
pr_debug("hmm update the range, need validate again\n");
r = -EAGAIN;
}
/* Free the hmm range */
- amdgpu_hmm_range_free(range);
+ if (range)
+ amdgpu_hmm_range_free(range);
if (!r && !list_empty(&prange->child_list)) {
