Re: [PATCH] pmdomain: imx: Propagate SCU provider registration failures

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Silent ignoring of memory allocation failures (-ENOMEM) during PM domain initialization.
- [Medium] imx_sc_get_pd_power() returns garbage union data on RPC failure.
- [Medium] Uninitialized kernel stack memory is sent to the SCU firmware via IPC messages, leaking 1-2 bytes of kernel stack memory per call.
--

commit 0f88257985e0a31b74141d36afa67a7454eaca5c
Author: Pengpeng Hou <hppiscas@163.com>

pmdomain: imx: Propagate SCU provider registration failures

imx_scu_init_pm_domains() ignores of_genpd_add_provider_onecell() and
returns success even when none of the initialized domains are published to
DT consumers.

Return the provider error and remove the initialized domains in reverse
order before devm releases their backing storage on probe failure.

> diff --git a/drivers/pmdomain/imx/scu-pd.c b/drivers/pmdomain/imx/scu-pd.c
> index e3f9e741daf76..05d4364f34be9 100644
> --- a/drivers/pmdomain/imx/scu-pd.c
> +++ b/drivers/pmdomain/imx/scu-pd.c
[ ... ]
> @@ -503,7 +503,13 @@ static int imx_scu_init_pm_domains(struct device *dev,
>  	pd_data->num_domains = count;
>  	pd_data->xlate = imx_scu_pd_xlate;
>  
> -	of_genpd_add_provider_onecell(dev->of_node, pd_data);
> +	ret = of_genpd_add_provider_onecell(dev->of_node, pd_data);
> +	if (ret) {
> +		while (count)
> +			pm_genpd_remove(domains[--count]);
> +
> +		return ret;
> +	}
>  
>  	return 0;
>  }

While reviewing this patch, I noticed a few pre-existing issues in this file.

drivers/pmdomain/imx/scu-pd.c:imx_sc_get_pd_power() {
    ...
	struct imx_sc_msg_req_get_resource_power_mode msg;
    ...
	msg.data.req.resource = rsrc;

	ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
    ...
}

[Severity: Medium]
This is a pre-existing issue, but does this leak uninitialized kernel stack
memory to the firmware? The msg structure is allocated on the stack and only
partially initialized. Due to padding, some bytes remain uninitialized before
the full structure is transmitted via imx_scu_call_rpc(). A similar situation
occurs with struct imx_sc_msg_req_set_resource_power_mode msg in
imx_sc_pd_power().


drivers/pmdomain/imx/scu-pd.c:imx_sc_get_pd_power() {
    ...
	ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
	if (ret)
		dev_err(dev, "failed to get power resource %d mode, ret %d\n",
			rsrc, ret);

	return msg.data.resp.mode;
}

[Severity: Medium]
This is a pre-existing issue, but when imx_scu_call_rpc() fails, does this
return uninitialized or overlapped union data instead of propagating the error
code? The returned garbage data might randomly match IMX_SC_PM_PW_MODE_ON,
leading to incorrect power state initialization.


drivers/pmdomain/imx/scu-pd.c:imx_scu_init_pm_domains() {
    ...
	for (i = 0; i < pd_soc->num_ranges; i++) {
		for (j = 0; j < pd_ranges[i].num; j++) {
			sc_pd = imx_scu_add_pm_domain(dev, j, &pd_ranges[i]);
			if (IS_ERR_OR_NULL(sc_pd))
				continue;
    ...
}

[Severity: High]
This is a pre-existing issue, but are memory allocation failures silently
ignored here? If devm_kzalloc() inside imx_scu_add_pm_domain() returns
ERR_PTR(-ENOMEM), catching this with IS_ERR_OR_NULL(sc_pd) and continuing
the loop fails to propagate the resource management error, which could leave
dependent devices broken.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906033806.84671-1-hppiscas@163.com?part=1