[PATCH v3] iio: trigger: use put_device() in viio_trigger_alloc() error path

[PATCH v3] iio: trigger: use put_device() in viio_trigger_alloc() error path

[Salah Triki]

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

Re: [PATCH v3] iio: trigger: use put_device() in viio_trigger_alloc() error path

[Andy Shevchenko]

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

Re: [PATCH v3] iio: trigger: use put_device() in viio_trigger_alloc() error path

[Salah Triki]

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.

Re: [PATCH v3] iio: trigger: use put_device() in viio_trigger_alloc() error path

[Nuno Sá]

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á

Re: [PATCH v3] iio: trigger: use put_device() in viio_trigger_alloc() error path

[Andy Shevchenko]

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

Re: [PATCH v3] iio: trigger: use put_device() in viio_trigger_alloc() error path

[Nuno Sá]

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á