Re: [PATCH 1/2] accel/amdxdna: reject user command submission without a command BO

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The current task's `mm_struct` is assigned to `job->mm` without taking a reference count. If the submitting process exits before the DRM scheduler asynchronously executes the job, `job->mm` will point to a freed `mm_struct`, leading to a use-after-free when the scheduler thread accesses it via `mmget_not_zero()`.
--

--- Patch [1]: [PATCH 1/2] accel/amdxdna: reject user command submission without a command BO ---
commit 66fd358bad7187572b37789ea6dae38b8738ff00
Author: Doruk Tan Ozturk <doruk@0sec.ai>

accel/amdxdna: reject user command submission without a command BO

This commit adds a check in amdxdna_cmd_submit() to ensure user command
submissions have a valid command BO handle. It rejects invalid handles
early to prevent a NULL pointer dereference in the drm_sched worker.

> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
> index 8f8df9d04ec56..a5c8c2c4de6d9 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -603,6 +603,16 @@ int amdxdna_cmd_submit(struct amdxdna_client *client,
>  			ret = -EINVAL;
>  			goto free_job;
>  		}
> +	} else if (!drv_cmd) {
> +		/*
> +		 * Only internal driver commands (drv_cmd != NULL) may omit a
> +		 * command BO. A user command submission with the invalid handle
> +		 * would leave job->cmd_bo NULL and later fault when the scheduler
> +		 * dereferences it in amdxdna_cmd_set_state().
> +		 */
> +		XDNA_DBG(xdna, "Command BO handle required for user submission");
> +		ret = -EINVAL;
> +		goto free_job;
>  	}
>  
>  	ret = amdxdna_arg_bos_lookup(client, job, arg_bo_hdls, arg_bo_cnt);

[ ... ]

> 	job->hwctx = hwctx;
> 	job->mm = current->mm;

[Severity: High]
This is a pre-existing issue, but is it possible for job->mm to become a
dangling pointer if the submitting process exits before the DRM scheduler
asynchronously executes the job?

The current task's mm_struct is assigned to job->mm without taking a reference
via mmgrab(). If a process opens the DRM file descriptor, shares it with a child
process that submits a command and then immediately exits, the parent keeping
the file descriptor open prevents the file's release callback from flushing
pending jobs.

When the DRM scheduler asynchronously picks up the child's job in
aie2_sched_job_run(), it calls mmget_not_zero(job->mm) on the freed mm_struct:

drivers/accel/amdxdna/aie2_ctx.c:aie2_sched_job_run() {
    ...
    if (!mmget_not_zero(job->mm))
        return ERR_PTR(-ESRCH);
    ...
}

Would it be better to pin the mm_struct when assigning it to the job to prevent
a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713173030.87541-1-doruk@0sec.ai?part=1