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 <[email protected]> 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/[email protected]?part=1
