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

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

[Wentao Liang]

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@iscas.ac.cn>
---
 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);
-- 
2.34.1

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@iscas.ac.cn> 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@iscas.ac.cn>

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);

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

[Wentao Liang]

Hi Jonathan,

I am sorry for the very late reply and for the extra review work caused by
sending many similar patches without first waiting for feedback.

I understand that mixing devm-managed resources with manual trigger
cleanup is not an acceptable solution here. A broader solution needs to
account for runtime trigger changes from userspace and distinguish the
references that managed cleanup should release.

Please disregard this patch; I will not resend this approach as v2.

Thank you for explaining the lifetime issues and how this work should be
submitted. This has been an important lesson for me. I will personally
review and respond to the remaining feedback one item at a time, and I
will keep future submissions within my capacity to follow up promptly.

For transparency, we use static analysis tools to identify candidates,
then DeepSeek and GPT-6.1 Sol to assist with auditing and preparing fixes.
I am responsible for verifying the findings and changes before submission.

Best regards,
Wentao

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

[Jonathan Cameron]

On Fri, 06 Nov 2026 11:38:03 -0000
Wentao Liang <vulab@iscas.ac.cn> wrote:

> Hi Jonathan,
> 
> I am sorry for the very late reply and for the extra review work caused by
> sending many similar patches without first waiting for feedback.
> 
> I understand that mixing devm-managed resources with manual trigger
> cleanup is not an acceptable solution here. A broader solution needs to
> account for runtime trigger changes from userspace and distinguish the
> references that managed cleanup should release.
> 
> Please disregard this patch; I will not resend this approach as v2.

I have a plan for a more general solution to these trigger references.
Hopefully will get time tomorrow to try it out.

Key I think is that we need to stop setting iio_dev->trig directly
but instead just set a iio_dev->initial_trigger and then
the get() can be done at iio_dev registration time - lining up with
code that drops the reference in the remove paths.

Anyhow, you got me to finally spend a bit of time thinking about
the lifetimes of this stuff, so thanks for the push!

Jonathan

> 
> Thank you for explaining the lifetime issues and how this work should be
> submitted. This has been an important lesson for me. I will personally
> review and respond to the remaining feedback one item at a time, and I
> will keep future submissions within my capacity to follow up promptly.
> 
> For transparency, we use static analysis tools to identify candidates,
> then DeepSeek and GPT-6.1 Sol to assist with auditing and preparing fixes.
> I am responsible for verifying the findings and changes before submission.
> 
> Best regards,
> Wentao
>