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

[PATCH v4 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 v4:
- Added Sidong Yang's Reviewed-by tags, using the corrected email address.
- Dropped Cc: stable from patch 3 because propagating submission errors
  changes userspace-visible ioctl behavior.
- Rebased onto the current drm-misc-next branch.

Changes in v3:
- Kept the individual count checks before GEM lookup, but changed bo_count to
  int so check_add_overflow() rejects combined counts above INT_MAX, as
  suggested by Sidong Yang.

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

v3: https://lore.kernel.org/r/20260828050805.38548-1-Naixumogu@whut.edu.cn
v2: https://lore.kernel.org/r/20260827170608.39511-1-Naixumogu@whut.edu.cn
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 | 32 ++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)


base-commit: f9c2f70ee41544717b3c209083073db1e21f919b
-- 
2.43.0

[PATCH v4 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 cannot be
represented safely by the GEM lookup helper.

Reject each count above INT_MAX before looking up the BOs.

rocket_job_push() already uses check_add_overflow() for the combined count,
but stores the result in u32, so it only detects unsigned wraparound. Store
the result in int so sums above INT_MAX are rejected before the count is
passed to the reservation helpers.

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Cc: stable@vger.kernel.org
Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
Reviewed-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, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index bb77b6bf0f231..e6052d1973afa 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -190,7 +190,7 @@ static int rocket_job_push(struct rocket_job *job)
 	struct rocket_device *rdev = job->rdev;
 	struct drm_gem_object **bos;
 	struct ww_acquire_ctx acquire_ctx;
-	u32 bo_count;
+	int bo_count;
 	int ret = 0;
 
 	if (check_add_overflow(job->in_bo_count, job->out_bo_count, &bo_count))
@@ -556,6 +556,11 @@ static int rocket_ioctl_submit_job(struct drm_device *dev, struct drm_file *file
 	if (job->task_count == 0)
 		return -EINVAL;
 
+	/* GEM lookup takes a signed object count. */
+	if (job->in_bo_handle_count > INT_MAX ||
+	    job->out_bo_handle_count > INT_MAX)
+		return -EINVAL;
+
 	rjob = kzalloc_obj(*rjob);
 	if (!rjob)
 		return -ENOMEM;
-- 
2.43.0

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

[Greg KH]

On Fri, Aug 28, 2026 at 02:19:14PM +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 cannot be
> represented safely by the GEM lookup helper.
> 
> Reject each count above INT_MAX before looking up the BOs.
> 
> rocket_job_push() already uses check_add_overflow() for the combined count,
> but stores the result in u32, so it only detects unsigned wraparound. Store
> the result in int so sums above INT_MAX are rejected before the count is
> passed to the reservation helpers.
> 
> Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
> Cc: stable@vger.kernel.org
> Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
> Reviewed-by: Sidong Yang <sidong.yang@furiosa.ai>
> Signed-off-by: MoGGuU <Naixumogu@whut.edu.cn>

Real name please.

thanks,

greg k-h

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

[Haoming Gao]

Thanks for pointing this out.

My real name is Haoming Gao. I have corrected the author identity and
Signed-off-by tags and will resend the series as v5.

Regards,
Haoming Gao

Re: [PATCH v4 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:
- [High] Missing BO deduplication causes -EALREADY on in-place operations.
- [High] Missing read fence attachment for input BOs breaks implicit synchronization.
- [High] Integer overflow in task parsing loop allows out-of-bounds memory write.
--

commit 88dcb66252c59bcede3a94150e673b09305b3bd6
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 cannot be
represented safely by the GEM lookup helper.

Reject each count above INT_MAX before looking up the BOs.

[ ... ]

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 3141f210fcd1b..13f4915c1b07c 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -190,7 +190,7 @@ static int rocket_job_push(struct rocket_job *job)
>  	struct rocket_device *rdev = job->rdev;
>  	struct drm_gem_object **bos;
>  	struct ww_acquire_ctx acquire_ctx;
> -	u32 bo_count;
> +	int bo_count;
>  	int ret = 0;
>  
>  	if (check_add_overflow(job->in_bo_count, job->out_bo_count, &bo_count))
>  		return -EINVAL;
>  
>  	bos = kvmalloc_array(bo_count, sizeof(*bos), GFP_KERNEL);
>  	if (!bos)
>  		return -ENOMEM;
>  	memcpy(bos, job->in_bos, job->in_bo_count * sizeof(void *));
>  	memcpy(&bos[job->in_bo_count], job->out_bos, job->out_bo_count * sizeof(void *));
>  
>  	ret = drm_gem_lock_reservations(bos, bo_count, &acquire_ctx);

[Severity: High]
This is a pre-existing issue, but does this submission path handle duplicate
BOs correctly?

Since the in_bos and out_bos arrays are concatenated without deduplication,
if userspace submits a job where the same BO handle appears in both arrays
(e.g., an in-place accelerator operation), drm_gem_lock_reservations() will
lock the array sequentially. 

When it hits the duplicate object, dma_resv_lock_interruptible() detects the
ww_mutex is already held by the same context and returns -EALREADY. Since
drm_gem_lock_reservations() does not handle -EALREADY, it will abort the
submission.

>  	if (ret)
>  		goto err;

[ ... ]

>  	scoped_guard(mutex, &rdev->sched_lock) {
>  		drm_sched_job_arm(&job->base);
>  
>  		job->inference_done_fence = dma_fence_get(&job->base.s_fence->finished);
>  
>  		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);

[Severity: High]
This is a pre-existing issue, but does this code properly synchronize the
input BOs?

The completion fence is exclusively attached to the output buffers. Because
the fence is never attached to in_bos (which should receive a read usage
fence), the DMA reservation object of the input buffers remains unaware of
this job's execution.

Could this lead to a write-after-read data corruption if subsequent jobs
acquire a write lock on those input buffers without waiting for this job
to finish reading?

[ ... ]

> @@ -557,6 +557,11 @@ static int rocket_ioctl_submit_job(struct drm_device *dev, struct drm_file *file
>  	if (job->task_count == 0)
>  		return -EINVAL;
>  
> +	/* GEM lookup takes a signed object count. */
> +	if (job->in_bo_handle_count > INT_MAX ||
> +	    job->out_bo_handle_count > INT_MAX)
> +		return -EINVAL;
> +

[Severity: High]
This isn't a bug introduced by this patch, but since we are limiting the BO
counts to INT_MAX to prevent integer overflow, should we do the same for
job->task_count?

If job->task_count exceeds INT_MAX (e.g., 0x80000001), it can cause an integer
overflow in 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};

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

When the signed counter i reaches 0x7FFFFFFF, i++ wraps it to negative. The
loop condition i < rjob->task_count promotes i to u32, so it continues,
leading to out-of-bounds writes using a negative array index.

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

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

[PATCH v4 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>
Reviewed-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 e6052d1973afa..b55e12aecfe64 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 v4 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] Job completion fence is not attached to input BOs, breaking write-after-read implicit synchronization.
--

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

accel/rocket: Collect job dependencies before arming

This commit modifies rocket_job_push() to collect implicit dependencies
before taking the scheduler lock and arming the job. This prevents an armed
job from being incorrectly aborted if dependency collection fails with
-ENOMEM.

Cc: stable@vger.kernel.org

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 13f4915c1b07c..1634c30ede456 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);
>  	}
>
> 	rocket_attach_object_fences(job->out_bos, job->out_bo_count, job->inference_done_fence);

[Severity: High]
This is a pre-existing issue, but does this code miss attaching the completion
fence to the input buffers?

If a userspace application submits a job with input buffers, then immediately
submits another job (or performs a CPU write) to modify those same buffers, the
hardware might read corrupted or new data because it is still reading from them.

Because no read fence is explicitly attached to the input buffers (in_bos)
after the job is pushed, the DRM scheduler and CPU wait paths will not see a
dependency and will not wait for the inference job to complete its read
operations.

Should the driver also explicitly attach the completion fence to job->in_bos
(perhaps with DMA_RESV_USAGE_READ) so that subsequent writers wait for this job
to finish reading?

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

[PATCH v4 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")
Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
Reviewed-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 b55e12aecfe64..8f1bdf4a57f20 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -646,8 +646,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 v4 3/3] accel/rocket: Propagate job submission errors

[sashiko-bot]

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 <Naixumogu@whut.edu.cn>
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/20260828061916.20405-1-Naixumogu@whut.edu.cn?part=3