Re: [PATCH] mfd: twl6040: fix IRQ action memory leaks during teardown

[Lee Jones]

On Tue, 27 Oct 2026, Guangshuo Li wrote:

> twl6040_remove() currently removes the regmap IRQ chip before
> removing the MFD child devices. This destroys the IRQ mappings
> before the child drivers can release their interrupt handlers.
>
> The parent driver also registers READY and thermal interrupt
> handlers using devm_request_threaded_irq(). These handlers are
> normally released by device-managed cleanup after remove returns,
> when the corresponding IRQ descriptors have already been removed.
>
> Consequently, free_irq() cannot find the IRQ descriptors and the
> allocated struct irqaction objects are not freed. The associated
> IRQ threads also cannot be stopped through the normal cleanup path.
>
> The probe error paths have a similar issue, as they destroy the IRQ
> chip before the device-managed IRQ handlers are released.
>
> Remove the MFD child devices first, then explicitly free the
> parent's READY and thermal IRQ handlers using devm_free_irq()
> before removing the regmap IRQ chip.
>
> Also update the probe error paths to release successfully requested
> IRQs before destroying their mappings.
>
> This issue was found by our static analysis tool under development.
>
> Fixes: ab7edb149c75 ("mfd: twl6040: Convert to use regmap_irq")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
>  drivers/mfd/twl6040.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index 562a0f939f6e..7b05d21cd17d 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -745,7 +745,7 @@ static int twl6040_probe(struct i2c_client *client)
>  					"twl6040_irq_th", twl6040);
>  	if (ret) {
>  		dev_err(twl6040->dev, "Thermal IRQ request failed: %d\n", ret);
> -		goto readyirq_err;
> +		goto thirq_err;
>  	}
>
>  	/*
> @@ -791,10 +791,14 @@ static int twl6040_probe(struct i2c_client *client)
>  	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
>  			      NULL, 0, NULL);
>  	if (ret)
> -		goto readyirq_err;
> +		goto mfd_err;
>
>  	return 0;
>
> +mfd_err:
> +	devm_free_irq(twl6040->dev, twl6040->irq_th, twl6040);
> +thirq_err:
> +	devm_free_irq(twl6040->dev, twl6040->irq_ready, twl6040);
>  readyirq_err:
>  	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
>  gpio_err:
> @@ -809,9 +813,10 @@ static void twl6040_remove(struct i2c_client *client)
>  	if (twl6040->power_count)
>  		twl6040_power(twl6040, 0);
>
> -	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
> -
>  	mfd_remove_devices(&client->dev);
> +	devm_free_irq(twl6040->dev, twl6040->irq_th, twl6040);
> +	devm_free_irq(twl6040->dev, twl6040->irq_ready, twl6040);
> +	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);

Why not use more devm_*() handlers?

devm_regmap_add_irq_chip()

devm_free* eeks me out.  The point of devm_*() is that it's managed.

>  	regulator_bulk_disable(TWL6040_NUM_SUPPLIES, twl6040->supplies);
>  }
> -- 
> 2.43.0
>

-- 
Lee Jones