Re: [PATCH v41 3/7] firmware: imx: add driver for NXP EdgeLock Enclave

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Devres LIFO execution causes a Use-After-Free on `priv->se_if_cmd_lock` during unbind, and a memory leak on probe failure.
--

--- Patch [3]: [PATCH v41 3/7] firmware: imx: add driver for NXP EdgeLock Enclave ---
commit 49588215c50994b46eebe37520853007e8a9a892
Author: Pankaj Gupta <pankaj.gupta@nxp.com>

firmware: imx: add driver for NXP EdgeLock Enclave

This commit adds the MU-based communication interface for the secure
enclave on i.MX SoC(s). It allows the secure enclave firmware to communicate
with Linux over dedicated messaging unit (MU) based interfaces.

> diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/firmware/imx/se_ctrl.c

[ ... ]

> +static void se_if_probe_cleanup(void *plat_dev)
> +{
> +	struct platform_device *pdev = plat_dev;
> +	struct device *dev = &pdev->dev;
> +	struct se_if_priv *priv;
> +
> +	priv = dev_get_drvdata(dev);
> +	if (!priv)
> +		return;

[ ... ]

> +	kfree(priv);
> +}
> +
> +static int se_if_probe(struct platform_device *pdev)
> +{
> +	const struct se_soc_info *se_info;
> +	const struct se_if_node *if_node;
> +	struct se_fw_load_info *load_fw;
> +	struct device *dev = &pdev->dev;
> +	struct se_if_priv *priv;
> +	int ret;
> +
> +	if_node = device_get_match_data(dev);
> +	if (!if_node)
> +		return -EINVAL;
> +
> +	se_info = if_node->se_info;
> +
> +	priv = kzalloc_obj(*priv, GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = dev;
> +	priv->if_defs = &if_node->if_defs;
> +	dev_set_drvdata(dev, priv);
> +
> +	ret = devm_mutex_init(dev, &priv->se_if_cmd_lock);

[Severity: High]
Does this code introduce a memory leak on the probe error path?

Since priv is allocated with kzalloc_obj() (which is not devm-managed),
if devm_mutex_init() fails, the function returns immediately without
freeing priv.

> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to init mutex: priv se_if_cmd_lock.\n");
> +	spin_lock_init(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock);
> +	spin_lock_init(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock);
> +	atomic_set(&priv->fw_busy, 0);
> +	init_completion(&priv->waiting_rsp_clbk_hdl.done);
> +	init_completion(&priv->cmd_receiver_clbk_hdl.done);
> +
> +	ret = devm_add_action_or_reset(dev, se_if_probe_cleanup, pdev);

[Severity: High]
Could this devres action ordering lead to a use-after-free during device
unbind?

Because devres actions execute in reverse order of their registration
(LIFO), upon device unbind se_if_probe_cleanup() runs first and frees
priv. Subsequently, the devres action for devm_mutex_init() runs and
calls mutex_destroy(&priv->se_if_cmd_lock) on the already freed priv
pointer.

> +	if (ret)
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-imx-se-if-v41-0-29cf129e4d86@nxp.com?part=3