Re: [PATCH v2 3/3] soc: ti: Add TDA54 SCM configuration syscon support

[sashiko-bot@kernel.org]

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

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

soc: ti: Add TDA54 SCM configuration syscon support

This commit adds the TDA54 SCM configuration driver to provide regmap/syscon
access to the SCM register space with proxy-based register claim/unlock
handling, and registers TDA54 SoC identification info via k3-socinfo.

> 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
[ ... ]
> +static int tda54_scm_conf_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct tda54_scm_conf *scm;
> +	struct regmap *regmap;
> +	void __iomem *base;
> +	int ret;
> +
> +	scm = devm_kzalloc(dev, sizeof(*scm), GFP_KERNEL);
> +	if (!scm)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
[ ... ]
> +	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]
Can unloading this module cause a use-after-free when consumers access the
syscon regmap?

TI_TDA54_MCU_SYSCONF is a tristate Kconfig option (defaulting to m on
ARCH_K3), and module_platform_driver(tda54_scm_conf_driver) generates a
module_exit callback that invokes platform_driver_unregister(). While
.suppress_bind_attrs = true hides the sysfs bind/unbind attributes, it does
not prevent unloading the module via rmmod or modprobe -r.

In tda54_scm_conf_probe(), scm, base, and regmap are allocated with devres
(devm_kzalloc(), devm_platform_ioremap_resource(), and devm_regmap_init()),
and regmap is then published globally via of_syscon_register_regmap():

drivers/mfd/syscon.c:of_syscon_register_regmap() {
    ...
	list_for_each_entry(entry, &syscon_list, list)
		if (entry->np == np) {
			ret = -EEXIST;
			goto err_unlock;
		}

	syscon->regmap = regmap;
	syscon->np = np;

	/* register the regmap in syscon list */
	list_add_tail(&syscon->list, &syscon_list);
    ...
}

Because of_syscon_register_regmap() permanently stores the raw regmap pointer
on syscon_list without taking a module reference or providing an
unregistration path, unloading the module detaches the device and runs
devres_release_all(), freeing regmap, unmapping base, freeing scm, and
unmapping the module text (tda54_scm_conf_reg_read() and
tda54_scm_conf_reg_write()) while syscon_list retains the dangling regmap
pointer.

Any subsequent or existing consumer using syscon_node_to_regmap() or
syscon_regmap_lookup_by_phandle() will dereference the freed regmap and call
into unmapped module text, and reloading the module will fail with -EEXIST in
of_syscon_register_regmap().

Should TI_TDA54_MCU_SYSCONF be bool (using builtin_platform_driver()), or if
it remains a module, avoid registering a module_exit callback so it cannot be
unloaded once registered with syscon?

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