* Re: [PATCH v2] drm/rocket: Check allocations before use
2026-08-17 9:30 ` [PATCH v2] " Igor Paunovic
@ 2026-08-17 13:14 ` Triet Hoang
2026-08-17 14:01 ` [PATCH v4 1/2] " Triet Hoang
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Triet Hoang @ 2026-08-17 13:14 UTC (permalink / raw)
To: royalnet026; +Cc: dri-devel, linux-kernel, ogabbay, tomeu, triet.hoang.dev
From: Triet Hoang <triet.hoang.dev@gmail.com>
Hi Igor,
Thanks for the suggestion. I agree that keeping the allocation in rocket_file_priv
would make the ownership clearer. I will implement this idea as a seperate patch.
Best Regards,
Triet
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH v4 1/2] drm/rocket: Check allocations before use
2026-08-17 9:30 ` [PATCH v2] " Igor Paunovic
2026-08-17 13:14 ` Triet Hoang
@ 2026-08-17 14:01 ` Triet Hoang
2026-08-17 14:01 ` [PATCH v4 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv Triet Hoang
2026-08-17 16:40 ` [PATCH v4 1/2] drm/rocket: Check allocations before use Markus Elfring
2026-08-17 14:28 ` [PATCH v2] " Triet Hoang
2026-08-18 4:15 ` [PATCH v5 1/2] " Triet Hoang
3 siblings, 2 replies; 21+ messages in thread
From: Triet Hoang @ 2026-08-17 14:01 UTC (permalink / raw)
To: tomeu
Cc: ogabbay, dri-devel, linux-kernel, royalnet026, Markus.Elfring,
Triet Hoang
From: Triet Hoang <triet.hoang.dev@gmail.com>
Check the result of kvmalloc_array() in rocket_job_push() and
kmalloc_objs() in rocket_job_open() before using the allocated
buffers.
Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
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.
drivers/accel/rocket/rocket_job.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index ac51bff39833..adcc792541ec 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -192,6 +192,9 @@ static int rocket_job_push(struct rocket_job *job)
bos = kvmalloc_array(job->in_bo_count + job->out_bo_count, sizeof(void *),
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 *));
@@ -501,6 +504,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;
@@ -509,9 +515,9 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
scheds,
rdev->num_cores, NULL);
if (WARN_ON(ret))
- return ret;
+ kfree(scheds);
- return 0;
+ return ret;
}
void rocket_job_close(struct rocket_file_priv *rocket_priv)
--
2.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv
2026-08-17 14:01 ` [PATCH v4 1/2] " Triet Hoang
@ 2026-08-17 14:01 ` Triet Hoang
2026-08-17 17:06 ` Markus Elfring
2026-08-17 16:40 ` [PATCH v4 1/2] drm/rocket: Check allocations before use Markus Elfring
1 sibling, 1 reply; 21+ messages in thread
From: Triet Hoang @ 2026-08-17 14:01 UTC (permalink / raw)
To: tomeu
Cc: ogabbay, dri-devel, linux-kernel, royalnet026, Markus.Elfring,
Triet Hoang
From: Triet Hoang <triet.hoang.dev@gmail.com>
Keep the scheduler allocation in rocket_file_priv
and free it unconditionally in rocket_job_close().
Suggested-by: Igor Paunovic <royalnet026@gmail.com>
Link: https://lore.kernel.org/all/20260817093009.22359-1-royalnet026@gmail.com/#t
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
drivers/accel/rocket/rocket_drv.h | 1 +
drivers/accel/rocket/rocket_job.c | 4 ++++
2 files changed, 5 insertions(+)
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 adcc792541ec..ff1d9e802024 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -514,8 +514,11 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
DRM_SCHED_PRIORITY_NORMAL,
scheds,
rdev->num_cores, NULL);
+
if (WARN_ON(ret))
kfree(scheds);
+ else
+ rocket_priv->scheds = scheds;
return ret;
}
@@ -525,6 +528,7 @@ void rocket_job_close(struct rocket_file_priv *rocket_priv)
struct drm_sched_entity *entity = &rocket_priv->sched_entity;
kfree(entity->sched_list);
+ kfree(rocket_priv->scheds);
drm_sched_entity_destroy(entity);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv
2026-08-17 14:01 ` [PATCH v4 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv Triet Hoang
@ 2026-08-17 17:06 ` Markus Elfring
0 siblings, 0 replies; 21+ messages in thread
From: Markus Elfring @ 2026-08-17 17:06 UTC (permalink / raw)
To: Triet Hoang, dri-devel, Oded Gabbay, Tomeu Vizoso; +Cc: LKML, Igor Paunovic
…
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -514,8 +514,11 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
> DRM_SCHED_PRIORITY_NORMAL,
> scheds,
> rdev->num_cores, NULL);
> +
> if (WARN_ON(ret))
> kfree(scheds);
…
Why do you think that an additional blank would be helpful at this place?
Regards,
Markus
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] drm/rocket: Check allocations before use
2026-08-17 14:01 ` [PATCH v4 1/2] " Triet Hoang
2026-08-17 14:01 ` [PATCH v4 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv Triet Hoang
@ 2026-08-17 16:40 ` Markus Elfring
2026-08-18 1:31 ` Triet Hoang
1 sibling, 1 reply; 21+ messages in thread
From: Markus Elfring @ 2026-08-17 16:40 UTC (permalink / raw)
To: Triet Hoang, dri-devel, Oded Gabbay, Tomeu Vizoso
Cc: LKML, kernel-janitors, Igor Paunovic
…
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -192,6 +192,9 @@ static int rocket_job_push(struct rocket_job *job)
>
> bos = kvmalloc_array(job->in_bo_count + job->out_bo_count, sizeof(void *),
> GFP_KERNEL);
…
I see opportunities for corresponding collateral evolution.
How do you think about to apply the attribute “__free(kvfree)” also in such
a function implementation by another update step?
https://elixir.bootlin.com/linux/v7.2-rc7/source/include/linux/slab.h#L1420
Regards,
Markus
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] drm/rocket: Check allocations before use
2026-08-17 16:40 ` [PATCH v4 1/2] drm/rocket: Check allocations before use Markus Elfring
@ 2026-08-18 1:31 ` Triet Hoang
2026-08-18 5:48 ` [v4 " Markus Elfring
0 siblings, 1 reply; 21+ messages in thread
From: Triet Hoang @ 2026-08-18 1:31 UTC (permalink / raw)
To: markus.elfring
Cc: dri-devel, kernel-janitors, linux-kernel, ogabbay, royalnet026,
tomeu, triet.hoang.dev
> I see opportunities for corresponding collateral evolution.
> How do you think about to apply the attribute “__free(kvfree)” also in such
> a function implementation by another update step?
I am not sure I understand your idea.
Could you explain more? This attribute is a new thing to media
Regards,
Triet
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v4 1/2] drm/rocket: Check allocations before use
2026-08-18 1:31 ` Triet Hoang
@ 2026-08-18 5:48 ` Markus Elfring
2026-08-18 6:21 ` [PATCH] " Triet Hoang
0 siblings, 1 reply; 21+ messages in thread
From: Markus Elfring @ 2026-08-18 5:48 UTC (permalink / raw)
To: Triet Hoang, dri-devel
Cc: kernel-janitors, linux-kernel, Igor Paunovic, Oded Gabbay,
Tomeu Vizoso
>> I see opportunities for corresponding collateral evolution.
>> How do you think about to apply the attribute “__free(kvfree)” also in such
>> a function implementation by another update step?
>
> I am not sure I understand your idea.
> Could you explain more? This attribute is a new thing to media
There are programming interfaces supported to some degree
for the application of scope-based resource management.
https://elixir.bootlin.com/linux/v7.2-rc7/source/include/linux/cleanup.h#L10
Would you like to take any adjustment possibilities better into account accordingly?
Regards,
Markus
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] drm/rocket: Check allocations before use
2026-08-18 5:48 ` [v4 " Markus Elfring
@ 2026-08-18 6:21 ` Triet Hoang
0 siblings, 0 replies; 21+ messages in thread
From: Triet Hoang @ 2026-08-18 6:21 UTC (permalink / raw)
To: markus.elfring
Cc: dri-devel, kernel-janitors, linux-kernel, ogabbay, royalnet026,
tomeu, triet.hoang.dev
> > There are programming interfaces supported to some degree
> for the application of scope-based resource management.
> https://elixir.bootlin.com/linux/v7.2-rc7/source/include/linux/cleanup.h#L10
>
> Would you like to take any adjustment possibilities better into account accordingly?
Thanks for explaining. I am happy to continue working on this, but I think
this would be more of a refactoring to use the new API than a bug fix.
Furthermore, I don't have much experience with the new cleanup API yet, so
I think it would be better to consider this as a separate change in the
future, after these patches have been reviewed and I have gained more
experience with the new API.
Anyway, thanks for suggesting this. I'll take a look at it and keep it in
mind for future changes.
Regards,
Triet
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] drm/rocket: Check allocations before use
2026-08-17 9:30 ` [PATCH v2] " Igor Paunovic
2026-08-17 13:14 ` Triet Hoang
2026-08-17 14:01 ` [PATCH v4 1/2] " Triet Hoang
@ 2026-08-17 14:28 ` Triet Hoang
2026-08-18 1:55 ` Triet Hoang
2026-08-18 1:56 ` Triet Hoang
2026-08-18 4:15 ` [PATCH v5 1/2] " Triet Hoang
3 siblings, 2 replies; 21+ messages in thread
From: Triet Hoang @ 2026-08-17 14:28 UTC (permalink / raw)
To: royalnet026; +Cc: dri-devel, linux-kernel, ogabbay, tomeu, triet.hoang.dev
Hi Igor,
After reading the code carefully, I think that we can check
if entity->sched_list is NULL or not, then we can free the array
that rocket_job_open() allocated.
This will look like
if (WARN_ON(ret) || !(&rocket_priv->sched_entity->sched_list))
kfree(scheds);
How do you think about that
Best Regards,
Triet
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] drm/rocket: Check allocations before use
2026-08-17 14:28 ` [PATCH v2] " Triet Hoang
@ 2026-08-18 1:55 ` Triet Hoang
2026-08-18 1:56 ` Triet Hoang
1 sibling, 0 replies; 21+ messages in thread
From: Triet Hoang @ 2026-08-18 1:55 UTC (permalink / raw)
To: triet.hoang.dev; +Cc: dri-devel, linux-kernel, ogabbay, royalnet026, tomeu
> This will look like
> if (WARN_ON(ret) || !(&rocket_priv->sched_entity->sched_list))
> kfree(scheds);
> How do you think about that
Hi Igor,
This is a bad idea. Sorry about that and just forget it.
Regards,
Triet
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] drm/rocket: Check allocations before use
2026-08-17 14:28 ` [PATCH v2] " Triet Hoang
2026-08-18 1:55 ` Triet Hoang
@ 2026-08-18 1:56 ` Triet Hoang
1 sibling, 0 replies; 21+ messages in thread
From: Triet Hoang @ 2026-08-18 1:56 UTC (permalink / raw)
To: triet.hoang.dev; +Cc: dri-devel, linux-kernel, ogabbay, royalnet026, tomeu
> This will look like
> if (WARN_ON(ret) || !(&rocket_priv->sched_entity->sched_list))
> kfree(scheds);
> How do you think about that
Hi Igor,
This is a bad idea. Sorry about that and just forget it.
Regards,
Triet
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 1/2] drm/rocket: Check allocations before use
2026-08-17 9:30 ` [PATCH v2] " Igor Paunovic
` (2 preceding siblings ...)
2026-08-17 14:28 ` [PATCH v2] " Triet Hoang
@ 2026-08-18 4:15 ` Triet Hoang
2026-08-18 4:15 ` [PATCH v5 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv Triet Hoang
3 siblings, 1 reply; 21+ messages in thread
From: Triet Hoang @ 2026-08-18 4:15 UTC (permalink / raw)
To: royalnet026
Cc: dri-devel, linux-kernel, ogabbay, tomeu, triet.hoang.dev,
markus.elfring
Check the result of kvmalloc_array() in rocket_job_push() and
kmalloc_objs() in rocket_job_open() before using the allocated
buffers.
Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
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().
---
drivers/accel/rocket/rocket_job.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index ac51bff39833..15e93c355c38 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -189,9 +189,15 @@ static int rocket_job_push(struct rocket_job *job)
struct drm_gem_object **bos;
struct ww_acquire_ctx acquire_ctx;
int ret = 0;
+ size_t bos_count;
+
+ if (check_add_overflow(job->in_bo_count, job->out_bo_count, &bos_count))
+ return -EOVERFLOW;
+
+ bos = kvmalloc_array(bos_count, sizeof(void *), GFP_KERNEL);
+ if (!bos)
+ return -ENOMEM;
- bos = kvmalloc_array(job->in_bo_count + job->out_bo_count, sizeof(void *),
- GFP_KERNEL);
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 *));
@@ -501,6 +507,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;
@@ -509,9 +518,9 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
scheds,
rdev->num_cores, NULL);
if (WARN_ON(ret))
- return ret;
+ kfree(scheds);
- return 0;
+ return ret;
}
void rocket_job_close(struct rocket_file_priv *rocket_priv)
--
2.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv
2026-08-18 4:15 ` [PATCH v5 1/2] " Triet Hoang
@ 2026-08-18 4:15 ` Triet Hoang
2026-08-18 4:23 ` [PATCH v5] drm/rocket: Check allocations before use Triet Hoang
0 siblings, 1 reply; 21+ messages in thread
From: Triet Hoang @ 2026-08-18 4:15 UTC (permalink / raw)
To: royalnet026
Cc: dri-devel, linux-kernel, ogabbay, tomeu, triet.hoang.dev,
markus.elfring
Keep the scheduler allocation in rocket_file_priv
and free it unconditionally in rocket_job_close().
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>
---
Changses in v5:
- Free rocket_priv->scheds instead of entity->sched_list in rocket_job_close().
---
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 15e93c355c38..50b6e0e06ed8 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -519,6 +519,8 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
rdev->num_cores, NULL);
if (WARN_ON(ret))
kfree(scheds);
+ else
+ rocket_priv->scheds = scheds;
return ret;
}
@@ -527,7 +529,7 @@ void rocket_job_close(struct rocket_file_priv *rocket_priv)
{
struct drm_sched_entity *entity = &rocket_priv->sched_entity;
- kfree(entity->sched_list);
+ kfree(rocket_priv->scheds);
drm_sched_entity_destroy(entity);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5] drm/rocket: Check allocations before use
2026-08-18 4:15 ` [PATCH v5 2/2] drm/rocket: Keep scheduler allocation in rocket_file_priv Triet Hoang
@ 2026-08-18 4:23 ` Triet Hoang
0 siblings, 0 replies; 21+ messages in thread
From: Triet Hoang @ 2026-08-18 4:23 UTC (permalink / raw)
To: triet.hoang.dev
Cc: dri-devel, linux-kernel, markus.elfring, ogabbay, royalnet026,
tomeu
Hi Everyone,
Thanks for the detailed review. This is my first time contributing to the
kernel and using the mailing list, so I really appreciate your feedback.
The thread has become quite long, so I'll leave it here for now and wait
for feedback from Tomeu before making further changes.
> I have not run your patch, so no tag from me. If it would help, I can
> test it on RK3588 with three cores and again with two of them unbound,
> and check the single-core case with kmemleak.
I don't have the hardware to test these cases myself, so it would be very
helpful if you could run the patch (v5) and share any feedback. It would also
help me better understand the behavior of the driver and the testing
process.
Thanks again for taking the time to review and test it.
Regards,
Triet
^ permalink raw reply [flat|nested] 21+ messages in thread