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@xxxxxxxxxxxxxxx
> Signed-off-by: MoGGuU <Naixumogu@xxxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>