Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] The ioctl introduces ordered partial-submit semantics but fails to report how many jobs were successfully submitted, making error recovery impossible for userspace. Pre-existing issues: - [High] Unbounded memory allocation due to missing limit on args->job_count. -- commit 41a2028e803565dfe6bad77987c577fafe68aee4 Author: MoGGuU <[email protected]> accel/rocket: Propagate job submission errors The patch updates rocket_ioctl_submit() to check the return value of each job submission instead of discarding it. It now returns the first error encountered and stops submitting the remaining jobs, providing ordered partial-submit semantics. > diff --git a/drivers/accel/rocket/rocket_job.c > b/drivers/accel/rocket/rocket_job.c > index 1634c30ede456..cd2ae1def74d1 100644 > --- a/drivers/accel/rocket/rocket_job.c > +++ b/drivers/accel/rocket/rocket_job.c > @@ -647,8 +647,11 @@ int rocket_ioctl_submit(struct drm_device *dev, void > *data, struct drm_file *fil > } > > [Severity: High] This is a pre-existing issue, but is there a missing limit on args->job_count before the loop? Looking at rocket_ioctl_submit(), there is an unbounded allocation based on this user-controlled parameter earlier in the function: jobs = kvmalloc_objs(*jobs, args->job_count); if (!jobs) { drm_dbg(dev, "Failed to allocate incoming job array\n"); return -ENOMEM; } Could an untrusted user supply a maliciously large args->job_count to cause system memory exhaustion and invoke the OOM killer? > - for (i = 0; i < args->job_count; i++) > - rocket_ioctl_submit_job(dev, file, &jobs[i]); > + for (i = 0; i < args->job_count; i++) { > + ret = rocket_ioctl_submit_job(dev, file, &jobs[i]); > + if (ret) > + goto exit; [Severity: High] Does this code fail to report how many jobs were successfully submitted when it aborts partially through? When a job fails partway through the submission loop, the ioctl aborts and propagates the error, leaving any previously queued jobs in place (ordered partial-submit semantics). But without returning the progress state (e.g., which index failed) to userspace, doesn't this create an unrecoverable job state ambiguity? If userspace retries, they might duplicate execution of the already-submitted jobs. > + } > > exit: > kvfree(jobs); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
