Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] The dynamically allocated `done_fence` is leaked on error paths during job submission in `ethosu_ioctl_submit_job`. - [High] NULL pointer dereference in `ethosu_ioctl_submit_job` error path due to premature cleanup of an uninitialized scheduler job. -- commit f1366d3916a7ec335127047da47dbe3724095b0c Author: Rob Herring (Arm) <[email protected]> accel: ethosu: Fix ethosu_job_open() return value A WARN_ON() returns a 0 or 1, not the original negative errno. Just drop the WARN_ON() as the FD open will pass the return code to userspace and there's only one possible source of the error (drm_sched_entity_init()). > diff --git a/drivers/accel/ethosu/ethosu_job.c > b/drivers/accel/ethosu/ethosu_job.c > index 6a038c0384ccd..99c8812691e55 100644 > --- a/drivers/accel/ethosu/ethosu_job.c > +++ b/drivers/accel/ethosu/ethosu_job.c [ ... ] While reviewing this file, I noticed a couple of pre-existing issues in the job submission error paths. If the ejob->done_fence allocation fails, it bypasses job initialization but still attempts cleanup: drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job() { ... ejob->done_fence = kzalloc_obj(*ejob->done_fence); if (!ejob->done_fence) { ret = -ENOMEM; goto out_cleanup_job; } ret = drm_sched_job_init(&ejob->base, &file_priv->sched_entity, 1, NULL, file->client_id); ... out_cleanup_job: if (ret) drm_sched_job_cleanup(&ejob->base); out_put_job: ethosu_job_err_cleanup(ejob); return ret; } [Severity: High] This isn't a bug introduced by this patch, but does calling drm_sched_job_cleanup(&ejob->base) cause a NULL pointer dereference here? Since drm_sched_job_init() hasn't been called yet, ejob->base.s_fence is completely zero-initialized. drm_sched_job_cleanup() unconditionally dereferences job->s_fence when checking the reference count, which would trigger a panic on this memory allocation failure path. Additionally, looking at the cleanup function called further down that same error path: drivers/accel/ethosu/ethosu_job.c:ethosu_job_err_cleanup() { unsigned int i; ethosu_perfmon_put(job->perfmon); for (i = 0; i < job->region_cnt; i++) drm_gem_object_put(job->region_bo[i]); drm_gem_object_put(job->cmd_bo); kfree(job); } [Severity: High] This is a pre-existing issue, but will this leak ejob->done_fence? If ethosu_ioctl_submit_job() fails later in the function (for example, if drm_gem_object_lookup() returns an error), it jumps to out_cleanup_job and eventually calls ethosu_job_err_cleanup(). The cleanup function frees the job container and BOs, but appears to miss freeing the dynamically allocated ejob->done_fence pointer. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
