[PATCH v4] iio: core: Clean up device correctly on viio_trigger_alloc() failure

[PATCH v4] iio: core: Clean up device correctly on viio_trigger_alloc() failure

[Salah Triki]

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

Re: [PATCH v4] iio: core: Clean up device correctly on viio_trigger_alloc() failure

[David Lechner]

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>

Re: [PATCH v4] iio: core: Clean up device correctly on viio_trigger_alloc() failure

[Jonathan Cameron]

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

Re: [PATCH v4] iio: core: Clean up device correctly on viio_trigger_alloc() failure

[Salah Triki]

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