[PATCH 0/4] accel/rocket: resource leak and stability improvements

[PATCH 0/4] accel/rocket: resource leak and stability improvements

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

Hi,

This series fixes rocket driver handling on certain error paths to
avoid unexpected resource leaks and panics.

Chaoyi Chen (4):
  accel/rocket: Fix the IOMMU domain leak in rocket_ioctl_create_bo
  accel/rocket: Fix the panic in rocket_iommu_domain_put when the domain
    is NULL
  accel/rocket: Fix the extra iommu_group_get call in
    rocket_job_handle_irq
  MAINTAINERS: accel/rocket: Add rockchip mail list for rocket

 MAINTAINERS                       | 1 +
 drivers/accel/rocket/rocket_drv.c | 3 ++-
 drivers/accel/rocket/rocket_gem.c | 2 ++
 drivers/accel/rocket/rocket_job.c | 2 +-
 4 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.53.0

[PATCH 1/4] accel/rocket: Fix the IOMMU domain leak in rocket_ioctl_create_bo

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

When rocket_ioctl_create_bo fails, rocket_iommu_domain_put should be
called to avoid an IOMMU domain leak.

Fixes: 658ebeac3351 ("accel/rocket: Add IOCTL for BO creation")
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---
 drivers/accel/rocket/rocket_gem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/accel/rocket/rocket_gem.c b/drivers/accel/rocket/rocket_gem.c
index a5fffa51ff35..438694ba567b 100644
--- a/drivers/accel/rocket/rocket_gem.c
+++ b/drivers/accel/rocket/rocket_gem.c
@@ -127,6 +127,8 @@ int rocket_ioctl_create_bo(struct drm_device *dev, void *data, struct drm_file *
 	mutex_unlock(&rocket_priv->mm_lock);
 
 err:
+	rocket_iommu_domain_put(rkt_obj->domain);
+	rkt_obj->domain = NULL;
 	drm_gem_shmem_object_free(gem_obj);
 
 	return ret;
-- 
2.53.0

[PATCH 2/4] accel/rocket: Fix the panic in rocket_iommu_domain_put when the domain is NULL

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

In rocket_ioctl_submit_job(), since the domain is assigned last, an
error before that triggers rocket_job_put() -> rocket_job_cleanup()
-> rocket_iommu_domain_put() with the domain still NULL,
causing a panic in that function.

Therefore, the input parameters should be validated.

Fixes: 658ebeac3351 ("accel/rocket: Add IOCTL for BO creation")
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---
 drivers/accel/rocket/rocket_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
index 8bbbce594883..77cd2ecce001 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -65,7 +65,8 @@ rocket_iommu_domain_get(struct rocket_file_priv *rocket_priv)
 void
 rocket_iommu_domain_put(struct rocket_iommu_domain *domain)
 {
-	kref_put(&domain->kref, rocket_iommu_domain_destroy);
+	if (domain)
+		kref_put(&domain->kref, rocket_iommu_domain_destroy);
 }
 
 static int
-- 
2.53.0

[PATCH 3/4] accel/rocket: Fix the extra iommu_group_get call in rocket_job_handle_irq

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

The rocket attaches the IOMMU group in rocket_job_run() and detaches it
in rocket_job_handle_irq(). Calling  iommu_group_get() in
rocket_job_handle_irq() causes the reference count to increase
unexpectedly. And this causes rocket_core_fini() to fail to release the
IOMMU group, leading to a resource leak.

Fixes: 658ebeac3351 ("accel/rocket: Add IOCTL for BO creation")
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---
 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 2f1861f960cc..db10848926d8 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -340,7 +340,7 @@ static void rocket_job_handle_irq(struct rocket_core *core)
 				return;
 			}
 
-			iommu_detach_group(NULL, iommu_group_get(core->dev));
+			iommu_detach_group(NULL, core->iommu_group);
 			dma_fence_signal(core->in_flight_job->done_fence);
 			pm_runtime_put_autosuspend(core->dev);
 			core->in_flight_job = NULL;
-- 
2.53.0

[PATCH 4/4] MAINTAINERS: accel/rocket: Add rockchip mail list for rocket

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

The rocket is a DRM accel driver for the Rockchip NPU. Add the Rockchip
mailing listso that changes can be tracked.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4a8b0fd665ce..388ec1758ce7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7942,6 +7942,7 @@ F:	include/uapi/drm/ivpu_accel.h
 DRM ACCEL DRIVER FOR ROCKCHIP NPU
 M:	Tomeu Vizoso <tomeu@tomeuvizoso.net>
 L:	dri-devel@lists.freedesktop.org
+L:	linux-rockchip@lists.infradead.org
 S:	Supported
 T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	Documentation/accel/rocket/
-- 
2.53.0

Re: [PATCH 1/4] accel/rocket: Fix the IOMMU domain leak in rocket_ioctl_create_bo

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The return value of `iommu_map_sgtable` (`ssize_t`) is stored in an `int`, causing an overflow to a negative value for BOs of 2GB or larger, leading to a massive IOMMU mapping leak.
- [High] In `rocket_ioctl_prep_bo`, `dma_sync_sgtable_for_cpu` is called unconditionally even if `dma_resv_wait_timeout` fails with a timeout or error.
- [High] The per-file `rocket_priv` context is freed on file close, but GEM objects can outlive the file descriptor, causing a Use-After-Free when they are eventually destroyed.
--

--- Patch [1]: [PATCH 1/4] accel/rocket: Fix the IOMMU domain leak in rocket_ioctl_create_bo ---
commit ea4421559353add8c0bf8f5cce3e937cd76171d8
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

accel/rocket: Fix the IOMMU domain leak in rocket_ioctl_create_bo
    
When rocket_ioctl_create_bo fails, rocket_iommu_domain_put should be
called to avoid an IOMMU domain leak.

> diff --git a/drivers/accel/rocket/rocket_gem.c b/drivers/accel/rocket/rocket_gem.c
> index a5fffa51ff355..438694ba567b9 100644
> --- a/drivers/accel/rocket/rocket_gem.c
> +++ b/drivers/accel/rocket/rocket_gem.c
> @@ -127,6 +127,8 @@ int rocket_ioctl_create_bo(struct drm_device *dev, void *data, struct drm_file *
>  	mutex_unlock(&rocket_priv->mm_lock);
>  
>  err:
> +	rocket_iommu_domain_put(rkt_obj->domain);
> +	rkt_obj->domain = NULL;
>  	drm_gem_shmem_object_free(gem_obj);
>  
>  	return ret;

[Severity: Critical]
This is a pre-existing issue, but does rocket_ioctl_create_bo() in 
drivers/accel/rocket/rocket_gem.c truncate the return value of
iommu_map_sgtable() to a signed integer?

	ret = iommu_map_sgtable(rocket_priv->domain->domain,
				rkt_obj->mm.start,
				shmem_obj->sgt,
				IOMMU_READ | IOMMU_WRITE);
	if (ret < 0 || ret < args->size) {

If a user requests a buffer size of 2 GiB, iommu_map_sgtable() successfully
maps it and returns 2147483648 (0x80000000) as ssize_t. Is this truncated
into the local int ret, becoming -2147483648, which triggers the ret < 0
check? This would wrongly treat success as failure, bypass the IOMMU unmap
logic, and leak the IOMMU mappings for the freed backing pages.

[Severity: High]
This isn't a bug introduced by this patch, but can the per-file
rocket_priv context be freed while exported GEM objects still exist?

When a userspace client creates a GEM object, exports it, and then closes
its DRM file descriptor, rocket_postclose() in
drivers/accel/rocket/rocket_drv.c destructs rocket_priv:

	mutex_destroy(&rocket_priv->mm_lock);
	drm_mm_takedown(&rocket_priv->mm);
	rocket_iommu_domain_put(rocket_priv->domain);
	kfree(rocket_priv);

Later, when the final reference to the exported GEM object is dropped, would
rocket_gem_bo_free() in drivers/accel/rocket/rocket_gem.c dereference the
freed rocket_priv?

	mutex_lock(&rocket_priv->mm_lock);
	drm_mm_remove_node(&bo->mm);
	mutex_unlock(&rocket_priv->mm_lock);

This could lead to a use-after-free since the GEM object outlives the file
descriptor.

[Severity: High]
This is a pre-existing issue, but does rocket_ioctl_prep_bo() in
drivers/accel/rocket/rocket_gem.c unconditionally sync the buffer even if
the wait fails?

	ret = dma_resv_wait_timeout(gem_obj->resv, DMA_RESV_USAGE_WRITE, true, timeout);
	if (!ret)
		ret = timeout ? -ETIMEDOUT : -EBUSY;
	else if (ret > 0)
		ret = 0;

	shmem_obj = &to_rocket_bo(gem_obj)->base;

	dma_sync_sgtable_for_cpu(dev->dev, shmem_obj->sgt, DMA_BIDIRECTIONAL);

If dma_resv_wait_timeout() fails due to a timeout or interrupted wait
and returns a negative error code, the code continues executing without
an early return. Could this lead to data corruption since
dma_sync_sgtable_for_cpu() is invoked while the GPU may still be actively
writing to the buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814022453.437-1-kernel@airkyi.com?part=1

Re: [PATCH 2/4] accel/rocket: Fix the panic in rocket_iommu_domain_put when the domain is NULL

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:

New issues:
- [Medium] The commit message describes a factually incorrect premise regarding `domain` initialization.

Pre-existing issues:
- [Critical] GEM objects exported via PRIME can outlive their originating DRM file descriptor, causing a use-after-free in `rocket_gem_bo_free`.
- [Critical] Partial IOMMU mappings bypass the unmap error path in `rocket_ioctl_create_bo()`, leaking the mapping.
- [High] An IOMMU group reference is leaked on every interrupt handling due to an unbalanced `iommu_group_get()` call.
- [High] Calling `drm_sched_job_cleanup()` on an armed job corrupts the GPU scheduler timeline.
- [High] The loop in `rocket_ioctl_submit()` completely ignores the return value of `rocket_ioctl_submit_job()`.
- [High] Calling `memcpy` with a NULL pointer as the source when `job->in_bo_count` is 0.
- [High] The `scheds` array is leaked in `rocket_job_open`.
- [High] Incorrect teardown order in `rocket_job_fini` leads to Use-After-Free of the scheduler.
- [High] Incorrect teardown order in `rocket_core_fini` causes NULL pointer dereferences for concurrent jobs.
--

--- Patch [2]: [PATCH 2/4] accel/rocket: Fix the panic in rocket_iommu_domain_put when the domain is NULL ---
commit 30af1c047db9952f07dbaa552d86ca1a67f4701b
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

accel/rocket: Fix the panic in rocket_iommu_domain_put when the domain is NULL

This commit adds a null check in rocket_iommu_domain_put() to prevent
a panic when cleaning up a job that failed early in submission. The 
commit message states this is necessary because the domain is assigned
last in rocket_ioctl_submit_job().

> In rocket_ioctl_submit_job(), since the domain is assigned last, an
> error before that triggers rocket_job_put() -> rocket_job_cleanup()
> -> rocket_iommu_domain_put() with the domain still NULL,
> causing a panic in that function.

[Severity: Medium]
This isn't a bug introduced by this patch, but the commit message describes
a premise that doesn't match the code.

Looking at rocket_ioctl_submit_job(), the domain is actually assigned
immediately after allocation:

drivers/accel/rocket/rocket_job.c:rocket_ioctl_submit_job() {
...
	rjob = kzalloc_obj(*rjob);
	if (!rjob)
		return -ENOMEM;

	kref_init(&rjob->refcount);

	rjob->rdev = rdev;
	rjob->domain = rocket_iommu_domain_get(file_priv);
...
}

Because the domain is assigned before any operations that could fail, the
panic described in the commit message appears impossible in this specific
path.

Could the commit message be updated to clarify the exact failure path being
fixed?

> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> index 8bbbce594883e..77cd2ecce001e 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -65,7 +65,8 @@ rocket_iommu_domain_get(struct rocket_file_priv *rocket_priv)
>  void
>  rocket_iommu_domain_put(struct rocket_iommu_domain *domain)
>  {
> -	kref_put(&domain->kref, rocket_iommu_domain_destroy);
> +	if (domain)
> +		kref_put(&domain->kref, rocket_iommu_domain_destroy);
>  }
>  
>  static int

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential use-after-free
with GEM objects and PRIME.

If a user exports a GEM buffer via dma-buf and closes the DRM file
descriptor, rocket_postclose() unconditionally frees rocket_priv:

drivers/accel/rocket/rocket_drv.c:rocket_postclose() {
...
	rocket_iommu_domain_put(rocket_priv->domain);
	kfree(rocket_priv);
...
}

Later, when the dma-buf is finally closed, rocket_gem_bo_free() will
dereference the freed rocket_priv:

drivers/accel/rocket/rocket_gem.c:rocket_gem_bo_free() {
...
	struct rocket_file_priv *rocket_priv = bo->driver_priv;
...
	mutex_lock(&rocket_priv->mm_lock);
...
}

Does this code need to reference count rocket_priv or tie its lifetime to
the GEM objects rather than the DRM file?


[Severity: Critical]
This is a pre-existing issue, but there is a potential IOMMU mapping leak
in the BO creation path:

drivers/accel/rocket/rocket_gem.c:rocket_ioctl_create_bo() {
...
	ret = iommu_map_sgtable(rocket_priv->domain->domain,
				rkt_obj->mm.start,
				shmem_obj->sgt,
				IOMMU_READ | IOMMU_WRITE);
	if (ret < 0 || ret < args->size) {
		drm_err(dev, "failed to map buffer: size=%d request_size=%u\n",
			ret, args->size);
		ret = -ENOMEM;
		goto err_remove_node;
	}
...
}

If iommu_map_sgtable() returns a partial mapping where ret > 0 but
ret < args->size, the code jumps directly to err_remove_node, bypassing
err_unmap.

Won't the successfully mapped pages remain in the IOMMU while the GEM pages
are freed, allowing potential use-after-free corruption?


[Severity: High]
This is a pre-existing issue, but calling drm_sched_job_cleanup() on an
armed job might corrupt the scheduler timeline.

drivers/accel/rocket/rocket_job.c:rocket_job_push() {
...
		drm_sched_job_arm(&job->base);
...
		ret = rocket_acquire_object_fences(job->in_bos, job->in_bo_count, &job->base, false);
		if (ret)
			goto err_unlock;
...
}

If an error occurs after drm_sched_job_arm(), the code jumps to
out_cleanup_job and cleans it up:

drivers/accel/rocket/rocket_job.c:rocket_ioctl_submit_job() {
...
out_cleanup_job:
	if (ret)
		drm_sched_job_cleanup(&rjob->base);
...
}

According to DRM scheduler rules, once a job is armed, it must be pushed to
the entity.

Doesn't cleaning it up directly leave a missing sequence number in the
fence timeline?


[Severity: High]
This is a pre-existing issue, but there appears to be a reference leak
in the interrupt handler:

drivers/accel/rocket/rocket_job.c:rocket_job_handle_irq() {
...
		iommu_detach_group(NULL, iommu_group_get(core->dev));
...
}

The call to iommu_group_get() increments the reference count, but
iommu_detach_group() does not consume it.

Won't this leak a reference on every handled interrupt, eventually pinning
the group permanently?


[Severity: High]
This is a pre-existing issue, but the job submission loop ignores errors:

drivers/accel/rocket/rocket_job.c:rocket_ioctl_submit() {
...
	for (i = 0; i < args->job_count; i++)
		rocket_ioctl_submit_job(dev, file, &jobs[i]);

exit:
	kvfree(jobs);

	return ret;
}

If rocket_ioctl_submit_job() fails with an error like -ENOMEM, the loop
ignores the return value and continues. The ioctl will eventually return 0
for success.

Should this code check the return value and notify userspace of failures
to prevent hangs when waiting on fences that will never be signaled?


[Severity: High]
This is a pre-existing issue, but there is a risk of undefined behavior
when copying job buffers:

drivers/accel/rocket/rocket_job.c:rocket_job_push() {
...
	bos = kvmalloc_array(bo_count, sizeof(*bos), GFP_KERNEL);
	if (!bos)
		return -ENOMEM;
	memcpy(bos, job->in_bos, job->in_bo_count * sizeof(void *));
...
}

If a job is submitted with in_bo_count = 0 but out_bo_count > 0,
job->in_bos remains null. This leads to memcpy() being called with a null
source pointer, which is undefined behavior.

Should this code check if in_bo_count is greater than 0 before the copy?


[Severity: High]
This is a pre-existing issue, but the scheduler array appears to be leaked:

drivers/accel/rocket/rocket_job.c:rocket_job_open() {
...
	struct drm_gpu_scheduler **scheds = kmalloc_objs(*scheds,
							 rdev->num_cores);
...
	ret = drm_sched_entity_init(&rocket_priv->sched_entity,
				    DRM_SCHED_PRIORITY_NORMAL,
				    scheds,
				    rdev->num_cores, NULL);
...
}

If drm_sched_entity_init() fails, it returns early without freeing scheds.
Additionally, if rdev->num_cores is 1, drm_sched_entity_init() might not
take ownership of the array, meaning the allocation is orphaned and cannot
be freed in rocket_job_close().

Does this array need to be freed if it's not consumed or if an error
occurs?


[Severity: High]
This is a pre-existing issue, but there seems to be a teardown ordering
problem in the job destruction path:

drivers/accel/rocket/rocket_job.c:rocket_job_fini() {
	drm_sched_fini(&core->sched);

	cancel_work_sync(&core->reset.work);
...
}

drm_sched_fini() is called before the background reset work is canceled.

If the reset work runs concurrently, rocket_reset() might invoke
drm_sched_stop() and drm_sched_start() on a destroyed scheduler.

Should the background work be canceled before the scheduler is torn down?


[Severity: High]
This is a pre-existing issue, but I noticed another teardown ordering
problem in the core destruction path:

drivers/accel/rocket/rocket_core.c:rocket_core_fini() {
...
	iommu_group_put(core->iommu_group);
	core->iommu_group = NULL;
	rocket_job_fini(core);
}

Critical resources like core->iommu_group are torn down before stopping
the DRM scheduler in rocket_job_fini().

If the scheduler dispatches a job concurrently, won't rocket_job_run()
attempt to use a null iommu_group?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814022453.437-1-kernel@airkyi.com?part=2

Re: [PATCH 3/4] accel/rocket: Fix the extra iommu_group_get call in rocket_job_handle_irq

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:

New issues:
- [High] The patch introduces a guaranteed NULL pointer dereference in the interrupt handler during device teardown by caching the IOMMU group pointer.

Pre-existing issues:
- [High] The shared IRQ handler accesses hardware registers on a potentially suspended device and lacks `synchronize_irq()` in the runtime suspend path.
- [High] The device can be left permanently powered on with clocks enabled indefinitely after driver removal if an asynchronous suspend is cancelled.
- [High] Unsafe teardown ordering allows the timeout reset work to dereference a freed or NULL IOMMU group.
- [High] The driver does not verify that an IOMMU group was successfully retrieved during initialization, leading to crashes if no IOMMU is present.
- [High] The shared IRQ handler is managed by devres but the resources it depends on are destroyed manually in `rocket_job_fini`, leading to a Use-After-Free.
- [High] Hardware state (`OPERATION_ENABLE`) is modified outside `core->job_lock` in the threaded IRQ handler, creating a critical TOCTOU race condition.
- [Medium] Timeout reset logic uses `pm_runtime_put_noidle()`, which can prevent the device from entering a suspended state indefinitely.
- [Medium] Memory leak of the dynamically allocated `scheds` array in `rocket_job_open` on the error path.
--

--- Patch [3]: [PATCH 3/4] accel/rocket: Fix the extra iommu_group_get call in rocket_job_handle_irq ---
commit a3d6f36e63b3077307646b0545b3207616881be9
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

accel/rocket: Fix the extra iommu_group_get call in rocket_job_handle_irq

This commit removes an extra iommu_group_get() call in rocket_job_handle_irq()
that caused a reference count leak. It replaces the call with the cached
core->iommu_group pointer to ensure the IOMMU group can be released properly
during teardown.

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 3141f210fcd1b..0fc4d220c48ce 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -355,7 +355,7 @@ static void rocket_job_handle_irq(struct rocket_core *core)
>  				return;
>  			}
>  
> -			iommu_detach_group(NULL, iommu_group_get(core->dev));
> +			iommu_detach_group(NULL, core->iommu_group);

[Severity: High]
Does this introduce a NULL pointer dereference during driver removal?

During driver removal, rocket_core_fini() explicitly sets core->iommu_group
to NULL before the DRM scheduler and devres-managed IRQ handler are stopped.
If an in-flight job completes or a delayed IRQ thread runs in this window,
rocket_job_handle_irq() will pass the NULL core->iommu_group to
iommu_detach_group(), which dereferences it unconditionally and causes a
kernel panic.

>  			dma_fence_signal(core->in_flight_job->done_fence);
>  			pm_runtime_put_autosuspend(core->dev);
>  			core->in_flight_job = NULL;

[Severity: High]
This is a pre-existing issue, but does the shared IRQ handler access hardware
registers on a potentially suspended device?

If a shared interrupt is triggered by another device while the NPU is
runtime-suspended, the driver will directly read INTERRUPT_RAW_STATUS without
verifying if the device is active:

drivers/accel/rocket/rocket_job.c:rocket_job_irq_handler() {
    ...
    u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
    ...
}

Accessing unclocked hardware registers can cause a bus fault. Additionally,
rocket_device_runtime_suspend() disables clocks without calling
synchronize_irq():

drivers/accel/rocket/rocket_drv.c:rocket_device_runtime_suspend() {
    ...
    clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks),
                               rdev->cores[core].clks);
    ...
}

This allows an in-flight IRQ handler to race with hardware power-down.

[Severity: High]
This is a pre-existing issue, but can the device be left permanently powered
on with clocks enabled indefinitely after driver removal?

During device removal, rocket_core_fini() calls pm_runtime_disable() which
cancels any pending asynchronous suspend work:

drivers/accel/rocket/rocket_core.c:rocket_core_fini() {
    ...
    pm_runtime_dont_use_autosuspend(core->dev);
    pm_runtime_disable(core->dev);
    ...
}

Because a synchronous suspend is not used prior to disabling runtime PM,
the device can be left permanently in the RPM_ACTIVE state.

[Severity: High]
This is a pre-existing issue, but does unsafe teardown ordering allow the
timeout reset work to dereference a freed or NULL IOMMU group?

During driver removal, rocket_core_fini() frees the IOMMU group and sets it
to NULL:

drivers/accel/rocket/rocket_core.c:rocket_core_fini() {
    ...
    iommu_group_put(core->iommu_group);
    core->iommu_group = NULL;
    rocket_job_fini(core);
    ...
}

rocket_job_fini() then cancels the reset work. If a job timeout occurs
concurrently and the reset work executes before cancellation, rocket_reset()
will use the NULL or freed group:

drivers/accel/rocket/rocket_job.c:rocket_reset() {
    ...
    iommu_detach_group(NULL, core->iommu_group);
    ...
}

[Severity: High]
This is a pre-existing issue, but does the driver fail to verify if an IOMMU
group was successfully retrieved during initialization?

In rocket_core_init(), core->iommu_group is assigned without validation:

drivers/accel/rocket/rocket_core.c:rocket_core_init() {
    ...
    core->iommu_group = iommu_group_get(dev);
    ...
}

If the device lacks an IOMMU, this returns NULL. When the first job is later
submitted, rocket_job_run() passes this NULL group to iommu_attach_group(),
causing a kernel panic:

drivers/accel/rocket/rocket_job.c:rocket_job_run() {
    ...
    ret = iommu_attach_group(job->domain->domain, core->iommu_group);
    ...
}

[Severity: Medium]
This is a pre-existing issue, but does the timeout reset logic cause a power
leak by failing to queue an idle check?

In rocket_reset(), the PM usage count is dropped using
pm_runtime_put_noidle():

drivers/accel/rocket/rocket_job.c:rocket_reset() {
    ...
    pm_runtime_put_noidle(core->dev);
    ...
}

This decrements the usage counter but explicitly avoids queuing an idle check
or autosuspend request. If no further jobs are submitted, the device will
remain in a powered-on state indefinitely.

[Severity: Medium]
This is a pre-existing issue, but does this code leak the scheds array on
the error path?

The scheds array is dynamically allocated, but if drm_sched_entity_init()
returns an error, the function returns immediately without freeing it:

drivers/accel/rocket/rocket_job.c:rocket_job_open() {
    ...
    ret = drm_sched_entity_init(&rocket_priv->sched_entity, ...);
    if (WARN_ON(ret))
        return ret;
    ...
}

[Severity: High]
This is a pre-existing issue, but does manual cleanup race with the devres
managed IRQ handler, leading to a use-after-free?

The IRQ is managed via devres, but critical data structures are destroyed
manually in rocket_job_fini() before devres teardown:

drivers/accel/rocket/rocket_job.c:rocket_job_fini() {
    ...
    drm_sched_fini(&core->sched);
    cancel_work_sync(&core->reset.work);
    destroy_workqueue(core->reset.wq);
    ...
}

If an interrupt fires between the manual cleanup and devres teardown, the IRQ
handler thread will attempt to access destroyed scheduler state or workqueues.

[Severity: High]
This is a pre-existing issue, but is there a time-of-check to time-of-use
race condition when modifying hardware state outside of core->job_lock?

The IRQ thread disables the hardware by writing to OPERATION_ENABLE before
acquiring the lock:

drivers/accel/rocket/rocket_job.c:rocket_job_handle_irq() {
    ...
    rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
    rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);

    scoped_guard(mutex, &core->job_lock)
    ...
}

If a job times out, rocket_reset() can restart the scheduler and submit a new
job. If the delayed IRQ thread for the timed-out job executes subsequently,
it will write 0x0 to OPERATION_ENABLE unprotected, aborting the new job and
falsely signaling success.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814022453.437-1-kernel@airkyi.com?part=3