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

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

[Haoming Gao]

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 v5:
- Replaced the nickname in the author identity and Signed-off-by tags with my
  real name, as requested by Greg Kroah-Hartman.

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.

v4: https://lore.kernel.org/r/20260828061916.20405-1-Naixumogu@whut.edu.cn
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

Haoming Gao (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 v5 1/3] accel/rocket: Validate BO handle counts on job submission

[Haoming Gao]

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: Haoming Gao <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 v5 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] The job's completion fence is not attached to input BOs, causing a Write-After-Read data corruption vulnerability.
- [High] Missing practical upper bound for BO handle counts allows hard lockups and memory exhaustion.
- [High] Signed integer overflow in task copy loop leads to out-of-bounds array access and memory corruption.
--

commit b8a6c0c385bb7b291f18a1ccd50949d83223c9d8
Author: Haoming Gao <Naixumogu@whut.edu.cn>

accel/rocket: Validate BO handle counts on job submission

This commit validates the input and output BO handle counts during job
submission to ensure they do not exceed INT_MAX. It also changes the
type of the combined count in rocket_job_push to prevent silent
truncation before passing it to GEM reservation helpers.

> 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))

[Severity: High]
This is a pre-existing issue, but does rocket_job_push() leave input BOs
unprotected against concurrent writers?

Later in rocket_job_push(), the completion fence is only attached to output
BOs:

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

Because the completion fence is never recorded on the input BOs with a read
usage flag, could this lead to a write-after-read data corruption?

If userspace submits a job that reads an input BO, and subsequently a second
job that writes to the same BO, the scheduler might execute the second job
immediately without waiting for the first job to finish reading, allowing the
input data to be overwritten prematurely.

[ ... ]

> @@ -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;

[Severity: High]
This isn't a bug introduced by this patch, but does rocket_copy_tasks() have
an integer overflow in the task copy loop?

In rocket_ioctl_submit_job(), task_count comes from userspace as a u32 and
is passed to rocket_copy_tasks(). If job->task_count exceeds INT_MAX
(e.g., 0x80000005) and the array allocation succeeds, the iteration uses a
signed int:

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;
                rjob->tasks[i].regcmd_count = task.regcmd_count;
        }
        ...
}

Because i is compared against the unsigned rjob->task_count, the loop
continues after i overflows and wraps to negative values. Will this result in
an out-of-bounds array access and memory corruption when writing to
rjob->tasks[i] with a negative index?

>  
> +	/* 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 does bounding the BO counts
to INT_MAX still allow for hard lockups?

If userspace submits a job with in_bo_handle_count set to INT_MAX, the check
passes and drm_gem_objects_lookup() will allocate massive arrays and iterate
over 2 billion times inside spin_lock(&filp->table_lock).

Could holding a spinlock for that duration exceed the NMI watchdog timeout
and cause a system crash due to a hard lockup?

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

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

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

[Haoming Gao]

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: Haoming Gao <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 v5 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 implicit synchronization for input BOs causes data corruption.
--

commit 00caeb1bd76ecd4176b74ead738756a7e287a306
Author: Haoming Gao <Naixumogu@whut.edu.cn>

accel/rocket: Collect job dependencies before arming

This commit fixes a job submission bug by collecting implicit dependencies
for input and output buffer objects before taking the scheduler lock and
arming the job. This ensures that dependency collection failures do not
cause an armed job to be aborted.

> 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);

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

	scoped_guard(mutex, &rdev->sched_lock) {
		...
		drm_sched_entity_push_job(&job->base);
	}

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

err_unlock:

Does this code also need to attach the completion fence to the input BOs?

While prior dependencies on in_bos are correctly acquired (ensuring we don't
read before prior writers finish), the job's completion fence
(inference_done_fence) is only attached to out_bos.

Because there is no read fence added to the input buffers, could future
writers (from userspace, CPU, or other GPU engines) bypass synchronization
and overwrite these BOs while the NPU is still actively reading them,
causing data corruption?

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

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

[Haoming Gao]

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: Haoming Gao <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 v5 3/3] accel/rocket: Propagate job submission errors

[sashiko-bot]

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

Pre-existing issues:
- [High] Unbounded `job_count` allows memory exhaustion and memcg limit bypass (Denial of Service).
--

commit 25d3cd6d4d07a127ac97a2a68ff62409e8f7d1fd
Author: Haoming Gao <Naixumogu@whut.edu.cn>

accel/rocket: Propagate job submission errors

This commit modifies rocket_ioctl_submit() to return the first error
encountered during job submission and stops processing remaining jobs,
leaving previously queued jobs intact.

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> --- 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]
This isn't a bug introduced by this patch, but is it possible for an
unprivileged user to exhaust system memory here?

The loop iterates based on args->job_count without an apparent upper bound.
During each iteration, rocket_ioctl_submit_job() allocates struct rocket_job,
task arrays, and DRM scheduler objects.

Because these are allocated using kzalloc_obj and kvmalloc_objs (which
default to GFP_KERNEL without __GFP_ACCOUNT), they are not charged to the
caller's memory cgroup.

Could a user pass a massive job_count to the ioctl via the DRM render node
to bypass container limits and trigger a system-wide out of memory condition?

>  
>  exit:
>  	kvfree(jobs);

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