[PATCH] accel/rocket: Fix job submit error handling

[PATCH] accel/rocket: Fix job submit error handling

[MoGGuU]

Several error paths in the job submission path can be triggered by
unprivileged userspace through malformed DRM_ROCKET_SUBMIT requests.

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: stable@vger.kernel.org
Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>
---
 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

Re: [PATCH] accel/rocket: Fix job submit error handling

[Sidong Yang]

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: stable@vger.kernel.org
> Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>
> ---
>  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 <sidong.yang@furiosa.ai>

Re: [PATCH] accel/rocket: Fix job submit error handling

[MoGGuU]

Thanks for testing the patch and for the suggestion.

I have split the three independent fixes into separate patches and will send
a v2 series. I will carry your Tested-by tag.

Regards,
MoGGuU