Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Critical] The patch introduces a deterministic double free and RCU list 
corruption of `job->done_fence` during normal job teardown.
--

commit db241a585a8f0c1a7f9163f155e4be54743c28b8
Author: Wentao Liang <[email protected]>

accel/ethosu: Fix use-after-free and memory leak in ioctl_submit_job

This commit fixes a null pointer dereference and a memory leak of the
done_fence allocation in the error paths of ethosu_ioctl_submit_job().
It skips calling drm_sched_job_cleanup() on an uninitialized struct and
adds a kfree() to the error cleanup routine.

> diff --git a/drivers/accel/ethosu/ethosu_job.c 
> b/drivers/accel/ethosu/ethosu_job.c
> index b76924645aaaf..860f4a4d4c503 100644
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -150,6 +150,7 @@ static void ethosu_job_err_cleanup(struct ethosu_job *job)
>       for (i = 0; i < job->region_cnt; i++)
>               drm_gem_object_put(job->region_bo[i]);
>  
> +     kfree(job->done_fence);

[Severity: Critical]
Will this lead to a double free and rcu list corruption during normal job
completion?

Looking at the normal teardown path in ethosu_job_cleanup(), the job's
reference to done_fence is released via dma_fence_put() before calling
this cleanup function:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_cleanup() {
        ...
        dma_fence_put(job->done_fence);
        dma_fence_put(job->inference_done_fence);

        ethosu_job_err_cleanup(job);
}

If dma_fence_put() drops the last reference and frees the fence, won't the
kfree() added here try to free the same pointer again?

Additionally, if the fence is still referenced elsewhere, couldn't this
synchronous kfree() cause a use-after-free for any other subsystem holding a
reference to the fence?

>       drm_gem_object_put(job->cmd_bo);
>  
>       kfree(job);

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

Reply via email to