Re: [PATCH v10 09/27] drm/xe: Introduce ADD_DEBUG_DATA and REMOVE_DEBUG_DATA vm bind ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing enforcement of the debug data extension for `ADD_DEBUG_DATA` and `REMOVE_DEBUG_DATA` operations, allowing them to silently execute as no-ops and pass a NULL pointer to eudebug.
--

commit 7fdd31439ab20b18aec25bd5e11c3a84839d451e
Author: Christoph Manszewski <c.manszewski@gmail.com>

drm/xe: Introduce ADD_DEBUG_DATA and REMOVE_DEBUG_DATA vm bind ops

This commit introduces new VM bind operations to add and remove per-VM
debug data. It adds extension parsing to the VM bind ioctl to accept
pseudopaths or file paths for mapping binary instructions to code lines.

> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
[ ... ]
> @@ -3903,6 +4061,10 @@ static int vm_bind_ioctl_check_args(struct xe_device *xe, struct xe_vm *vm,
>  			err = -EOPNOTSUPP;
>  			goto free_bind_ops;
>  		}
> +
> +		err = vm_bind_ioctl_check_user_extensions(vm, op, extensions);
> +		if (err)
> +			goto free_bind_ops;
>  	}
>  
>  	return 0;

[Severity: High]
Does this code correctly enforce that the extensions field must be
non-zero for DRM_XE_VM_BIND_OP_ADD_DEBUG_DATA and
DRM_XE_VM_BIND_OP_REMOVE_DEBUG_DATA?

If extensions is 0, vm_bind_ioctl_check_user_extensions() simply returns 0 
without validating that the required debug data struct is present. This
allows a malformed VM bind ioctl to proceed.

[ ... ]
> @@ -4180,11 +4342,17 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  		u64 obj_offset = bind_ops[i].obj_offset;
>  		u32 prefetch_region = bind_ops[i].prefetch_mem_region_instance;
>  		u16 pat_index = bind_ops[i].pat_index;
> +		u64 extensions = bind_ops[i].extensions;
>  
>  		ops[i] = vm_bind_ioctl_ops_create(vm, &vops, bos[i], obj_offset,
>  						  addr, range, op, flags,
> -						  prefetch_region, pat_index);
> -		if (IS_ERR(ops[i])) {
> +						  prefetch_region, pat_index, extensions);
> +
> +		if (!IS_ERR(ops[i]) && extensions) {
> +			err = vm_bind_ioctl_process_user_extensions(vm, ops[i], op, extensions);
> +			if (err)
> +				goto unwind_ops;
> +		} else if (IS_ERR(ops[i])) {
>  			err = PTR_ERR(ops[i]);
>  			ops[i] = NULL;
>  			goto unwind_ops;

[Severity: High]
When extensions is 0, this block is skipped entirely, leaving the added and
removed pointers in op->modify_debug_data as NULL.

Later, xe_eudebug_vm_bind_execute() extracts this NULL dd pointer and
passes it to vm_bind_op_event(), where debug_data->addr is unconditionally
dereferenced. Can an unprivileged local user trigger a kernel panic by
opening the DRM device, connecting via DRM_IOCTL_XE_EUDEBUG_CONNECT, and
submitting a DRM_XE_VM_BIND_OP_ADD_DEBUG_DATA request with extensions set
to 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903145952.848051-1-mika.kuoppala@linux.intel.com?part=9