Re: [PATCH v3 09/19] drm/xe/svm: Split system allocator vma incase of madvise call

[Dan Carpenter <dan.carpenter@linaro.org>]

Hi Himal,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Himal-Prasad-Ghimiray/Introduce-drm_gpuvm_sm_map_ops_flags-enums-for-sm_map_ops/20250528-041919
base:   https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next
patch link:    https://lore.kernel.org/r/20250527164003.1068118-10-himal.prasad.ghimiray%40intel.com
patch subject: [PATCH v3 09/19] drm/xe/svm: Split system allocator vma incase of madvise call
config: loongarch-randconfig-r073-20250529 (https://download.01.org/0day-ci/archive/20250530/202505300720.uhAJlLnM-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202505300720.uhAJlLnM-lkp@intel.com/

New smatch warnings:
drivers/gpu/drm/xe/xe_vm.c:4255 xe_vm_alloc_madvise_vma() warn: variable dereferenced before check 'ops' (see line 4194)

vim +/ops +4255 drivers/gpu/drm/xe/xe_vm.c

524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4175  int xe_vm_alloc_madvise_vma(struct xe_vm *vm, uint64_t start, uint64_t range)
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4176  {
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4177  	struct xe_vma_ops vops;
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4178  	struct drm_gpuva_ops *ops = NULL;
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4179  	struct drm_gpuva_op *__op;
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4180  	bool is_cpu_addr_mirror = false;
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4181  	int err;
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4182  
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4183  	vm_dbg(&vm->xe->drm, "MADVISE IN: addr=0x%016llx, size=0x%016llx", start, range);
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4184  
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4185  	lockdep_assert_held_write(&vm->lock);
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4186  
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4187  	vm_dbg(&vm->xe->drm, "MADVISE_OPS_CREATE: addr=0x%016llx, size=0x%016llx", start, range);
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4188  	ops = drm_gpuvm_sm_map_ops_create(&vm->gpuvm, start, range,
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4189  					  DRM_GPUVM_SKIP_GEM_OBJ_VA_SPLIT_MADVISE,
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4190  					  NULL, start);
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4191  	if (IS_ERR(ops))
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4192  		return PTR_ERR(ops);
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4193  
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27 @4194  	if (list_empty(&ops->list)) {
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4195  		err = 0;
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4196  		goto free_ops;
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4197  	}
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4198  

[ snip ]

524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4252  unwind_ops:
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4253  	vm_bind_ioctl_ops_unwind(vm, &ops, 1);
                                                                                     ^^^^
It might  be nicer to change this...  See below?

524911c937a5a5 Himal Prasad Ghimiray 2025-05-27  4254  free_ops:
524911c937a5a5 Himal Prasad Ghimiray 2025-05-27 @4255  	if (ops)
                                                        ^^^^^^^^
This check is unnecessary.

If we applied this diff then it wouldn't look like maybe
vm_bind_ioctl_ops_unwind() frees ops and sets it to NULL?

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 5a978da411b0..48723754b0f9 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -2822,13 +2822,13 @@ static void xe_vma_op_unwind(struct xe_vm *vm, struct xe_vma_op *op,
 }
 
 static void vm_bind_ioctl_ops_unwind(struct xe_vm *vm,
-				     struct drm_gpuva_ops **ops,
+				     struct drm_gpuva_ops *ops,
 				     int num_ops_list)
 {
 	int i;
 
 	for (i = num_ops_list - 1; i >= 0; --i) {
-		struct drm_gpuva_ops *__ops = ops[i];
+		struct drm_gpuva_ops *__ops = &ops[i];
 		struct drm_gpuva_op *__op;
 
 		if (!__ops)
@@ -3703,7 +3703,7 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 
 unwind_ops:
 	if (err && err != -ENODATA)
-		vm_bind_ioctl_ops_unwind(vm, ops, args->num_binds);
+		vm_bind_ioctl_ops_unwind(vm, *ops, args->num_binds);
 	xe_vma_ops_fini(&vops);
 	for (i = args->num_binds - 1; i >= 0; --i)
 		if (ops[i])
@@ -3788,7 +3788,7 @@ struct dma_fence *xe_vm_bind_kernel_bo(struct xe_vm *vm, struct xe_bo *bo,
 
 unwind_ops:
 	if (err && err != -ENODATA)
-		vm_bind_ioctl_ops_unwind(vm, &ops, 1);
+		vm_bind_ioctl_ops_unwind(vm, ops, 1);
 
 	xe_vma_ops_fini(&vops);
 	drm_gpuva_ops_free(&vm->gpuvm, ops);


-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki