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

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

[MoGGuU]

Several independent error paths in the Rocket job submission ioctl can be
triggered by unprivileged userspace. This series validates userspace BO
counts before passing them to helpers with signed count parameters, collects
implicit dependencies before the scheduler job is armed, and propagates the
first per-job submission error to userspace.

The series was 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.

Changes in v2:
- Split the three independent fixes into separate patches as suggested by
  Sidong Yang.
- Rebased onto the current drm-misc-next branch.
- Dropped incidental blank-line-only changes from the original patch.
- Added Sidong Yang's Tested-by tag.

v1: https://lore.kernel.org/r/20260813142059.151644-1-Naixumogu@whut.edu.cn

MoGGuU (3):
  accel/rocket: Validate BO handle counts on job submission
  accel/rocket: Collect job dependencies before arming
  accel/rocket: Propagate job submission errors

 drivers/accel/rocket/rocket_job.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)


base-commit: 20331505df9c7d8b29f2f9309231607a41a74120
-- 
2.43.0

[PATCH v2 1/3] accel/rocket: Validate BO handle counts on job submission

[MoGGuU]

The input and output BO handle counts are __u32, while GEM lookup and
reservation helpers take int counts. A count above INT_MAX, or a combined
count above INT_MAX, cannot be represented safely at those call sites.

Reject such counts before looking up the BOs.

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Cc: stable@vger.kernel.org
Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>
---
 drivers/accel/rocket/rocket_job.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index bb77b6bf0f231..7e3d123afc5ad 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -556,6 +556,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;
-- 
2.43.0

Re: [PATCH v2 1/3] accel/rocket: Validate BO handle counts on job submission

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Integer overflow in `rocket_copy_tasks` allows arbitrary out-of-bounds write of user data.
- [High] iommu_group_get() refcount leak in rocket_job_handle_irq.
- [High] Missing implicit execution fence for input BOs allows write-after-read hazards.
--

commit 335ce4beaa240be7a9d792c09e9cc0672c21caac
Author: MoGGuU <Naixumogu@whut.edu.cn>

accel/rocket: Validate BO handle counts on job submission

The input and output BO handle counts are __u32, while GEM lookup and
reservation helpers take int counts. A count above INT_MAX, or a combined
count above INT_MAX, cannot be represented safely at those call sites.

Reject such counts before looking up the BOs.

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index bb77b6bf0f231..7e3d123afc5ad 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -556,6 +556,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;

[Severity: High]
This isn't a bug introduced by this patch, but looking at rocket_job_handle_irq():

drivers/accel/rocket/rocket_job.c:rocket_job_handle_irq()
			iommu_detach_group(NULL, iommu_group_get(core->dev));
			dma_fence_signal(core->in_flight_job->done_fence);

Does this leak the IOMMU group reference?

iommu_group_get() increments the reference count of the returned IOMMU group,
but iommu_detach_group() doesn't consume this reference. Since there is no
matching iommu_group_put(), won't this reference leak on every successfully
completed job?

[Severity: High]
This is a pre-existing issue, but looking at rocket_job_push():

drivers/accel/rocket/rocket_job.c:rocket_job_push()
		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);
	}

	rocket_attach_object_fences(job->out_bos, job->out_bo_count, job->inference_done_fence);

Does this omit the required implicit execution fence attachment for input BOs?

The execution fence (inference_done_fence) is attached to out_bos, but it
seems in_bos are skipped. If in_bos don't receive the execution fence, could
this cause a write-after-read hazard if subsequent operations write to these
BOs while the NPU is still reading them?

[Severity: Critical]
This is a pre-existing issue, but looking at rocket_copy_tasks():

drivers/accel/rocket/rocket_job.c:rocket_copy_tasks()
	for (int i = 0; i < rjob->task_count; i++) {
		struct drm_rocket_task task = {0};

		if (copy_from_user(&task,
				   u64_to_user_ptr(job->tasks) + i * job->task_struct_size,
				   sizeof(task))) {
			drm_dbg(dev, "Failed to copy incoming tasks\n");
			ret = -EFAULT;
			goto fail;
		}

[ ... ]

		rjob->tasks[i].regcmd = task.regcmd;
		rjob->tasks[i].regcmd_count = task.regcmd_count;
	}

Does this code overflow the i counter?

Since int i is signed and rjob->task_count is a u32, if a user submits a job
with a task_count greater than INT_MAX, could i increment past 0x7FFFFFFF and
overflow to negative values? This would mean the loop condition
i < rjob->task_count evaluates to true due to unsigned promotion, but the
array access rjob->tasks[i] would use a negative index, leading to an
out-of-bounds write with user-controlled data.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827170608.39511-1-Naixumogu@whut.edu.cn?part=1

Re: [PATCH v2 1/3] accel/rocket: Validate BO handle counts on job submission

[Sidong Yang]

On Fri, Aug 28, 2026 at 01:06:06AM +0800, MoGGuU wrote:
> The input and output BO handle counts are __u32, while GEM lookup and
> reservation helpers take int counts. A count above INT_MAX, or a combined
> count above INT_MAX, cannot be represented safely at those call sites.
> 
> Reject such counts before looking up the BOs.
> 
> Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
> Cc: stable@vger.kernel.org
> Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
> Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>
> ---
>  drivers/accel/rocket/rocket_job.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index bb77b6bf0f231..7e3d123afc5ad 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -556,6 +556,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)

I think checking in/out is okay but the sum of in/out would be checked with check_add_overflow()
in rocket_job_push(). But it only caches overflow UINT_MAX because bo_count is u32. It seems that it
would be good to change bo_count to int.
ㅏ

> +		return -EINVAL;
> +
>  	rjob = kzalloc_obj(*rjob);
>  	if (!rjob)
>  		return -ENOMEM;
> -- 
> 2.43.0
> 

Re: [PATCH v2 1/3] accel/rocket: Validate BO handle counts on job submission

[MoGGuU]

Thanks, that makes sense.

The two individual checks still need to happen before
drm_gem_objects_lookup(), because its count argument is int. For the
combined count, changing bo_count in rocket_job_push() from u32 to int
makes check_add_overflow() reject sums above INT_MAX before the count is
passed to the reservation helpers.

I addressed this in v3:
https://lore.kernel.org/r/20260828050805.38548-2-Naixumogu@whut.edu.cn

Regards,
MoGGuU

[PATCH v2 2/3] accel/rocket: Collect job dependencies before arming

[MoGGuU]

rocket_job_push() arms the scheduler job before collecting its implicit
dependencies. Dependency collection can fail with -ENOMEM, but an armed
job must be pushed and 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 drm_sched_job_arm().

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Cc: stable@vger.kernel.org
Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>
---
 drivers/accel/rocket/rocket_job.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 7e3d123afc5ad..51e8b43c05131 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -206,19 +206,21 @@ 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);
-- 
2.43.0

Re: [PATCH v2 2/3] accel/rocket: Collect job dependencies before arming

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing read-fence attachment on input BOs leaves buffers unprotected from subsequent writers.
--

commit ac74f22db3a0d06711502b6864c7b006136b26f3
Author: MoGGuU <Naixumogu@whut.edu.cn>

accel/rocket: Collect job dependencies before arming

rocket_job_push() arms the scheduler job before collecting its implicit
dependencies. Dependency collection can fail with -ENOMEM, but an armed
job must be pushed and must not be aborted with drm_sched_job_cleanup().

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 7e3d123afc5ad..51e8b43c05131 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
[ ... ]
> @@ -206,19 +206,21 @@ static int rocket_job_push(struct rocket_job *job)
[ ... ]
>  	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);

[Severity: High]
This is a pre-existing issue, but does this leave the input buffers
unprotected from subsequent writers?

Looking at rocket_job_push() in drivers/accel/rocket/rocket_job.c, the
completion fence (job->inference_done_fence) is only attached to job->out_bos
just after this scoped_guard block:

    rocket_attach_object_fences(job->out_bos, job->out_bo_count,
                                job->inference_done_fence);

The job->in_bos receive no completion fence.

Furthermore, rocket_attach_object_fences() hardcodes DMA_RESV_USAGE_WRITE:

    for (i = 0; i < bo_count; i++)
        dma_resv_add_fence(bos[i]->resv, fence, DMA_RESV_USAGE_WRITE);

This hardcoding prevents it from correctly attaching a read-fence for in_bos
even if it were called for them.

If a subsequent job writes to these input BOs, could it begin execution
without waiting for the current job to finish reading them? Since it will
lock the reservations and find no implicit read dependency from this job,
this might lead to hardware data races if unprivileged users submit sequential
jobs where one reads and the next writes to the same BO.

Should we also attach a read-fence to the input BOs and allow
rocket_attach_object_fences() to specify the usage type?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827170608.39511-1-Naixumogu@whut.edu.cn?part=2

[PATCH v2 3/3] accel/rocket: Propagate job submission errors

[MoGGuU]

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.

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Cc: stable@vger.kernel.org
Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>
---
 drivers/accel/rocket/rocket_job.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 51e8b43c05131..964d12475a0d5 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
 	}
 
 
-	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);
-- 
2.43.0

Re: [PATCH v2 3/3] accel/rocket: Propagate job submission errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Returning `-ERESTARTSYS` after partially submitting jobs causes implicit resubmission, executing already queued jobs twice.

Pre-existing issues:
- [High] The driver fails to attach the job's execution fence to its input BOs, causing read-after-write hazards.
- [High] Reference count leak of the IOMMU group in rocket_job_handle_irq() due to unmatched iommu_group_get().
--

commit 2749eed226ce8a85f8e14c26ff4f82f1e0274b87
Author: MoGGuU <Naixumogu@whut.edu.cn>

    accel/rocket: Propagate job submission errors
    
    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.
    
    Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
    Cc: stable@vger.kernel.org
    Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
    Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 51e8b43c05131..964d12475a0d5 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
>  	}
>  
>  
> -	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]
If rocket_ioctl_submit_job() fails on a later iteration (for example, by
returning -ERESTARTSYS or -EINTR when drm_gem_lock_reservations() is
interrupted by a signal), will returning this error to userspace cause
the entire batch of jobs to be resubmitted?

Because jobs 0 through i - 1 have already been pushed to the DRM
scheduler, it appears returning -ERESTARTSYS would cause the kernel
to restart the ioctl, executing the already queued jobs a second time
and potentially causing data corruption.

> +	}
>  
>  exit:
>  	kvfree(jobs);


While reviewing this file, I noticed a couple of pre-existing issues in
other functions:

drivers/accel/rocket/rocket_job.c:rocket_job_push()
> 	rocket_attach_object_fences(job->out_bos, job->out_bo_count, job->inference_done_fence);
> 
> err_unlock:

[Severity: High]
This is a pre-existing issue, but does this function fail to attach the
job's execution fence to its input BOs?

It appears the job's completion fence is attached to the output BOs, but
the equivalent call for input BOs is missing. Without attaching the fence
to the input BOs (which should be attached with DMA_RESV_USAGE_READ), will
subsequent jobs that write to these input BOs fail to wait for the current
job to finish reading them, leading to hardware execution races?


drivers/accel/rocket/rocket_job.c:rocket_job_handle_irq()
> 			iommu_detach_group(NULL, iommu_group_get(core->dev));
> 			dma_fence_signal(core->in_flight_job->done_fence);

[Severity: High]
This isn't a bug introduced by this patch, but does this leak the IOMMU
group reference count?

During the handling of a completed job, iommu_group_get() increments the
reference count, but iommu_detach_group() does not consume it. Since the
pointer isn't saved, there is no corresponding iommu_group_put() call.
This could eventually overflow the reference count or prevent the group
from ever being freed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827170608.39511-1-Naixumogu@whut.edu.cn?part=3