On Thu, Aug 13, 2026 at 10:20:59PM +0800, MoGGuU wrote:
> Several error paths in the job submission path can be triggered by
> unprivileged userspace through malformed DRM_ROCKET_SUBMIT requests.
I think it would be better to split this per fix.
And I've tested this patch, it works.
>
> First, the input and output BO handle counts are __u32, but GEM lookup and
> reservation helpers take int counts. Values outside the signed range can
> become negative. A combined count above INT_MAX can also overflow at the
> call sites. Reject counts that cannot be represented safely before looking
> up the BOs.
>
> Second, rocket_job_push() arms the scheduler job before collecting its
> implicit dependencies. Dependency collection can fail with -ENOMEM, but
> drm_sched_job_arm() is a point of no return. An armed job must be pushed;
> it must not be aborted with drm_sched_job_cleanup(). Collect dependencies
> before taking the scheduler lock and arming the job. Only operations that
> cannot fail remain after arm().
>
> Finally, rocket_ioctl_submit() discards each job's return value and reports
> success even when every job fails. Return the first error
> and stop submitting the remaining jobs. Jobs queued before an error remain
> queued, giving the ioctl ordered partial-submit semantics.
>
> Tested on RK3588 with zero task counts, invalid task pointers, invalid BO
> handles, and oversized BO counts. The requests returned the expected error
> codes without warnings or errors in the kernel log.
>
> Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
> Cc: [email protected]
> Signed-off-by: MoGGuU <[email protected]>
> ---
> drivers/accel/rocket/rocket_job.c | 34 +++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/accel/rocket/rocket_job.c
> b/drivers/accel/rocket/rocket_job.c
> index bb77b6bf0..c42bbf486 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -206,21 +206,20 @@ static int rocket_job_push(struct rocket_job *job)
> if (ret)
> goto err;
>
> + ret = rocket_acquire_object_fences(job->in_bos, job->in_bo_count,
> + &job->base, false);
> + if (ret)
> + goto err_unlock;
> +
> + ret = rocket_acquire_object_fences(job->out_bos, job->out_bo_count,
> + &job->base, true);
> + if (ret)
> + goto err_unlock;
> +
> scoped_guard(mutex, &rdev->sched_lock) {
> drm_sched_job_arm(&job->base);
> -
> job->inference_done_fence =
> dma_fence_get(&job->base.s_fence->finished);
> -
> - ret = rocket_acquire_object_fences(job->in_bos,
> job->in_bo_count, &job->base, false);
> - if (ret)
> - goto err_unlock;
> -
> - ret = rocket_acquire_object_fences(job->out_bos,
> job->out_bo_count, &job->base, true);
> - if (ret)
> - goto err_unlock;
> -
> kref_get(&job->refcount); /* put by scheduler job completion */
> -
> drm_sched_entity_push_job(&job->base);
> }
>
> @@ -556,6 +555,12 @@ static int rocket_ioctl_submit_job(struct drm_device
> *dev, struct drm_file *file
> if (job->task_count == 0)
> return -EINVAL;
>
> + /* GEM lookup and reservation helpers take signed object counts. */
> + if (job->in_bo_handle_count > INT_MAX ||
> + job->out_bo_handle_count > INT_MAX ||
> + job->in_bo_handle_count > INT_MAX - job->out_bo_handle_count)
> + return -EINVAL;
> +
> rjob = kzalloc_obj(*rjob);
> if (!rjob)
> return -ENOMEM;
> @@ -639,8 +644,11 @@ int rocket_ioctl_submit(struct drm_device *dev, void
> *data, struct drm_file *fil
> }
>
>
> - 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;
> + }
>
> exit:
> kvfree(jobs);
>
> base-commit: d85a5e9dfb42449d9f37b0ddc6ec30b129f481ce
> --
> 2.43.0
>
Tested-by: Sidong Yang <[email protected]>
