Re: [PATCH] remoteproc: imx_dsp: acquire runtime PM before adding carveouts

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] If `imx_dsp_rproc_add_carveout()` fails during its iteration, or if later stages like firmware parsing fail after a successful prepare, previously added carveouts and coredump segments are permanently leaked on the remoteproc lists.
- [High] Mailbox channels allocated during runtime resume are leaked if clock enable fails in `imx_dsp_runtime_resume()`.
--

commit 1aa0eaf53776f4fb1deb0e079f298b5ed6746813
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

remoteproc: imx_dsp: acquire runtime PM before adding carveouts

This commit fixes the prepare callback by acquiring the runtime-PM
reference before publishing DSP carveouts. This ensures that if the PM
resume fails, the transaction is not partially published.

> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
> index fd60c67ba8a9f..96d619daf3f5b 100644
> --- a/drivers/remoteproc/imx_dsp_rproc.c
> +++ b/drivers/remoteproc/imx_dsp_rproc.c
> @@ -726,14 +726,17 @@ static int imx_dsp_rproc_prepare(struct rproc *rproc)
>  	struct device *dev = rproc->dev.parent;
>  	int ret;
>  
> +	ret = pm_runtime_resume_and_get(dev);

[Severity: High]
This isn't a bug introduced by this patch, but looking at what happens during
this runtime resume, if clk_bulk_prepare_enable() fails in
imx_dsp_runtime_resume(), it returns directly:

drivers/remoteproc/imx_dsp_rproc.c:imx_dsp_runtime_resume() {
	...
	ret = clk_bulk_prepare_enable(DSP_RPROC_CLK_MAX, priv->clks);
	if (ret) {
		dev_err(dev, "failed on clk_bulk_prepare_enable\n");
		return ret;
	}
	...
}

Does this bypass imx_dsp_rproc_free_mbox() and permanently leak the exclusive
mailbox channels allocated just above it via imx_dsp_rproc_mbox_init()?

Subsequent runtime resume attempts might fail with -EBUSY when
mbox_request_channel_byname() is called again for the same channels, breaking
device functionality.

> +	if (ret < 0)
> +		return ret;
> +
>  	ret = imx_dsp_rproc_add_carveout(priv);
>  	if (ret) {
>  		dev_err(dev, "failed on imx_dsp_rproc_add_carveout\n");
> +		pm_runtime_put_sync(dev);
>  		return ret;

[Severity: High]
This is a pre-existing issue, but if imx_dsp_rproc_add_carveout() fails
midway through its loop (e.g., returning -ENOMEM or -EINVAL), it returns an
error without freeing earlier allocations.

Furthermore, if this prepare callback fully succeeds but rproc_parse_fw()
subsequently fails in the remoteproc core, the core jumps to the
unprepare_rproc label (which calls imx_dsp_rproc_unprepare) and completely
bypasses rproc_resource_cleanup().

Does this permanently leak the previously added rproc_mem_entry and
rproc_dump_segment objects on the remoteproc lists?

Repeated boot failures could continually append memory to these lists,
eventually causing an out-of-memory condition.

>  	}
>  
> -	pm_runtime_get_sync(dev);
> -
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830133310.6926-1-pengpeng@iscas.ac.cn?part=1