Re: [PATCH v4] dt-bindings: mfd: Convert TI TWL6040 to DT schema

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] IRQ domain destroyed before devm-managed threaded IRQs are freed and before child devices are removed, causing Use-After-Free.
- [High] The child driver twl6040-vibra allocates regulator devres actions on the parent device using a pointer allocated on the child, leading to a Use-After-Free.
--

commit 0404bd7a83f494396c387e5f75dbf3991d0ac035
Author: Eduard Bostina <egbostina@gmail.com>

dt-bindings: mfd: Convert TI TWL6040 to DT schema

Convert the Texas Instruments TWL6040 audio codecs family bindings to DT
schema. It updates naming and optional parameters for modern platforms while
maintaining hardware requirements for the vibra child node.

Link to v3: https://lore.kernel.org/all/20260817100207.2970303-1-egbostina@gmail.com/
Link to v2: https://lore.kernel.org/all/20260816092847.2522994-1-egbostina@gmail.com/
Link to v1: https://lore.kernel.org/all/20260815083451.2147129-1-egbostina@gmail.com/

> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
> new file mode 100644
> index 0000000000000..5efcd79b527c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl6040.yaml
[ ... ]

While reviewing this dt-bindings conversion, I noticed a few pre-existing
issues in the related C drivers for this hardware family.

In drivers/mfd/twl6040.c:

> static void twl6040_remove(struct i2c_client *client)
> {
> 	struct twl6040 *twl6040 = i2c_get_clientdata(client);
> 
> 	if (twl6040->power_count)
> 		twl6040_power(twl6040, 0);
> 
> 	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
> 
> 	mfd_remove_devices(&client->dev);

[Severity: High]
This isn't a bug introduced by this patch, but does this sequence lead to a
use-after-free during teardown?

It looks like twl6040_remove() destroys the IRQ domain via
regmap_del_irq_chip() before the child devices are removed via
mfd_remove_devices(). When the children unbind and attempt to free their
IRQs, won't they access the already-destroyed IRQ domain and descriptors?

A similar sequence seems to occur in the twl6040_probe() error path:

drivers/mfd/twl6040.c:twl6040_probe() {
[ ... ]
> 	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
> 			      NULL, 0, NULL);
> 	if (ret)
> 		goto readyirq_err;
> 
> 	return 0;
> 
> readyirq_err:
> 	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);

[Severity: High]
This is a pre-existing issue, but does this error path similarly destroy the
IRQ domain before the devm-managed threaded IRQs are automatically freed by
devres? This appears to cause a use-after-free when probe fails.


In drivers/input/misc/twl6040-vibra.c:

> 	info->supplies[0].supply = "vddvibl";
> 	info->supplies[1].supply = "vddvibr";
> 	/*
> 	 * When booted with Device tree the regulators are attached to the
> 	 * parent device (twl6040 MFD core)
> 	 */
> 	error = devm_regulator_bulk_get(twl6040_core_dev,
> 					ARRAY_SIZE(info->supplies),
> 					info->supplies);

[Severity: High]
This is a pre-existing issue, but does this devres attachment cause a
use-after-free if the parent device unbinds after the child device?

The info structure is allocated via devm_kzalloc() on the child device. By
passing info->supplies to devm_regulator_bulk_get() for the parent device
(twl6040_core_dev), the parent's devres context stores this child-managed
pointer. If the child unbinds, info is freed, and when the parent later
unbinds, won't its devres call regulator_bulk_free() on the now-dangling
info->supplies pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905215234.454266-1-egbostina@gmail.com?part=1