* [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; 11+ 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] 11+ 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:25 ` sashiko-bot 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, 2 replies; 11+ 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 related [flat|nested] 11+ 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:25 ` sashiko-bot 2026-08-28 5:44 ` Sidong Yang 1 sibling, 0 replies; 11+ messages in thread From: sashiko-bot @ 2026-08-28 5:25 UTC (permalink / raw) To: MoGGuU; +Cc: dri-devel Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: Pre-existing issues: - [High] Integer overflow in the rocket_copy_tasks() loop counter leads to an out-of-bounds write in kernel memory. - [High] The execution fence is not attached to input BOs, allowing concurrent modifications from other execution contexts. - [High] rocket_job_handle_irq() leaks an IOMMU group reference on every interrupt. -- commit 47c63098f063d8d22a7ebbc0a4a54fb92597a27b Author: MoGGuU <Naixumogu@whut.edu.cn> Date: Fri Aug 28 05:08:03 2026 +0000 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. 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. > 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)) [Severity: High] This isn't a bug introduced by this patch, but does this function miss attaching the execution fence to the input BOs? Later in rocket_job_push(), the inference_done_fence is attached to the output BOs, but the same is not done for the input 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); ... } Without attaching a read fence to the input BOs' dma_resv objects, subsequent jobs that attempt to write to these input BOs will not wait for the NPU to finish reading. Can this lead to data corruption from concurrent access? [ ... ] > @@ -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; > + [Severity: High] This is a pre-existing issue, but can the signed loop counter in rocket_copy_tasks() overflow and cause an out-of-bounds write? Since job->task_count is a u32 and not validated against INT_MAX like the BO counts above, if a user submits a job with task_count >= 0x80000001 (assuming a large enough memory allocation succeeds), the signed integer `i` in the copy loop will overflow when it reaches 0x7FFFFFFF and becomes -2147483648: drivers/accel/rocket/rocket_job.c:rocket_copy_tasks() { ... for (int i = 0; i < rjob->task_count; i++) { ... rjob->tasks[i].regcmd = task.regcmd; ... } Because rjob->task_count is unsigned, the comparison `i < rjob->task_count` uses unsigned arithmetic. As a result, the negative `i` is treated as a valid index, causing `rjob->tasks[i].regcmd` to write out-of-bounds at a negative offset in kernel memory. [Severity: High] This is also a pre-existing issue, but does rocket_job_handle_irq() leak an IOMMU group reference on every interrupt? In the interrupt handler, an IOMMU group reference is acquired but never released: drivers/accel/rocket/rocket_job.c:rocket_job_handle_irq() { ... iommu_detach_group(NULL, iommu_group_get(core->dev)); ... } Since iommu_group_get() acquires a reference, and iommu_detach_group() does not consume it, the reference is leaked. Should there be a matching iommu_group_put() call here to prevent a permanent memory and reference leak? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260828050805.38548-1-Naixumogu@whut.edu.cn?part=1 ^ permalink raw reply [flat|nested] 11+ 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:25 ` sashiko-bot @ 2026-08-28 5:44 ` Sidong Yang 1 sibling, 0 replies; 11+ 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] 11+ 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:22 ` sashiko-bot 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, 2 replies; 11+ 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 related [flat|nested] 11+ 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:22 ` sashiko-bot 2026-08-28 5:46 ` Sidong Yang 1 sibling, 0 replies; 11+ messages in thread From: sashiko-bot @ 2026-08-28 5:22 UTC (permalink / raw) To: MoGGuU; +Cc: dri-devel Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Missing read fence attachment for input BOs can lead to data races (Write-after-Read hazards). - [High] Duplicate GEM objects in job submission arrays cause the IOCTL to fail with -EALREADY. -- commit bd2a88e8890e01dc0085ebd07568e63dc8dda773 Author: MoGGuU <Naixumogu@whut.edu.cn> accel/rocket: Collect job dependencies before arming This commit moves the job dependency collection to occur before the scheduler job is armed and the scheduler lock is taken. This ensures that memory allocation failures during dependency collection do not leave the armed job in an invalid state that cannot be safely aborted. 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 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; [Severity: High] This is a pre-existing issue, but does the array construction preceding this error check allow userspace to trigger an -EALREADY error? Looking at the setup before this point in rocket_job_push(): 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); If a userspace client submits an in-place inference job where the same BO is used as both an input and an output (or passed multiple times as input), the bos array will contain duplicates. When drm_gem_lock_reservations() attempts to lock the same reservation object twice with the same context, it fails with -EALREADY, breaking the API and denying valid requests. > + 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); > [ ... ] > > drm_sched_entity_push_job(&job->base); > } [Severity: High] This isn't a bug introduced by this patch, but are the input BOs left unsynchronized here? Looking further down in rocket_job_push(), the completion fence is attached only to the output BOs: rocket_attach_object_fences(job->out_bos, job->out_bo_count, job->inference_done_fence); Because the input BOs are entirely omitted, if an unprivileged userspace client submits an inference job with input buffers, then immediately submits a job to another engine (like a GPU) to write to those same buffers, can a Write-after-Read hazard occur? It seems the inference_done_fence isn't attached to the in_bos reservation objects to protect the read, leading to potential data corruption. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260828050805.38548-1-Naixumogu@whut.edu.cn?part=2 ^ permalink raw reply [flat|nested] 11+ 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:22 ` sashiko-bot @ 2026-08-28 5:46 ` Sidong Yang 2026-08-28 5:50 ` Sidong Yang 1 sibling, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ 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:19 ` sashiko-bot 2026-08-28 5:47 ` Sidong Yang 2 siblings, 2 replies; 11+ 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 related [flat|nested] 11+ 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:19 ` sashiko-bot 2026-08-28 5:47 ` Sidong Yang 1 sibling, 0 replies; 11+ messages in thread From: sashiko-bot @ 2026-08-28 5:19 UTC (permalink / raw) To: MoGGuU; +Cc: dri-devel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Unrecoverable partial job submission due to missing progress reporting. -- commit 62a6d62ce92d524109acbfd3cf943820e07879c2 Author: MoGGuU <Naixumogu@whut.edu.cn> accel/rocket: Propagate job submission errors This commit updates rocket_ioctl_submit() to return the first error encountered during job submission and stop processing remaining jobs. Previously, errors from individual job submissions were ignored. > 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); [Severity: High] How does userspace determine how many jobs were successfully submitted before the error occurred? When rocket_ioctl_submit() halts on the first error, the previously submitted jobs remain queued, giving ordered partial-submit semantics. However, looking at the UAPI struct drm_rocket_submit in include/uapi/drm/rocket_accel.h: struct drm_rocket_submit { __u64 jobs; __u32 job_count; __u32 job_struct_size; __u64 reserved; }; There is no output field (like jobs_submitted) to communicate the partial success state back to userspace, and the kernel does not modify the incoming job_count. Without knowing the index of the failed job, if a user application encounters an error (for example, due to an invalid BO handle or memory allocation failure), how can it safely recover? If userspace retries the submission, it risks double-submitting already queued jobs, which can corrupt hardware state. If it aborts entirely, it may hang waiting for implicit fences on BOs that were successfully submitted. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260828050805.38548-1-Naixumogu@whut.edu.cn?part=3 ^ permalink raw reply [flat|nested] 11+ 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:19 ` sashiko-bot @ 2026-08-28 5:47 ` Sidong Yang 1 sibling, 0 replies; 11+ 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] 11+ messages in thread
end of thread, other threads:[~2026-08-28 7:45 UTC | newest] Thread overview: 11+ 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:25 ` sashiko-bot 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:22 ` sashiko-bot 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:19 ` sashiko-bot 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; as well as URLs for NNTP newsgroup(s).