Re: [PATCH v4 4/4] soc: ti: Add TDA54 SCM configuration syscon support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] soc: ti: dangling syscon regmap on module unload in
  tda54_scm_conf_probe()
--

commit 7e23274fb2904b304448cf5fedb029681da31a63
Author: Moteen Shah <m-shah@ti.com>

soc: ti: Add TDA54 SCM configuration syscon support

This commit adds the TDA54 SCM configuration syscon driver, handling
proxy-based register access, SCM unlock/claim, and K3 SoC info
registration.

> diff --git a/drivers/soc/ti/tda54-mcu-sysconf.c b/drivers/soc/ti/tda54-mcu-sysconf.c
> new file mode 100644
> index 0000000000000..3ef82e737d095
> --- /dev/null
> +++ b/drivers/soc/ti/tda54-mcu-sysconf.c
[ ... ]
> +	regmap = devm_regmap_init(dev, NULL, scm, &tda54_scm_conf_regmap_cfg);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	writel(TDA54_SCM_CONF_LOCK_KICK0_UNLOCK_VAL, scm->base + scm->pdata->lock_offset);
> +	writel(TDA54_SCM_CONF_LOCK_KICK1_UNLOCK_VAL, scm->base + scm->pdata->lock_offset + 4);
> +
> +	/*
> +	 * There is no way to unregister a syscon regmap, so publish it only
> +	 * once nothing else in probe can fail.
> +	 */
> +	ret = of_syscon_register_regmap(dev->of_node, regmap);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register syscon regmap\n");
[ ... ]
> +static struct platform_driver tda54_scm_conf_driver = {
> +	.probe = tda54_scm_conf_probe,
> +	.driver = {
> +		.name = "k3-tda54-scm-conf",
> +		.of_match_table = tda54_scm_conf_of_match,
> +		.suppress_bind_attrs = true,
> +	},
> +};
> +
> +module_platform_driver(tda54_scm_conf_driver);

[Severity: High]
As reported by Andrew Davis on v1 [1], does unloading this module leave a
dangling regmap pointer on the global syscon_list?

TI_TDA54_MCU_SYSCONF is tristate (default m on ARCH_K3) and uses
module_platform_driver(), while .suppress_bind_attrs only disables the
sysfs bind/unbind files.  When the module is unloaded, devres frees regmap,
scm, and the MMIO mapping, and the module text containing
tda54_scm_conf_reg_read() and tda54_scm_conf_reg_write() is unmapped, while
syscon_list retains the freed regmap pointer.  Any syscon consumer that
has looked up or later looks up the regmap triggers a use-after-free and
calls into unmapped module memory, and reloading the module fails in
of_syscon_register_regmap() with -EEXIST.

Registering the driver with builtin_platform_driver() (or module_init()
without module_exit()) like other of_syscon_register_regmap() callers
allows building it as a loadable module while preventing module unload.

[1] https://lore.kernel.org/r/b5bc4c10-b364-4992-ae84-63e5b86771a3@ti.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112091027.1601122-1-m-shah@ti.com?part=4