Once `device_initialize()` is called, the lifecycle of the trigger must be
managed by the kobject reference counting. Currently, if `kvasprintf()`
fails, the code manually calls kfree() and `irq_free_descs()`.

Switching to `put_device()` ensures that the device's release callback
(`iio_trig_release()`) is properly invoked. This simplifies the error
path by centralizing the cleanup logic (including `irq_free_descs()`)
inside the release handler, following the standard driver model pattern.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
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 | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 54416a384232..7f53e2a5a101 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -576,7 +576,7 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
 
 	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
 	if (trig->name == NULL)
-		goto free_descs;
+		goto free_trig;
 
 	INIT_LIST_HEAD(&trig->list);
 
@@ -594,10 +594,8 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
 
 	return trig;
 
-free_descs:
-	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
 free_trig:
-	kfree(trig);
+	put_device(&trig->dev);
 	return NULL;
 }
 
-- 
2.43.0

On Sun, Feb 15, 2026 at 11:23:47PM +0100, Salah Triki wrote:
> Once `device_initialize()` is called, the lifecycle of the trigger must be
> managed by the kobject reference counting. Currently, if `kvasprintf()`
> fails, the code manually calls kfree() and `irq_free_descs()`.
> 
> Switching to `put_device()` ensures that the device's release callback
> (`iio_trig_release()`) is properly invoked. This simplifies the error
> path by centralizing the cleanup logic (including `irq_free_descs()`)
> inside the release handler, following the standard driver model pattern.

How did you test this, please?

...

> struct iio_trigger *viio_trigger_alloc(struct device *parent,

>  	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
>  	if (trig->name == NULL)
> -		goto free_descs;
> +		goto free_trig;
>  
>  	INIT_LIST_HEAD(&trig->list);

>  	return trig;
>  
> -free_descs:
> -	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
>  free_trig:
> -	kfree(trig);
> +	put_device(&trig->dev);

Now, in iio_trig_release() you will call a bunch of code with
trig->subirq_base != 0.

Please, test your changes before submitting.

>  	return NULL;
>  }

-- 
With Best Regards,
Andy Shevchenko

Hi Andy,

You are absolutely right. My previous version (v3) was logically flawed as
it could trigger the release callback before the necessary fields were
initialized, leading to an unsafe irq_free_descs() call.

Since I don't have the physical hardware to perform runtime injection
tests,I relied on manual code path analysis and clearly failed to account
for the side effects of put_device().

I'm sending a v4 which takes the safer approach: moving
device_initialize() after all potential failure points. This way, we can
safely use kfree() and irq_free_descs() in the error path without
involving the device lifecycle prematurely.

Thank you for the catch.

On Mon, 2026-02-16 at 09:45 +0100, Salah Triki wrote:
> Hi Andy,
> 
> You are absolutely right. My previous version (v3) was logically flawed as
> it could trigger the release callback before the necessary fields were
> initialized, leading to an unsafe irq_free_descs() call.
> 
> Since I don't have the physical hardware to perform runtime injection
> tests,I relied on manual code path analysis and clearly failed to account
> for the side effects of put_device().
> 
> I'm sending a v4 which takes the safer approach: moving
> device_initialize() after all potential failure points. This way, we can
> safely use kfree() and irq_free_descs() in the error path without
> involving the device lifecycle prematurely.
> 
> Thank you for the catch.

Please just use the same approach Andy did for iio_device_alloc(). I posted links (in one of your
versions) to that and for the discussion we already had on this same function some time ago.

- Nuno Sá

On Tue, Feb 17, 2026 at 01:51:08PM +0000, Nuno Sá wrote:
> On Mon, 2026-02-16 at 09:45 +0100, Salah Triki wrote:
> > 
> > You are absolutely right. My previous version (v3) was logically flawed as
> > it could trigger the release callback before the necessary fields were
> > initialized, leading to an unsafe irq_free_descs() call.
> > 
> > Since I don't have the physical hardware to perform runtime injection
> > tests,I relied on manual code path analysis and clearly failed to account
> > for the side effects of put_device().
> > 
> > I'm sending a v4 which takes the safer approach: moving
> > device_initialize() after all potential failure points. This way, we can
> > safely use kfree() and irq_free_descs() in the error path without
> > involving the device lifecycle prematurely.
> > 
> > Thank you for the catch.
> 
> Please just use the same approach Andy did for iio_device_alloc(). I posted
> links (in one of your versions) to that and for the discussion we already had
> on this same function some time ago.

I believe this is the approach mentioned above, id est move device_initialize()
further in the code. But I might misread something...

-- 
With Best Regards,
Andy Shevchenko

On Tue, 2026-02-17 at 17:08 +0200, Andy Shevchenko wrote:
> On Tue, Feb 17, 2026 at 01:51:08PM +0000, Nuno Sá wrote:
> > On Mon, 2026-02-16 at 09:45 +0100, Salah Triki wrote:
> > > 
> > > You are absolutely right. My previous version (v3) was logically flawed as
> > > it could trigger the release callback before the necessary fields were
> > > initialized, leading to an unsafe irq_free_descs() call.
> > > 
> > > Since I don't have the physical hardware to perform runtime injection
> > > tests,I relied on manual code path analysis and clearly failed to account
> > > for the side effects of put_device().
> > > 
> > > I'm sending a v4 which takes the safer approach: moving
> > > device_initialize() after all potential failure points. This way, we can
> > > safely use kfree() and irq_free_descs() in the error path without
> > > involving the device lifecycle prematurely.
> > > 
> > > Thank you for the catch.
> > 
> > Please just use the same approach Andy did for iio_device_alloc(). I posted
> > links (in one of your versions) to that and for the discussion we already had
> > on this same function some time ago.
> 
> I believe this is the approach mentioned above, id est move device_initialize()
> further in the code. But I might misread something...

Me too. But if that was the case, I would not expect any put_device() call. Ah, but I just read
Salah's reply. It will be done in v4.

- Nuno Sá