mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] iio: trigger: free the trigger ID after device_del()
@ 2026-11-06 17:47 Changyul Lee
  2026-11-07  1:31 ` Jonathan Cameron
  0 siblings, 1 reply; 2+ messages in thread
From: Changyul Lee @ 2026-11-06 17:47 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel, Changyul Lee

iio_trigger_unregister() returns the trigger ID to iio_trigger_ida
before device_del() removes the "trigger<N>" device. A trigger
registered in between can get the same ID, and its device_add() fails:

  sysfs: cannot create duplicate filename '/bus/iio/devices/trigger0'

For example, adding a sysfs trigger while an hrtimer trigger is being
removed through configfs, or the other way around, hits this because the
two paths do not share a lock.

Free the ID after device_del(), as the error path of
iio_trigger_register() already does, and drop the "Possible issue in
here" comment.

Fixes: 1637db44413e ("Staging: IIO: Trigger support added to core.")
Signed-off-by: Changyul Lee <lcy8047@gmail.com>
---
Found with a static analysis tool we are developing.

Reproduced in QEMU (x86_64) with two threads running for 20 seconds.
One writes to add_trigger and remove_trigger of iio_sysfs_trigger, the
other creates and removes an hrtimer trigger in configfs. Without this
patch, device_add() failed with the error above 160 to 1148 times per
run in three runs. With it, it never failed.

 drivers/iio/industrialio-trigger.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 76926fee0f84..206ede2c9b3c 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -105,9 +105,8 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)
 	scoped_guard(mutex, &iio_trigger_list_lock)
 		list_del(&trig_info->list);
 
-	ida_free(&iio_trigger_ida, trig_info->id);
-	/* Possible issue in here */
 	device_del(&trig_info->dev);
+	ida_free(&iio_trigger_ida, trig_info->id);
 }
 EXPORT_SYMBOL(iio_trigger_unregister);
 

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] iio: trigger: free the trigger ID after device_del()
  2026-11-06 17:47 [PATCH] iio: trigger: free the trigger ID after device_del() Changyul Lee
@ 2026-11-07  1:31 ` Jonathan Cameron
  0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2026-11-07  1:31 UTC (permalink / raw)
  To: Changyul Lee
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel

On Fri,  6 Nov 2026 12:47:37 -0500
Changyul Lee <lcy8047@gmail.com> wrote:

> iio_trigger_unregister() returns the trigger ID to iio_trigger_ida
> before device_del() removes the "trigger<N>" device. A trigger
> registered in between can get the same ID, and its device_add() fails:
> 
>   sysfs: cannot create duplicate filename '/bus/iio/devices/trigger0'
> 
> For example, adding a sysfs trigger while an hrtimer trigger is being
> removed through configfs, or the other way around, hits this because the
> two paths do not share a lock.
> 
> Free the ID after device_del(), as the error path of
> iio_trigger_register() already does, and drop the "Possible issue in
> here" comment.
> 
> Fixes: 1637db44413e ("Staging: IIO: Trigger support added to core.")
> Signed-off-by: Changyul Lee <lcy8047@gmail.com>
Nice find. Applied to the fixes-togreg branch of iio.git.

One passing comment below given you also deleted a very 'odd'
comment!


> ---
> Found with a static analysis tool we are developing.
> 
> Reproduced in QEMU (x86_64) with two threads running for 20 seconds.
> One writes to add_trigger and remove_trigger of iio_sysfs_trigger, the
> other creates and removes an hrtimer trigger in configfs. Without this
> patch, device_add() failed with the error above 160 to 1148 times per
> run in three runs. With it, it never failed.
> 
>  drivers/iio/industrialio-trigger.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 76926fee0f84..206ede2c9b3c 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -105,9 +105,8 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)
>  	scoped_guard(mutex, &iio_trigger_list_lock)
>  		list_del(&trig_info->list);
>  
> -	ida_free(&iio_trigger_ida, trig_info->id);
> -	/* Possible issue in here */
This made me wonder - what is that on about?  I checked
the history and all the code on either side of that comment has
changed since it was added.  So who knows! :)


>  	device_del(&trig_info->dev);
> +	ida_free(&iio_trigger_ida, trig_info->id);
>  }
>  EXPORT_SYMBOL(iio_trigger_unregister);
>  
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-07  1:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-06 17:47 [PATCH] iio: trigger: free the trigger ID after device_del() Changyul Lee
2026-11-07  1:31 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®