Re: [Patch v1] drm/amdgpu: null check for hmm_pfns ptr before freeing it

[Khatri, Sunil]

On 27-10-2025 07:58 pm, Christian König wrote:

On 10/23/25 17:30, Kuehling, Felix wrote:
On 2025-10-23 03:48, Arunpravin Paneer Selvam wrote:
Acked-by: Arunpravin Paneer Selvam<arunpravin.paneersel...@amd.com>

Regards,
Arun.
On 10/23/2025 12:28 PM, Sunil Khatri wrote:
Due to low memory or when num of pages is too big to be
accomodated, allocation could fail for pfn's.

Chekc hmm_pfns for NULL before calling the kvfree for the it.

Signed-off-by: Sunil Khatri<sunil.kha...@amd.com>
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c | 6 +++++-
   1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
index d6f903a2d573..6ac206e2bc46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
@@ -286,7 +286,11 @@ void amdgpu_hmm_range_free(struct amdgpu_hmm_range *range)
       if (!range)
           return;
   -    kvfree(range->hmm_range.hmm_pfns);
+    if (range->hmm_range.hmm_pfns) {
+        kvfree(range->hmm_range.hmm_pfns);
+        range->hmm_range.hmm_pfns = NULL;
+    }
NULL-checks before kfree and friends are unnecessary. There are actually static 
checkers that complain about such unnecessary NULL-checks. For example, 
seehttps://lkml.org/lkml/2024/8/11/168.

The same is also true for the standard libc free in 
usermode:https://stackoverflow.com/questions/1912325/checking-for-null-before-calling-free.

Finally, setting range->hmm_range.hmm_pfns = NULL is also unnecessary because 
you're about to free the whole range structure anyway.
Agree completely with Felix.

Sunil why do you think that this is necessary and blocking KFD for some reason?

Regards,
Christian.

KFD side reported the error of NULL dereference

pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL); //fails if the 
size is too big.

Now when we free the memory in function amdgpu_hmm_range_free and try to 
do a kvfree of the range->hmm_range.hmm_pfns which is NULL and we were 
seeing the NULL dereference.
So i added a check to check for the memory to be valid ptr first before 
calling kvfree.

This actually fixed the issue but i do agree that *"setting 
range->hmm_range.hmm_pfns = NULL could be avoided and that why i did not 
added that check in the final patch that i merged" This is the final 
code after this merge.*

voidamdgpu_hmm_range_free(structamdgpu_hmm_range*range)
{
if(!range)
return;
if(range->hmm_range.hmm_pfns)
kvfree(range->hmm_range.hmm_pfns);
amdgpu_bo_unref(&range->bo);
kfree(range);
}


Regards Sunil Khatri

Regards,
   Felix


+
       amdgpu_bo_unref(&range->bo);
       kfree(range);
   }