Am 11.04.25 um 17:00 schrieb Srinivasan Shanmugam:
> This change adds a check to ensure that 'bo_va' is not null before
> dereferencing it. If 'bo_va' is null, the function returns early,
> preventing any potential crashes or undefined behavior
>
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c:139
> amdgpu_gem_update_bo_mapping()
> error: we previously assumed 'bo_va' could be null (see line 124)
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> 115 static void
> 116 amdgpu_gem_update_bo_mapping(struct drm_file *filp,
> 117 struct amdgpu_bo_va *bo_va,
> 118 uint32_t operation,
> 119 uint64_t point,
> 120 struct dma_fence *fence,
> 121 struct drm_syncobj *syncobj,
> 122 struct dma_fence_chain *chain)
> 123 {
> 124 struct amdgpu_bo *bo = bo_va ? bo_va->base.bo : NULL;
> ^^^^^^^^^^ If bo_va is NULL then bo is also
> NULL
>
> ...
> 135 case AMDGPU_VA_OP_REPLACE:
> 136 if (bo && (bo->tbo.base.resv ==
> vm->root.bo->tbo.base.resv))
> ^^
>
> 137 last_update = vm->last_update;
> 138 else
> --> 139 last_update = bo_va->last_pt_update;
> ^^^^^ This pointer is
> dereferenced without being checked.
>
> 140 break;
>
> Fixes: 16856d135622 ("drm/amdgpu: update userqueue BOs and PDs")
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Reported-by: Dan Carpenter <dan.carpen...@linaro.org>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmu...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index c1d8cee7894b..247fbd014b7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -134,8 +134,10 @@ amdgpu_gem_update_bo_mapping(struct drm_file *filp,
> case AMDGPU_VA_OP_REPLACE:
> if (bo && (bo->tbo.base.resv == vm->root.bo->tbo.base.resv))
> last_update = vm->last_update;
> - else
> + else if (bo_va)
> last_update = bo_va->last_pt_update;
> + else
> + return;
That warning is a false positive. The bo_va is only NULL on clear operations.
I think the whole switch/case here is superfluous. What we should do instead is
update the fence in amdgpu_gem_va_update_vm() instead:
if (operation == AMDGPU_VA_OP_MAP ||
operation == AMDGPU_VA_OP_REPLACE) {
r = amdgpu_vm_bo_update(adev, bo_va, false);
if (r)
goto error;
E.g. here.
}
Regards,
Christian.
> break;
> case AMDGPU_VA_OP_UNMAP:
> case AMDGPU_VA_OP_CLEAR:
