Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] `rocket_iommu_domain_put()` does not check for NULL before dereferencing, leading to a crash on the error path. - [Medium] The ioctl handler silently ignores errors when submitting multiple jobs, reporting success to userspace even if submissions fail. -- commit cc3020dc8d8efb7ac65346d6d8612ab4fbbadb48 Author: Maíra Canal <[email protected]> drm/rocket: Remove unused reset worker The reset.work work_struct and its rocket_reset_work() handler were initialized and cancelled, but never actually queued. This patch drops the dead worker, its initialization, cancellation, and the unused struct member. > diff --git a/drivers/accel/rocket/rocket_job.c > b/drivers/accel/rocket/rocket_job.c > index 2f1861f960ccb..8a26139a44f46 100644 > --- a/drivers/accel/rocket/rocket_job.c > +++ b/drivers/accel/rocket/rocket_job.c [ ... ] [Severity: High] This is a pre-existing issue, but can this lead to a NULL pointer dereference on the error path? In rocket_ioctl_submit_job(), if rocket_copy_tasks() fails, it jumps to out_cleanup_job, which then falls through to out_put_job: drivers/accel/rocket/rocket_job.c:rocket_ioctl_submit_job() { ... ret = rocket_copy_tasks(dev, file, job, rjob); if (ret) goto out_cleanup_job; ... out_cleanup_job: if (ret) drm_sched_job_cleanup(&rjob->base); out_put_job: rocket_job_put(rjob); return ret; } This calls rocket_job_cleanup(), which unconditionally passes job->domain to rocket_iommu_domain_put(): drivers/accel/rocket/rocket_job.c:rocket_job_cleanup() { ... rocket_iommu_domain_put(job->domain); ... } But rocket_iommu_domain_put() does not check if domain is NULL before passing it to kref_put(): drivers/accel/rocket/rocket_drv.c:rocket_iommu_domain_put() { kref_put(&domain->kref, rocket_iommu_domain_destroy); } Since job->domain is assigned after rocket_copy_tasks() succeeds, could this dereference a NULL pointer if an early failure occurs? [Severity: Medium] This is a pre-existing issue, but does this code silently ignore job submission errors? In rocket_ioctl_submit(), the loop calls rocket_ioctl_submit_job(), but the return value is not captured or checked: drivers/accel/rocket/rocket_job.c:rocket_ioctl_submit() { ... for (i = 0; i < args->job_count; i++) rocket_ioctl_submit_job(dev, file, &jobs[i]); exit: kvfree(jobs); return ret; } Since ret remains 0, the ioctl will return success to userspace even if some or all jobs fail to submit. Will userspace assume the jobs were successfully queued in this case? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
