[PATCH] drm/amdgpu: Add explicit NULL check for bo in amdgpu_vm_bo_update()

[PATCH] drm/amdgpu: Add explicit NULL check for bo in amdgpu_vm_bo_update()

[Srinivasan Shanmugam]

amdgpu_vm_bo_update() allows bo_va->base.bo to be NULL in some paths,
such as PRT-only updates.

Although amdgpu_vm_is_bo_always_valid() already returns false for a NULL
BO, Smatch still warns that bo may be NULL before it is dereferenced
later in the block.

Add an explicit `bo &&` check before calling
amdgpu_vm_is_bo_always_valid() to make the non-NULL condition clear and
fixes the below smatch error

drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353 amdgpu_vm_bo_update() error: we previously assumed 'bo' could be null (see line 1292)

Fixes: 26e20235ce00 ("drm/amdgpu: Add amdgpu_bo_is_vm_bo helper")
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b89013a6aa0b..0d26346178d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1349,7 +1349,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 	 * the evicted list so that it gets validated again on the
 	 * next command submission.
 	 */
-	if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
+	if (bo && amdgpu_vm_is_bo_always_valid(vm, bo)) {
 		if (bo->tbo.resource &&
 		    !(bo->preferred_domains &
 		      amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type)))
-- 
2.34.1

Re: [PATCH] drm/amdgpu: Add explicit NULL check for bo in amdgpu_vm_bo_update()

[Tvrtko Ursulin]

On 12/03/2026 14:13, Srinivasan Shanmugam wrote:
> amdgpu_vm_bo_update() allows bo_va->base.bo to be NULL in some paths,
> such as PRT-only updates.
> 
> Although amdgpu_vm_is_bo_always_valid() already returns false for a NULL
> BO, Smatch still warns that bo may be NULL before it is dereferenced
> later in the block.
> 
> Add an explicit `bo &&` check before calling
> amdgpu_vm_is_bo_always_valid() to make the non-NULL condition clear and
> fixes the below smatch error
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353 amdgpu_vm_bo_update() error: we previously assumed 'bo' could be null (see line 1292)
> 
> Fixes: 26e20235ce00 ("drm/amdgpu: Add amdgpu_bo_is_vm_bo helper")
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index b89013a6aa0b..0d26346178d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1349,7 +1349,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>   	 * the evicted list so that it gets validated again on the
>   	 * next command submission.
>   	 */
> -	if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
> +	if (bo && amdgpu_vm_is_bo_always_valid(vm, bo)) {

That would be unfortunate:

bool amdgpu_vm_is_bo_always_valid(struct amdgpu_vm *vm, struct amdgpu_bo 
*bo)
{
	return bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv;
}

Maybe Dan can make smatch smarter? :) Because I don't think papering 
randomly at a single call site is great. It is even in the same 
compilation unit. Hmm does the order matter to smatch? Should we maybe 
move amdgpu_vm_is_bo_always_valid() to be earlier in the file?

Regards,

Tvrtko

>   		if (bo->tbo.resource &&
>   		    !(bo->preferred_domains &
>   		      amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type)))

Re: [PATCH] drm/amdgpu: Add explicit NULL check for bo in amdgpu_vm_bo_update()

[Christian König]

On 3/12/26 15:39, Tvrtko Ursulin wrote:
> 
> On 12/03/2026 14:13, Srinivasan Shanmugam wrote:
>> amdgpu_vm_bo_update() allows bo_va->base.bo to be NULL in some paths,
>> such as PRT-only updates.
>>
>> Although amdgpu_vm_is_bo_always_valid() already returns false for a NULL
>> BO, Smatch still warns that bo may be NULL before it is dereferenced
>> later in the block.
>>
>> Add an explicit `bo &&` check before calling
>> amdgpu_vm_is_bo_always_valid() to make the non-NULL condition clear and
>> fixes the below smatch error
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353 amdgpu_vm_bo_update() error: we previously assumed 'bo' could be null (see line 1292)
>>
>> Fixes: 26e20235ce00 ("drm/amdgpu: Add amdgpu_bo_is_vm_bo helper")
>> Cc: Dan Carpenter <dan.carpenter@linaro.org>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index b89013a6aa0b..0d26346178d4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1349,7 +1349,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>>        * the evicted list so that it gets validated again on the
>>        * next command submission.
>>        */
>> -    if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
>> +    if (bo && amdgpu_vm_is_bo_always_valid(vm, bo)) {
> 
> That would be unfortunate:
> 
> bool amdgpu_vm_is_bo_always_valid(struct amdgpu_vm *vm, struct amdgpu_bo *bo)
> {
>     return bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv;
> }
> 
> Maybe Dan can make smatch smarter? :) Because I don't think papering randomly at a single call site is great. It is even in the same compilation unit. Hmm does the order matter to smatch? Should we maybe move amdgpu_vm_is_bo_always_valid() to be earlier in the file?

Yeah that patch doesn't make much sense.

We explicitly added the amdgpu_vm_is_bo_always_valid() function to avoid such NULL checks.

Regards,
Christian.

> 
> Regards,
> 
> Tvrtko
> 
>>           if (bo->tbo.resource &&
>>               !(bo->preferred_domains &
>>                 amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type)))
> 

Re: [PATCH] drm/amdgpu: Add explicit NULL check for bo in amdgpu_vm_bo_update()

[Dan Carpenter]

On Thu, Mar 12, 2026 at 02:39:34PM +0000, Tvrtko Ursulin wrote:
> 
> On 12/03/2026 14:13, Srinivasan Shanmugam wrote:
> > amdgpu_vm_bo_update() allows bo_va->base.bo to be NULL in some paths,
> > such as PRT-only updates.
> > 
> > Although amdgpu_vm_is_bo_always_valid() already returns false for a NULL
> > BO, Smatch still warns that bo may be NULL before it is dereferenced
> > later in the block.
> > 
> > Add an explicit `bo &&` check before calling
> > amdgpu_vm_is_bo_always_valid() to make the non-NULL condition clear and
> > fixes the below smatch error
> > 
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353 amdgpu_vm_bo_update() error: we previously assumed 'bo' could be null (see line 1292)
> > 
> > Fixes: 26e20235ce00 ("drm/amdgpu: Add amdgpu_bo_is_vm_bo helper")
> > Cc: Dan Carpenter <dan.carpenter@linaro.org>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index b89013a6aa0b..0d26346178d4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -1349,7 +1349,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
> >   	 * the evicted list so that it gets validated again on the
> >   	 * next command submission.
> >   	 */
> > -	if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
> > +	if (bo && amdgpu_vm_is_bo_always_valid(vm, bo)) {
> 
> That would be unfortunate:
> 
> bool amdgpu_vm_is_bo_always_valid(struct amdgpu_vm *vm, struct amdgpu_bo
> *bo)
> {
> 	return bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv;
> }
> 
> Maybe Dan can make smatch smarter? :) Because I don't think papering
> randomly at a single call site is great. It is even in the same compilation
> unit. Hmm does the order matter to smatch? Should we maybe move
> amdgpu_vm_is_bo_always_valid() to be earlier in the file?

I'm glad moving the function earlier fixed it.  :)  The function ends
up getting inlined.  This warning should only show up if you don't have
the cross function database built.

https://staticthinking.wordpress.com/2023/05/02/the-cross-function-db/

regards,
dan carpenter