Re: [PATCH v41 4/7] firmware: imx: device context dedicated to priv

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing Use-After-Free of `priv->se_if_cmd_lock` due to incorrect devres action ordering.
--

--- Patch [4]: [PATCH v41 4/7] firmware: imx: device context dedicated to priv ---
commit d02e4a5df59f7ab91519903b9c6543123c1e4afc
Author: Pankaj Gupta <pankaj.gupta@nxp.com>

firmware: imx: device context dedicated to priv
    
Add priv_dev_ctx to prepare enabling misc-device context based send-receive
path, to communicate with FW.

> diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
> --- a/drivers/firmware/imx/se_ctrl.c
> +++ b/drivers/firmware/imx/se_ctrl.c
[ ... ]
> @@ -437,6 +465,11 @@ static int se_if_probe(struct platform_device *pdev)
>  		load_fw->imem_mgmt = true;
>  	}
>  
> +	ret = init_misc_device_context(priv, 0, &priv->priv_dev_ctx);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to create device contexts.\n");

[Severity: High]
This is a pre-existing issue, but does adding this new failure path make it
easier to trigger a use-after-free during devres unwind?

Looking at se_if_probe(), earlier in the function devm_mutex_init() is called
for priv->se_if_cmd_lock:

se_if_probe()
    ret = devm_mutex_init(dev, &priv->se_if_cmd_lock);
    ...
    ret = devm_add_action_or_reset(dev, se_if_probe_cleanup, pdev);

Because devres actions are executed in reverse order of addition, if this new
init_misc_device_context() fails, the unwind will first call
se_if_probe_cleanup(), which frees priv:

se_if_probe_cleanup()
    ...
    kfree(priv);

Then devres will run the mutex destroy action for priv->se_if_cmd_lock,
resulting in a use-after-free on the freed priv structure when
CONFIG_DEBUG_MUTEXES is enabled.

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