Re: [PATCH v10 10/27] drm/xe/eudebug: Introduce vm bind and vm bind debug data events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `xe_vm_in_lr_mode(vm)` check in `vm_discover_binds` causes debugger discovery to fail if non-LR VMs have debug data.
--

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

drm/xe/eudebug: Introduce vm bind and vm bind debug data events

This commit introduces events to track bind ioctls and their associated
debug data add and remove operations. It adds a bind event to signal debugger
execution and implements discovery of existing binds when attaching.

> diff --git a/drivers/gpu/drm/xe/xe_eudebug.c b/drivers/gpu/drm/xe/xe_eudebug.c
> index d70970f70a8bc..fe3b2ae06a31f 100644
> --- a/drivers/gpu/drm/xe/xe_eudebug.c
> +++ b/drivers/gpu/drm/xe/xe_eudebug.c

[ ... ]

> @@ -942,13 +1118,57 @@ static struct xe_file *xe_eudebug_target_get(struct xe_eudebug *d)
>  	return xef;
>  }
>  
> +static int vm_discover_binds(struct xe_eudebug *d, struct xe_vm *vm)
> +{
> +	struct xe_debug_data *dd;
> +	struct list_head *pos;
> +	unsigned int ops, count;
> +	u64 ref_seqno;
> +	int err;
> +
> +	/*
> +	 * Walked without vm->debug_data.lock. The caller holds the target's
> +	 * eudebug.ioctl_lock for write, and everything that adds to or
> +	 * removes from this list runs under DRM_XE_VM_BIND, which takes that
> +	 * same rwsem for read, so the list cannot change under us here.
> +	 */
> +	if (list_empty(&vm->debug_data.list))
> +		return 0;

[Severity: High]
Is it intentional that vm_discover_binds() lacks a check for LR mode? 
If a userspace client adds debug data to a non-LR VM, this function will
still attempt to process it since it's missing a check like
if (!xe_vm_in_lr_mode(vm)).

[ ... ]

> @@ -980,6 +1200,12 @@ static void discover_client(struct xe_eudebug *d)
>  		if (err)
>  			goto out;
>  		vm_count++;
> +
> +		err = vm_discover_binds(d, vm);
> +		if (err < 0)
> +			goto out;

[Severity: High]
Can this sequence cause the debugger discovery to fail completely?

In discover_client(), vm_create_event() correctly skips creating events 
and handles for non-LR VMs. However, vm_discover_binds() is called 
unconditionally for all VMs.

If userspace adds debug data to a non-LR VM, vm_discover_binds() will 
attempt to send bind events via vm_bind_event(). Because the VM handle was 
never registered, find_handle() will fail with -ENOENT.

This error will propagate to the out label, immediately disconnecting the
eudebug session. Could an unprivileged userspace client use this to
intentionally break the eudebug facility for their process?

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