Re: [PATCH v2 02/18] arm64: dts: ti: k3-j721e-main: Add multiple channels for CSI2RX DMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver unregisters contexts before unregistering the async and PM notifiers in remove(), leading to Use-After-Free and data corruption.
- [High] The driver permanently leaks all requested DMA channels during probe failures and device removal because DMA channel release incorrectly relies on the PM runtime status.
- [High] ti_csi2rx_runtime_resume() leaks successfully requested DMA channels when failing to initialize a subsequent context.
- [High] Missing pm_runtime_disable() on probe error paths after pm_runtime_enable() was called.
--

--- Patch [2]: [PATCH v2 02/18] arm64: dts: ti: k3-j721e-main: Add multiple channels for CSI2RX DMA ---
commit 9792c8b4a6bb16010ee3d8cdeef37baa49738183
Author: Vaishnav Achath <vaishnav.a@ti.com>

arm64: dts: ti: k3-j721e-main: Add multiple channels for CSI2RX DMA

J721E CSI2RX SHIM layer can support up to 32 DMA channel contexts. Add
additional DMA channels to enable multistream support for CSI2RX.

> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> index 5a8414fc57519..b1988437f52dc 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> @@ -600,8 +600,14 @@ ti_csi2rx0: ticsi2rx@4500000 {
>  		ranges;
>  		#address-cells = <2>;
>  		#size-cells = <2>;
> -		dmas = <&main_udmap 0x4940>;
> -		dma-names = "rx0";
> +		dmas = <&main_udmap 0x4940>, <&main_udmap 0x4941>, <&main_udmap 0x4942>,
> +		       <&main_udmap 0x4943>, <&main_udmap 0x4944>, <&main_udmap 0x4945>,
> +		       <&main_udmap 0x4946>, <&main_udmap 0x4947>, <&main_udmap 0x4948>,
> +		       <&main_udmap 0x4949>, <&main_udmap 0x494a>, <&main_udmap 0x494b>,
> +		       <&main_udmap 0x494c>, <&main_udmap 0x494d>, <&main_udmap 0x494e>,
> +		       <&main_udmap 0x494f>;
> +		dma-names = "rx0", "rx1", "rx2", "rx3", "rx4", "rx5", "rx6", "rx7",
> +			    "rx8", "rx9", "rx10", "rx11", "rx12", "rx13", "rx14", "rx15";

[Severity: High]
This is a pre-existing issue, but does increasing the number of DMA channels
here make a leak in the driver's resume path more likely to occur?

Looking at drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c,
ti_csi2rx_runtime_resume() loops over all contexts:

	for (unsigned int i = 0; i < csi->num_ctx; i++) {
		ret = ti_csi2rx_init_dma(&csi->ctx[i]);
		if (ret)
			return ret;
	}

If initialization fails for a subsequent context, does it return immediately
without releasing the successfully requested channels from prior iterations?

[Severity: High]
This isn't a bug introduced by this patch, but do the DMA channels get
permanently leaked on probe failures or driver unbind?

In drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c:
ti_csi2rx_cleanup_ctx() checks the runtime status:

	if (!pm_runtime_status_suspended(ctx->csi->dev))
		dma_release_channel(ctx->dma.chan);

During probe, DMA channels are requested before PM runtime is activated.
If probe fails, pm_runtime_status_suspended() returns true, causing the
cleanup to skip releasing the channels.

Similarly, in ti_csi2rx_remove():

	if (!pm_runtime_status_suspended(&pdev->dev))
		pm_runtime_set_suspended(&pdev->dev);

Since the driver forcefully sets the suspended state without invoking the
runtime suspend callback, will ti_csi2rx_cleanup_ctx() again skip releasing
the channels?

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk during
driver unbind?

In drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c:ti_csi2rx_remove(),
the contexts are cleaned up before the notifiers are unregistered:

	for (unsigned int i = 0; i < csi->num_ctx; i++)
		ti_csi2rx_cleanup_ctx(&csi->ctx[i]);

	ti_csi2rx_cleanup_notifier(csi);
	unregister_pm_notifier(&csi->pm_notifier);

If a system suspend or async notifier completion happens concurrently during
unbind, could it trigger callbacks that access the already destroyed
ctx->mutex or unregistered video_device?

[Severity: High]
This isn't a bug introduced by this patch, but does the probe error path
leave the device with an unbalanced PM runtime enable count?

In drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c:ti_csi2rx_probe(),
PM runtime is enabled:

	pm_runtime_enable(csi->dev);

	ret = ti_csi2rx_notifier_register(csi);
	if (ret)
		goto err_ctx;

Should the error paths (err_notifier, err_ctx) call pm_runtime_disable() to
clean up the enable count properly?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713113423.2310443-1-y-abhilashchandra@ti.com?part=2