Re: [PATCH v11 11/14] accel/rocket: select the per-core clock and reset counts from match data

[sashiko-bot@kernel.org]

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

New issues:
- [High] Missing NULL check for the return value of of_device_get_match_data(), leading to a guaranteed kernel panic on manual sysfs bind.

Pre-existing issues:
- [High] The driver tracks cores in an array using `num_cores` as both the array length and the probe index, causing resource leaks and array slot overwrites on out-of-order unbinds.
- [High] Devres memory leak of `struct rocket_device` and its children on probe deferral and manual unbind.
--

commit d68f29f9366dd1f885e0585ee67c1b358bda44b6
Author: Jiaxing Hu <gahing@gahingwoo.com>

accel/rocket: select the per-core clock and reset counts from match data

This patch introduces a soc_data structure to provide per-SoC clock and
reset counts from the device match data. This enables supporting the
RK3576, which has different counts compared to the RK3588.

> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index 5dd260bacbff6..b202d15816a39 100644
> --- a/drivers/accel/rocket/rocket_core.c
> +++ b/drivers/accel/rocket/rocket_core.c
> @@ -23,7 +23,7 @@ int rocket_core_init(struct rocket_core *core)
>  
>  	core->resets[0].id = "srst_a";
>  	core->resets[1].id = "srst_h";
> -	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
> +	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
>  						    core->resets);

[Severity: High]
Does this code crash with a NULL pointer dereference if a user with root
privileges force-binds the driver to a device using the sysfs bind interface?

The sysfs interface bypasses the device tree match, causing
of_device_get_match_data() (called in rocket_probe()) to return NULL, which
is then unconditionally dereferenced here as core->soc->num_resets.

[ ... ]

> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> index 8bbbce594883e..6e7dc91c5faac 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
>  
>  	rdev->cores[core].rdev = rdev;
>  	rdev->cores[core].dev = &pdev->dev;
> +	rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
>  	rdev->cores[core].index = core;
>  
>  	rdev->num_cores++;

[Severity: High]
This is a pre-existing issue, but does decrementing rdev->num_cores during
an out-of-order unbind in rocket_remove() leak resources and corrupt memory?

If a system has multiple cores (e.g., RK3588 has 3 RKNN cores) and a user
manually unbinds a lower-indexed core via sysfs before a higher-indexed
core, rdev->num_cores is decremented.

When the higher-indexed core is later unbound, find_core_for_dev() fails to
find it because the search space core < rdev->num_cores shrank, skipping
its teardown. A subsequent bind will then overwrite the active core's slot
at rdev->cores[rdev->num_cores].

[Severity: High]
This is also a pre-existing issue, but does probe deferral or manual unbind
permanently leak the struct rocket_device and its children?

During the first core probe, rocket_device_init() allocates rdev using
devm_drm_dev_alloc() tied to the global drm_dev.

If the core probe subsequently fails (e.g., -EPROBE_DEFER) or is manually
unbound, rocket_device_fini() unregisters the DRM device and rdev is set to
NULL. However, the devres allocations remain bound to the long-lived drm_dev
and persist indefinitely, leaking memory on every cycle.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831081956.84871-1-gahing@gahingwoo.com?part=11