[PATCH] drm/xe: keep process name fallback for diagnostics

[PATCH] drm/xe: keep process name fallback for diagnostics

[Ruoyu Wang]

xe_file_open() stores task->comm in xef->process_name with kstrdup(),
but this diagnostic string is optional and the file open continues if
the allocation fails.

The timeout and devcoredump paths initialize process_name to the safe
"no process" fallback, then overwrite it whenever q->vm->xef exists. If
the kstrdup() failed, the fallback is replaced with NULL and the NULL
pointer is passed to string handling in diagnostic paths.

Keep the fallback process name unless xef->process_name was actually
allocated. Still report the stored pid when the xe file is available.

Fixes: 8af13c3fc125 ("drm/xe: Store process name and pid in xe file")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/gpu/drm/xe/xe_devcoredump.c | 3 ++-
 drivers/gpu/drm/xe/xe_guc_submit.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
index 11e60d6..fbe1fba 100644
--- a/drivers/gpu/drm/xe/xe_devcoredump.c
+++ b/drivers/gpu/drm/xe/xe_devcoredump.c
@@ -343,7 +343,8 @@ static void devcoredump_snapshot(struct xe_devcoredump *coredump,
 	ss->boot_time = ktime_get_boottime();
 
 	if (q->vm && q->vm->xef) {
-		process_name = q->vm->xef->process_name;
+		if (q->vm->xef->process_name)
+			process_name = q->vm->xef->process_name;
 		ss->pid = q->vm->xef->pid;
 	}
 
diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
index 2ac87ff..0892400 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -1189,7 +1189,8 @@ trigger_reset:
 	}
 
 	if (q->vm && q->vm->xef) {
-		process_name = q->vm->xef->process_name;
+		if (q->vm->xef->process_name)
+			process_name = q->vm->xef->process_name;
 		pid = q->vm->xef->pid;
 	}
 
-- 
2.51.0

Re: [PATCH] drm/xe: keep process name fallback for diagnostics

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless access to `task->comm` via `kstrdup()` creates a TOCTOU race leading to a non-null-terminated string and an out-of-bounds read.
--

commit f329e0d91af948bfcb07b90ac00f44ede0a2f8fc
Author: Ruoyu Wang <ruoyuw560@gmail.com>

drm/xe: keep process name fallback for diagnostics

This commit addresses an issue where diagnostic paths would use a NULL
pointer for a process name if allocation failed during file open. It keeps
the "no process" fallback unless the process name was successfully
allocated.

[ ... ]

> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 4b247a3019d2d..697faf0f1048b 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1590,7 +1590,8 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>  	}
>  
>  	if (q->vm && q->vm->xef) {
> -		process_name = q->vm->xef->process_name;
> +		if (q->vm->xef->process_name)
> +			process_name = q->vm->xef->process_name;
>  		pid = q->vm->xef->pid;
>  	}

[Severity: High]
This is a pre-existing issue, but does using this string in formatting
functions risk an out-of-bounds memory read?

Looking at xe_file_open() in drivers/gpu/drm/xe/xe_device.c, task->comm is
directly passed to kstrdup() without holding task_lock(task):

    xef->process_name = kstrdup(task->comm, GFP_KERNEL);

Since task->comm can be modified concurrently by userspace via
prctl(PR_SET_NAME), kstrdup() can compute a short length during its internal
strlen() call, but copy characters from a longer replacement string during
memcpy(). This results in a heap buffer missing a null terminator.

When process_name is subsequently passed to xe_gt_notice() further down in
this function and printed via the %s format specifier:

    xe_gt_notice(guc_to_gt(guc),
                 "Timedout job: seqno=%u, lrc_seqno=%u, guc_id=%d, flags=0x%lx in %s [%d]",
                 xe_sched_job_seqno(job), xe_sched_job_lrc_seqno(job),
                 q->guc->id, q->flags, process_name, pid);

Can this trigger an out-of-bounds read of adjacent slab memory, potentially
leading to KASAN panics or kernel memory disclosure?

Could the string copying in xe_file_open() be safely changed to use
get_task_comm() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609162057.5-1-ruoyuw560@gmail.com?part=1