drivers/iio/industrialio-trigger.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Once we called device_initialize() we have to call put_device()
on it. Refactor the code to make it in the right order.
Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v4:
- Move device_initialize() after all potential failure points (kvasprintf and
irq_alloc_descs) to avoid premature lifecycle management via kobject.
- Revert the use of put_device() in the error path and use kfree() instead,
as the device is not yet initialized.
- Align the implementation with the pattern used in iio_device_alloc(),
as suggested by Nuno Sá.
Changes in v3:
- Rewrite commit message to focus on standard design patterns.
- Remove the "Fixes" tag as the change is a cleanup/robustness improvement.
- Simplify the description of the fix as requested by the maintainer.
- Change title to better reflect the change (not a use-after-free).
Changes in v2:
- Remove the manual call to irq_free_descs() in the error path to avoid
a double free, as this is already handled by iio_trig_release().
- Clarify the error path and the potential for memory corruption in
the commit description.
- Remove the blank line in the tag block to comply with kernel script
requirements.
drivers/iio/industrialio-trigger.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 54416a384232..c995311bf699 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -561,10 +561,6 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
if (!trig)
return NULL;
- trig->dev.parent = parent;
- trig->dev.type = &iio_trig_type;
- trig->dev.bus = &iio_bus_type;
- device_initialize(&trig->dev);
INIT_WORK(&trig->reenable_work, iio_reenable_work_fn);
mutex_init(&trig->pool_lock);
@@ -592,6 +588,11 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
}
+ trig->dev.parent = parent;
+ trig->dev.type = &iio_trig_type;
+ trig->dev.bus = &iio_bus_type;
+ device_initialize(&trig->dev);
+
return trig;
free_descs:
--
2.43.0
On 2/21/26 1:32 AM, Salah Triki wrote: > Once we called device_initialize() we have to call put_device() > on it. Refactor the code to make it in the right order. This could be a bit more clear. Something like ... Move device_initialize() after all error paths in viio_trigger_alloc(). Previously, we should have been calling put_device() on any error path after device_initialize(), but failed to do so. Rather than adding put_device(), we can just move device_initialize() to avoid needing to unwind it on error. Additional trig->dev initialization is also moved with this just to keep the code organized. > Did we decide this doesn't need a Fixes: tag? > Signed-off-by: Salah Triki <salah.triki@gmail.com> > --- No need to send a new revision for this right away. If Jonathan is in the mood, he might fix it up for you. With the commit message improved: Reviewed-by: David Lechner <dlechner@baylibre.com>
On Sat, 21 Feb 2026 11:07:10 -0600
David Lechner <dlechner@baylibre.com> wrote:
> On 2/21/26 1:32 AM, Salah Triki wrote:
> > Once we called device_initialize() we have to call put_device()
> > on it. Refactor the code to make it in the right order.
>
> This could be a bit more clear. Something like ...
>
> Move device_initialize() after all error paths in viio_trigger_alloc().
> Previously, we should have been calling put_device() on any error path
> after device_initialize(), but failed to do so.
>
> Rather than adding put_device(), we can just move device_initialize()
> to avoid needing to unwind it on error.
>
> Additional trig->dev initialization is also moved with this just to
> keep the code organized.
>
>
I used this but edited to be in imperative:
Move device_initialize() after all error paths in viio_trigger_alloc().
Previously, put_device() should have been called on all error paths after
device_initialize(), but that was not done.
Rather than adding put_device(), move device_initialize() to avoid
needing to unwind it on error.
In addition move trig->dev initialization to just before device_initialize()
to related code together
> >
>
> Did we decide this doesn't need a Fixes: tag?
We never identified an actual problem. It's wrong from a best
practice point of view, but I'm not convinced there is a bug
today without the change. Hence not something we'll rush to backport
and no fixes tag.
>
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> > ---
> No need to send a new revision for this right away. If Jonathan is
> in the mood, he might fix it up for you.
Done ;)
Applied to the testing branch of iio.git
Thanks for the persistence Salah
(and thanks David for the suggestions!)
>
> With the commit message improved:
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
>
Thank you Jonathan, David, Nuno and Andy for the feedback and for applying the patch. I appreciate your help in refining the commit message and the guidance throughout the versions. Best regards, Salah
© 2016 - 2026 Red Hat, Inc.