mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/3] accel/rocket: Fix job submit error handling
@ 2026-08-28  5:08 MoGGuU
  2026-08-28  5:08 ` [PATCH v3 1/3] accel/rocket: Validate BO handle counts on job submission MoGGuU
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: MoGGuU @ 2026-08-28  5:08 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Oded Gabbay, Jeff Hugo, Sidong Yang, dri-devel, linux-kernel, stable

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 v3:
- Keep the individual count checks before GEM lookup, but change 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 current drm-misc-next branch.
- Dropped incidental blank-line-only changes from the original patch.
- Added Sidong Yang's Tested-by tag.

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: 20331505df9c7d8b29f2f9309231607a41a74120
-- 
2.43.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] accel/rocket: Validate BO handle counts on job submission
  2026-08-28  5:08 [PATCH v3 0/3] accel/rocket: Fix job submit error handling MoGGuU
@ 2026-08-28  5:08 ` MoGGuU
  2026-08-28  5:44   ` Sidong Yang
  2026-08-28  5:08 ` [PATCH v3 2/3] accel/rocket: Collect job dependencies before arming MoGGuU
  2026-08-28  5:08 ` [PATCH v3 3/3] accel/rocket: Propagate job submission errors MoGGuU
  2 siblings, 1 reply; 8+ messages in thread
From: MoGGuU @ 2026-08-28  5:08 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Oded Gabbay, Jeff Hugo, Sidong Yang, dri-devel, linux-kernel, stable

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] accel/rocket: Collect job dependencies before arming
  2026-08-28  5:08 [PATCH v3 0/3] accel/rocket: Fix job submit error handling MoGGuU
  2026-08-28  5:08 ` [PATCH v3 1/3] accel/rocket: Validate BO handle counts on job submission MoGGuU
@ 2026-08-28  5:08 ` MoGGuU
  2026-08-28  5:46   ` Sidong Yang
  2026-08-28  5:08 ` [PATCH v3 3/3] accel/rocket: Propagate job submission errors MoGGuU
  2 siblings, 1 reply; 8+ messages in thread
From: MoGGuU @ 2026-08-28  5:08 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Oded Gabbay, Jeff Hugo, Sidong Yang, dri-devel, linux-kernel, stable

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] accel/rocket: Propagate job submission errors
  2026-08-28  5:08 [PATCH v3 0/3] accel/rocket: Fix job submit error handling MoGGuU
  2026-08-28  5:08 ` [PATCH v3 1/3] accel/rocket: Validate BO handle counts on job submission MoGGuU
  2026-08-28  5:08 ` [PATCH v3 2/3] accel/rocket: Collect job dependencies before arming MoGGuU
@ 2026-08-28  5:08 ` MoGGuU
  2026-08-28  5:47   ` Sidong Yang
  2 siblings, 1 reply; 8+ messages in thread
From: MoGGuU @ 2026-08-28  5:08 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Oded Gabbay, Jeff Hugo, Sidong Yang, dri-devel, linux-kernel, stable

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] accel/rocket: Validate BO handle counts on job submission
  2026-08-28  5:08 ` [PATCH v3 1/3] accel/rocket: Validate BO handle counts on job submission MoGGuU
@ 2026-08-28  5:44   ` Sidong Yang
  0 siblings, 0 replies; 8+ messages in thread
From: Sidong Yang @ 2026-08-28  5:44 UTC (permalink / raw)
  To: MoGGuU
  Cc: Tomeu Vizoso, Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, stable

On Fri, Aug 28, 2026 at 01:08:03PM +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>
> 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
> 

Looks good now, thanks.

Reviewed-by: Sidong Yang <sidong.yang@furiosa.ai>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/3] accel/rocket: Collect job dependencies before arming
  2026-08-28  5:08 ` [PATCH v3 2/3] accel/rocket: Collect job dependencies before arming MoGGuU
@ 2026-08-28  5:46   ` Sidong Yang
  2026-08-28  5:50     ` Sidong Yang
  0 siblings, 1 reply; 8+ messages in thread
From: Sidong Yang @ 2026-08-28  5:46 UTC (permalink / raw)
  To: MoGGuU
  Cc: Tomeu Vizoso, Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, stable

On Fri, Aug 28, 2026 at 01:08:04PM +0800, MoGGuU wrote:
> 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 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
> 

Reviewed-by: Sidong Yang <sidong.ynag@furiosa.ai>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] accel/rocket: Propagate job submission errors
  2026-08-28  5:08 ` [PATCH v3 3/3] accel/rocket: Propagate job submission errors MoGGuU
@ 2026-08-28  5:47   ` Sidong Yang
  0 siblings, 0 replies; 8+ messages in thread
From: Sidong Yang @ 2026-08-28  5:47 UTC (permalink / raw)
  To: MoGGuU
  Cc: Tomeu Vizoso, Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, stable

On Fri, Aug 28, 2026 at 01:08:05PM +0800, MoGGuU wrote:
> 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

I'm afraid that this patch changes userspace API. After this patch, userspace
programs get an error on submit. So it seems that the stable tag should be
dropped.

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

Reviewed-by: Sidong Yang <sidong.yang@furiosa.ai>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/3] accel/rocket: Collect job dependencies before arming
  2026-08-28  5:46   ` Sidong Yang
@ 2026-08-28  5:50     ` Sidong Yang
  0 siblings, 0 replies; 8+ messages in thread
From: Sidong Yang @ 2026-08-28  5:50 UTC (permalink / raw)
  To: MoGGuU
  Cc: Tomeu Vizoso, Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, stable

On Fri, Aug 28, 2026 at 02:46:16PM +0900, Sidong Yang wrote:
> On Fri, Aug 28, 2026 at 01:08:04PM +0800, MoGGuU wrote:
> > 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 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
> > 
> 
> Reviewed-by: Sidong Yang <sidong.ynag@furiosa.ai>

Sorry, I typo'd my address in the previous mail. It should be:

Reviewed-by: Sidong Yang <sidong.yang@furiosa.ai>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-28  5:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-28  5:08 [PATCH v3 0/3] accel/rocket: Fix job submit error handling MoGGuU
2026-08-28  5:08 ` [PATCH v3 1/3] accel/rocket: Validate BO handle counts on job submission MoGGuU
2026-08-28  5:44   ` Sidong Yang
2026-08-28  5:08 ` [PATCH v3 2/3] accel/rocket: Collect job dependencies before arming MoGGuU
2026-08-28  5:46   ` Sidong Yang
2026-08-28  5:50     ` Sidong Yang
2026-08-28  5:08 ` [PATCH v3 3/3] accel/rocket: Propagate job submission errors MoGGuU
2026-08-28  5:47   ` Sidong Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®