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

Pre-existing issues:
- [High] Incomplete fix for NULL pointer dereference of `job->cmd_bo` during 
internal driver command cleanup.
--

commit 85570ac4a3fda867670c08161dee7f7bfee5d853
Author: Deniz Aydogan <[email protected]>

accel/amdxdna: fix NULL deref and GEM object leak in error paths

amdxdna_cmd_submit() does not set job->cmd_bo for internal driver
commands that pass AMDXDNA_INVALID_BO_HANDLE. If arg BO lookup fails
after that, the cmd_put cleanup calls amdxdna_gem_put_obj(NULL) which
dereferences a NULL pointer.

Separately, amdxdna_cmd_set_error() acquires a GEM reference via
amdxdna_gem_get_obj() when handling a chained command, but leaks it
when the subsequent amdxdna_gem_vmap() returns NULL.

Add a NULL check in the submit cleanup path and release the GEM
reference on vmap failure.

> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c 
> b/drivers/accel/amdxdna/amdxdna_ctx.c
> index 31a414c3f0d96..c0444454afe6a 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -676,7 +678,8 @@ int amdxdna_cmd_submit(struct amdxdna_client *client,
>  put_bos:
>       amdxdna_arg_bos_put(job);
>  cmd_put:
> -     amdxdna_gem_put_obj(job->cmd_bo);
> +     if (job->cmd_bo)
> +             amdxdna_gem_put_obj(job->cmd_bo);

[Severity: High]
This isn't a bug introduced by this patch, but does the same unconditional NULL
pointer dereference occur in the normal job completion cleanup path?

If userspace issues a configuration IOCTL (such as 
DRM_AMDXDNA_HWCTX_ASSIGN_DBG_BUF)
that submits an internal driver command, job->cmd_bo is deliberately left as
NULL during submission.

When the job completes, the DRM scheduler invokes aie2_sched_job_free() ->
aie2_job_release() -> amdxdna_sched_job_cleanup(), which unconditionally
calls amdxdna_gem_put_obj():

drivers/accel/amdxdna/amdxdna_ctx.c:amdxdna_sched_job_cleanup() {
    ...
    amdxdna_pm_suspend_put(job->hwctx->client->xdna);
    amdxdna_arg_bos_put(job);
    amdxdna_gem_put_obj(job->cmd_bo);
    dma_fence_put(job->fence);
    ...
}

Will this cause a kernel panic if job->cmd_bo is NULL?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to