Re: [PATCH v10 13/27] drm/xe/eudebug: Add userptr vm pread/pwrite

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Time-of-Check to Time-of-Use (TOCTOU) race condition in `xe_eudebug_vma_access()` allows a debugger to bypass security checks and read/write arbitrary memory in an unrelated privileged process.
- [High] Debugger access to userptr memory spuriously fails with `-ESRCH` if the specific thread that bound the userptr exits, even though the process and address space remain fully valid.
--

commit f0c74c4990322a45ea5e521e420b8ab7f3f80507
Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
drm/xe/eudebug: Add userptr vm pread/pwrite

This commit implements debugger VM access for userptrs by taking a reference
to the current task when the bind is done. During pread/pwrite, it uses this
task parameter to access the debuggee VM via access_process_vm().

> diff --git a/drivers/gpu/drm/xe/xe_eudebug_vm.c b/drivers/gpu/drm/xe/xe_eudebug_vm.c
> index 316fe29586b0a..42e24c1bc09c7 100644
> --- a/drivers/gpu/drm/xe/xe_eudebug_vm.c
> +++ b/drivers/gpu/drm/xe/xe_eudebug_vm.c
[ ... ]
> @@ -52,6 +53,50 @@ static int xe_eudebug_vma_access(struct xe_vma *vma,
[ ... ]
> +	} else if (xe_vma_is_userptr(vma)) {
> +		struct xe_userptr *userptr = &to_userptr_vma(vma)->userptr;
> +		struct mm_struct *mm;
> +		int ret;
[ ... ]
> +		mm = get_task_mm(userptr->eudebug.task);
> +		if (!mm)
> +			return -ESRCH;

[Severity: High]
Will this cause spurious -ESRCH failures if the specific thread that bound
the userptr exits?

When the specific thread exits, its task->mm is cleared to NULL in exit_mm(),
causing get_task_mm() to return NULL here. If an application (like a
Vulkan/OpenGL driver) uses background worker threads to allocate and bind
memory, won't the debugger lose access to this valid memory once that worker
thread terminates, even though the process and mm_struct are still fully
alive?

> +
> +		ret = mm == userptr->notifier.mm ? 0 : -ESRCH;
> +		mmput(mm);
> +		if (ret)
> +			return ret;
> +
> +		return access_process_vm(userptr->eudebug.task,
> +					 xe_vma_userptr(vma) + offset_in_vma,
> +					 buf, bytes,
> +					 write ? FOLL_WRITE : 0);

[Severity: Critical]
Does this sequence introduce a Time-of-Check to Time-of-Use (TOCTOU) race
condition that could allow local privilege escalation?

The code verifies that the mm matches the expected userptr->notifier.mm,
but then it drops the reference with mmput(mm) before proceeding.

Because access_process_vm() internally re-resolves the mm dynamically
without ptrace_may_access() checks, could a malicious unprivileged process
bind a userptr, pass the eudebug fd to a collaborating process, and call
execve() on a setuid binary? 

If the execve() executes during the race window between mmput(mm) and
access_process_vm(), it appears this would fetch the new, privileged mm of
the setuid binary, granting the debugger arbitrary read/write access to it.

> diff --git a/drivers/gpu/drm/xe/xe_userptr.h b/drivers/gpu/drm/xe/xe_userptr.h
> index 2a3cd1b5efbb0..b0781618b3f46 100644
> --- a/drivers/gpu/drm/xe/xe_userptr.h
> +++ b/drivers/gpu/drm/xe/xe_userptr.h
[ ... ]
> @@ -133,4 +139,30 @@ static inline void xe_vma_userptr_force_invalidate(struct xe_userptr_vma *uvma)
[ ... ]
> +#if IS_ENABLED(CONFIG_DRM_XE_EUDEBUG)
> +static inline void xe_eudebug_track_userptr_task(struct xe_userptr *userptr)
> +{
> +	/*
> +	 * We could use the mm which is on notifier. But
> +	 * the access_remote_vm() is not exported. Thus
> +	 * we get reference to task for access_process_vm()
> +	 */
> +	userptr->eudebug.task = get_task_struct(current);

[Severity: High]
Should this track the process leader rather than the specific calling thread?

By saving the specific thread (current) here, the userptr becomes bound to
this thread's lifespan. Changing this to track the process leader instead
(e.g., current->group_leader) might prevent the spurious -ESRCH failures
in xe_eudebug_vma_access() when background worker threads exit.

> +}

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