[PATCH v6 0/2] accel/rocket: Fix allocation handling and scheduler array leak

[PATCH v6 0/2] accel/rocket: Fix allocation handling and scheduler array leak

[Triet Hoang]

This small series addresses allocation and memory management issues in 
the accel/rocket driver:

1. Patch 1 ensures proper allocation checks before using pointers 
   in rocket_job_open()
2. Patch 2 fixes an unreachable memory leak of the scheduler array when 
   running on single-core setups (e.g., num_cores == 1), where 
   drm_sched_entity_init() drops the array pointer.

Link to v5: https://lore.kernel.org/all/20260818041505.1579320-1-triet.hoang.dev@gmail.com/

Changes in v6:
- Rebase on top of drm-misc-next (base commit 5052ea2e6bfc43c64b851c596d3675d17e05e631)
- Addressed review feedback from previous iterations.
- Added Fixes and Tested-by tags where applicable.

Triet Hoang (2):
  drm/rocket: Check allocations before use
  drm/rocket: Keep scheduler allocation in rocket_file_priv

 drivers/accel/rocket/rocket_drv.h |  1 +
 drivers/accel/rocket/rocket_job.c | 13 +++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH v6 1/2] accel/rocket: Check allocations before use

[Triet Hoang]

Check the result of kmalloc_objs() in rocket_job_open() 
before using the allocated buffers.

Also replace the WARN_ON(ret) check in rocket_job_open() with a plain if
since ret can be -ENOMEM from drm_sched_entity_stats_new(). 
That is an allocation failure, not a driver bug, so a plain if 
seems better than a stack trace.

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
Tested-by: Igor Paunovic <royalnet026@gmail.com>
Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
---
Changes in v2:
- Free scheds when drm_sched_entity_init() fails.
- Initialize ret to 0.

Changes in v3:
- Move patch version descriptions below the '---' marker.

Changes in v4:
- Remove unnecessary initialization of ret to 0.
- Adjust commit message word wrapping.
- Add Fixes tag.

Changes in v5:
- Add check overflow before kvmalloc_array() in rocket_job_push().

Changes in v6:
- Rebase on top of drm-misc-next, remove the rocket_job_push() hunk.
- Change WARN_ON(ret) to plain if check in rocket_job_open().
- Add Tested-by tags.
---
 drivers/accel/rocket/rocket_job.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index f40435505818..704a15513179 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -516,6 +516,9 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
 	unsigned int core;
 	int ret;
 
+	if (!scheds)
+		return -ENOMEM;
+
 	for (core = 0; core < rdev->num_cores; core++)
 		scheds[core] = &rdev->cores[core].sched;
 
@@ -523,10 +526,10 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
 				    DRM_SCHED_PRIORITY_NORMAL,
 				    scheds,
 				    rdev->num_cores, NULL);
-	if (WARN_ON(ret))
-		return ret;
+	if (ret)
+		kfree(scheds);
 
-	return 0;
+	return ret;
 }
 
 void rocket_job_close(struct rocket_file_priv *rocket_priv)
-- 
2.53.0

Re: [PATCH v6 1/2] accel/rocket: Check allocations before use

[Igor Paunovic]

Hi Triet,

Re-ran v6 as posted, on its drm-misc-next base (5052ea2e6bfc), on
RK3588 (Orange Pi 5 Plus, three cores) with PROVE_LOCKING and
DEBUG_ATOMIC_SLEEP: 30 sequential clients, 10 pairs of concurrent
clients and 5 after runtime suspend, 55 inferences all bit-correct
against the reference, no warning or lockdep report, cores back in
runtime suspend afterwards. Same limit as on v5: the allocation
failure branches were reviewed, not fault-injected. The tag stands
for v6; please keep its comment:

Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, no fault injection

Regards,
Igor

[PATCH v6 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv

[Triet Hoang]

The scheduler array passed to drm_sched_entity_init() is retained by the
entity when the device has multiple cores. drm_sched_entity_init() stores 
the list conditionally:

	entity->sched_list = num_sched_list > 1 ? sched_list : NULL;

With one scheduler, it keeps only entity->rq and drops the array pointer
while rocket_job_open() allows its local copy to go out of scope. So
for num_cores == 1 the kfree() in rocket_job_close() is a kfree(NULL)
and the array leaks unreachably. For num_cores > 1 there is no leak,
which is why this is invisible in normal use on RK3588.

Keep the allocation in rocket_file_priv so rocket_job_close() can destroy
the entity before freeing the array. Free the array directly if entity
initialization fails, since rocket_job_open() does not call
rocket_job_close() on that error path.

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")

Suggested-by: Igor Paunovic <royalnet026@gmail.com>
Link: https://lore.kernel.org/all/20260817093009.22359-1-royalnet026@gmail.com/
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
Tested-by: Igor Paunovic <royalnet026@gmail.com>
Tested-by: Sidong Yang <sidong.yang@furiosa.ai>
---
Changes in v5:
- Free rocket_priv->scheds instead of entity->sched_list in rocket_job_close().

Changes in v6:
- Add Tested-by and Fixes tags.
- Move kfree() below the drm_sched_entity_destroy() call in rocket_job_close().
- Update commit message to explain why the scheduler array must outlive the scheduler entity.
---
 drivers/accel/rocket/rocket_drv.h | 1 +
 drivers/accel/rocket/rocket_job.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/rocket/rocket_drv.h b/drivers/accel/rocket/rocket_drv.h
index 2c673bb99ccc..9421e48ec5d8 100644
--- a/drivers/accel/rocket/rocket_drv.h
+++ b/drivers/accel/rocket/rocket_drv.h
@@ -23,6 +23,7 @@ struct rocket_file_priv {
 	struct drm_mm mm;
 	struct mutex mm_lock;
 
+	struct drm_gpu_scheduler **scheds;
 	struct drm_sched_entity sched_entity;
 };
 
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 704a15513179..f27e6264f91d 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -528,6 +528,8 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
 				    rdev->num_cores, NULL);
 	if (ret)
 		kfree(scheds);
+	else
+		rocket_priv->scheds = scheds;
 
 	return ret;
 }
@@ -536,8 +538,8 @@ void rocket_job_close(struct rocket_file_priv *rocket_priv)
 {
 	struct drm_sched_entity *entity = &rocket_priv->sched_entity;
 
-	kfree(entity->sched_list);
 	drm_sched_entity_destroy(entity);
+	kfree(rocket_priv->scheds);
 }
 
 int rocket_job_is_idle(struct rocket_core *core)
-- 
2.53.0

Re: [PATCH v6 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv

[Igor Paunovic]

Hi Triet,

Same run as on 1/2, on v6 as posted: ten pairs of concurrent clients,
so two rocket_file_priv instances with their own scheduler entities
alive and submitting at the same time, then closed, with the kfree()
now after drm_sched_entity_destroy(). All output bit-correct, no
lockdep or atomic-sleep report, clean autosuspend afterwards. The tag
stands for v6; please keep its comment:

Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, concurrent clients

Regards,
Igor