Re: [PATCH] iio: accel: bma220: Fix trigger reference leak in bma220_common_probe()

[Jonathan Cameron]

On Wed, 16 Sep 2026 16:08:23 +0000
Wentao Liang <vulab@xxxxxxxxxxx> wrote:

> indio_dev->trig holds an extra reference taken by iio_trigger_get(). It is
> only dropped by iio_dev_release() when the device has a triggered mode set,
> and indio_dev->modes is still INDIO_DIRECT_MODE until
> devm_iio_triggered_buffer_setup() runs. Both early error paths return
> before that point, so the reference is leaked.
> 
> Release the trigger reference on the IRQ request and the deinit action
> failure paths.
> 
> Fixes: b7e17ca10793 ("iio: accel: bma220: add interrupt trigger")
> Signed-off-by: Wentao Liang <vulab@xxxxxxxxxxx>

When doing a fix like this send one only and then wait before sending
more.  Also look at the discussions that have already gone on around
this topic on the list.  Sashiko mentions this ever time and there
are numerous replies to why it is a 'fix later' issue.

This is a lot more complex than simply doing a trigger put not because
it is technically wrong but because it leaves us with considerably less
readable and maintainable code.  The solution needs to be proper
devm_ registration and cleanup. However that is complex to actually do.

We have known it was an issue for some time and no fix has been forthcoming
yet.  There are userspace paths to put this trigger at runtime and we need
to distinguish between those registered via the devm path and those that
are not. Only the driver registered ones should be cleaned up.

Mixing and matching between devm calls and manual cleanup is not a solution
to this that I am going to consider.

Jonathan


> ---
>  drivers/iio/accel/bma220_core.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index f32d875b994e..4820d99e214a 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -544,14 +544,19 @@ int bma220_common_probe(struct device *dev, struct regmap *regmap, int irq)
>  		ret = devm_request_threaded_irq(dev, irq, NULL,
>  						&bma220_irq_handler, IRQF_ONESHOT,
>  						indio_dev->name, indio_dev);
> -		if (ret)
> +		if (ret) {
> +			iio_trigger_put(indio_dev->trig);
>  			return dev_err_probe(dev, ret,
>  					     "request irq %d failed\n", irq);
> +		}
>  	}
>  
>  	ret = devm_add_action_or_reset(dev, bma220_deinit, data);
> -	if (ret)
> +	if (ret) {
> +		if (indio_dev->trig)
> +			iio_trigger_put(indio_dev->trig);
>  		return ret;
> +	}
>  
>  	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
>  					      bma220_trigger_handler, NULL);