* [PATCH v2 0/3] accel/rocket: Validate task regcmd fields
@ 2026-10-04 16:47 Sidong Yang
2026-10-04 16:47 ` [PATCH v2 1/3] accel/rocket: Don't tear down the scheduler when drm_sched_init() fails Sidong Yang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sidong Yang @ 2026-10-04 16:47 UTC (permalink / raw)
To: Tomeu Vizoso; +Cc: Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, Sidong Yang
Patch 3 rejects misaligned regcmd addresses and out-of-range regcmd
counts at submission. It is unchanged from v1.
Patches 1 and 2 fix two pre-existing issues from Sashiko's review of
v1. Of its other findings, the ignored submit error [1], the
iommu_group leak [2] and the unchecked allocation in rocket_job_open()
[3] are handled by patches already on the list. Until [1] lands, a job
that patch 3 rejects is dropped, but the ioctl still returns 0. The
__u32 regcmd cannot truncate an IOVA, as the rockchip IOMMU aperture is
32-bit.
Tested on RK3588 (ROCK 5B+): Teflon MobileNet v1 output and latency are
unchanged, the bad regcmd values are rejected, and an injected
drm_sched_init() failure fails probe cleanly.
Changes in v2:
- Add patches 1 and 2.
- Rebase onto drm-misc-next.
v1: https://lore.kernel.org/r/20260711062137.36044-1-sidong.yang@furiosa.ai/
[1] https://lore.kernel.org/r/20260828064152.37822-4-Naixumogu@whut.edu.cn/
[2] https://lore.kernel.org/r/20260610071045.3414828-2-zhaojinming@uniontech.com/
[3] https://lore.kernel.org/r/20260818041505.1579320-1-triet.hoang.dev@gmail.com/
Sidong Yang (3):
accel/rocket: Don't tear down the scheduler when drm_sched_init()
fails
accel/rocket: Use an unsigned index to copy the job's tasks
accel/rocket: Validate task regcmd address and count on submission
drivers/accel/rocket/rocket_job.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
base-commit: 70456f05d4b6396b22048c4b8cd3cb98ecf9f9e3
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/3] accel/rocket: Don't tear down the scheduler when drm_sched_init() fails
2026-10-04 16:47 [PATCH v2 0/3] accel/rocket: Validate task regcmd fields Sidong Yang
@ 2026-10-04 16:47 ` Sidong Yang
2026-10-09 16:45 ` Jeff Hugo
2026-10-04 16:47 ` [PATCH v2 2/3] accel/rocket: Use an unsigned index to copy the job's tasks Sidong Yang
2026-10-04 16:47 ` [PATCH v2 3/3] accel/rocket: Validate task regcmd address and count on submission Sidong Yang
2 siblings, 1 reply; 6+ messages in thread
From: Sidong Yang @ 2026-10-04 16:47 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, Sidong Yang, Sashiko
drm_sched_init() cleans up after itself when it fails, but
rocket_job_init() then calls drm_sched_fini() on the same scheduler.
That wakes a wait queue that was never initialised, dereferencing a
NULL pointer, and can destroy the submit workqueue a second time.
Only destroy the reset workqueue on this path.
Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260912114524.7A6131F00893@smtp.kernel.org/
Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
drivers/accel/rocket/rocket_job.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index f40435505818..319365ad7976 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -494,8 +494,6 @@ int rocket_job_init(struct rocket_core *core)
return 0;
err_sched:
- drm_sched_fini(&core->sched);
-
destroy_workqueue(core->reset.wq);
return ret;
}
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 2/3] accel/rocket: Use an unsigned index to copy the job's tasks
2026-10-04 16:47 [PATCH v2 0/3] accel/rocket: Validate task regcmd fields Sidong Yang
2026-10-04 16:47 ` [PATCH v2 1/3] accel/rocket: Don't tear down the scheduler when drm_sched_init() fails Sidong Yang
@ 2026-10-04 16:47 ` Sidong Yang
2026-10-09 16:46 ` Jeff Hugo
2026-10-04 16:47 ` [PATCH v2 3/3] accel/rocket: Validate task regcmd address and count on submission Sidong Yang
2 siblings, 1 reply; 6+ messages in thread
From: Sidong Yang @ 2026-10-04 16:47 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, Sidong Yang, Sashiko
task_count is a u32 from userspace, but rocket_copy_tasks() walks it
with an int index, which would overflow for counts above INT_MAX. That
is not reachable today, since kvmalloc_objs() rejects any count above
INT_MAX / 16, but use an unsigned index to match the type.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260711063638.61B411F000E9@smtp.kernel.org/
Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
drivers/accel/rocket/rocket_job.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 319365ad7976..428de5a7cb03 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -78,7 +78,7 @@ rocket_copy_tasks(struct drm_device *dev,
return -ENOMEM;
}
- for (int i = 0; i < rjob->task_count; i++) {
+ for (unsigned int i = 0; i < rjob->task_count; i++) {
struct drm_rocket_task task = {0};
if (copy_from_user(&task,
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/3] accel/rocket: Use an unsigned index to copy the job's tasks
2026-10-04 16:47 ` [PATCH v2 2/3] accel/rocket: Use an unsigned index to copy the job's tasks Sidong Yang
@ 2026-10-09 16:46 ` Jeff Hugo
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Hugo @ 2026-10-09 16:46 UTC (permalink / raw)
To: Sidong Yang, Tomeu Vizoso; +Cc: Oded Gabbay, dri-devel, linux-kernel, Sashiko
On 10/4/2026 10:47 AM, Sidong Yang wrote:
> task_count is a u32 from userspace, but rocket_copy_tasks() walks it
> with an int index, which would overflow for counts above INT_MAX. That
> is not reachable today, since kvmalloc_objs() rejects any count above
> INT_MAX / 16, but use an unsigned index to match the type.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260711063638.61B411F000E9@smtp.kernel.org/
> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] accel/rocket: Validate task regcmd address and count on submission
2026-10-04 16:47 [PATCH v2 0/3] accel/rocket: Validate task regcmd fields Sidong Yang
2026-10-04 16:47 ` [PATCH v2 1/3] accel/rocket: Don't tear down the scheduler when drm_sched_init() fails Sidong Yang
2026-10-04 16:47 ` [PATCH v2 2/3] accel/rocket: Use an unsigned index to copy the job's tasks Sidong Yang
@ 2026-10-04 16:47 ` Sidong Yang
2 siblings, 0 replies; 6+ messages in thread
From: Sidong Yang @ 2026-10-04 16:47 UTC (permalink / raw)
To: Tomeu Vizoso; +Cc: Oded Gabbay, Jeff Hugo, dri-devel, linux-kernel, Sidong Yang
The regcmd fields in drm_rocket_task come from userspace and are
programmed into the PC unit without any validation.
Bits 31:4 of PC_BASE_ADDRESS hold the register command DMA address
and bit 0 selects slave mode, so a misaligned regcmd silently drops
its low bits, and an odd address flips the PC unit into slave mode.
Similarly, pc_data_amount is a 16-bit field holding
(regcmd_count + 1) / 2 - 1, so a larger regcmd_count is silently
truncated by the register encoding.
Reject unaligned regcmd addresses and out-of-range regcmd_count with
-EINVAL at submission time. Existing userspace is not affected: Mesa
places regcmd buffers at 64-byte aligned offsets, and its regcmd
counts stay far below the limit.
Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
drivers/accel/rocket/rocket_job.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 428de5a7cb03..ce994748dd5d 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -7,6 +7,7 @@
#include <drm/drm_file.h>
#include <drm/drm_gem.h>
#include <drm/rocket_accel.h>
+#include <linux/align.h>
#include <linux/interrupt.h>
#include <linux/overflow.h>
#include <linux/iommu.h>
@@ -21,6 +22,15 @@
#define JOB_TIMEOUT_MS 500
+/*
+ * The PC unit fetches two 64-bit register commands per pc_data_amount unit,
+ * and the field holds (regcmd_count + 1) / 2 - 1.
+ */
+#define ROCKET_MAX_REGCMDS ((PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT__MASK + 1) * 2U)
+
+/* Bits 3:0 of PC_BASE_ADDRESS hold the mode selection bit and reserved bits */
+#define ROCKET_REGCMD_ALIGN 16
+
static struct rocket_job *
to_rocket_job(struct drm_sched_job *sched_job)
{
@@ -95,6 +105,20 @@ rocket_copy_tasks(struct drm_device *dev,
goto fail;
}
+ if (task.regcmd_count > ROCKET_MAX_REGCMDS) {
+ drm_dbg(dev, "regcmd_count field in drm_rocket_task should be <= %u.\n",
+ ROCKET_MAX_REGCMDS);
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ if (!IS_ALIGNED(task.regcmd, ROCKET_REGCMD_ALIGN)) {
+ drm_dbg(dev, "regcmd field in drm_rocket_task should be aligned to %u bytes.\n",
+ ROCKET_REGCMD_ALIGN);
+ ret = -EINVAL;
+ goto fail;
+ }
+
rjob->tasks[i].regcmd = task.regcmd;
rjob->tasks[i].regcmd_count = task.regcmd_count;
}
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-09 16:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-04 16:47 [PATCH v2 0/3] accel/rocket: Validate task regcmd fields Sidong Yang
2026-10-04 16:47 ` [PATCH v2 1/3] accel/rocket: Don't tear down the scheduler when drm_sched_init() fails Sidong Yang
2026-10-09 16:45 ` Jeff Hugo
2026-10-04 16:47 ` [PATCH v2 2/3] accel/rocket: Use an unsigned index to copy the job's tasks Sidong Yang
2026-10-09 16:46 ` Jeff Hugo
2026-10-04 16:47 ` [PATCH v2 3/3] accel/rocket: Validate task regcmd address and count on submission 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®