Re: [PATCH] iio: adc: at91_adc: fix NULL pointer deref in trigger removal

[Jonathan Cameron <jic23@kernel.org>]

On Fri,  6 Nov 2026 22:08:09 +0200
Abdelnasser Hussein <abdelnasserhussein11@gmail.com> wrote:

> If 'atmel,adc-use-external-triggers' is not set in the device tree,
> at91_adc_trigger_init() leaves the external trigger slots in st->trig
> as NULL. at91_adc_trigger_remove(), also used by the probe error path,
> then calls iio_trigger_unregister() and iio_trigger_free() on them
> unconditionally, causing a NULL pointer dereference.
> 
> Skip NULL triggers when unregistering and freeing.
Good catch.

There doesn't seem to be anything stopping us just moving the
registration and tear down handling of these over to devm managed
calls.  That way we'd not need to clean them up explicitly at all.

That would mean calling devm_input_allocate_device() for the input
path and devm_iio_triggered_buffer_setup(), devm_iio_trigger_alloc()
devm_iio_trigger_register() and devm_iio_device_register() for iio instead
of the non devm cases.  The error handling and remove path would
go away + the NULL dereference would be solved because no cleanup
would be registered for the the case where the trigger was never
there.  I'd consider carefully if the various helpers make sense
after this change.  I suspect you'd be better off with these
calls inline in at91_adc_probe()  for the IIO ones at least
as less done in those than the input related one.

Whilst it will be more lines of code changed, it would both fix
the bug and improve the code.  So that would definitely be my
preferred solution.

Thanks,

Jonathan


> 
> Fixes: 0e589d5fb317 ("ARM: AT91: IIO: Add AT91 ADC driver.")
> Signed-off-by: Abdelnasser Hussein <abdelnasserhussein11@gmail.com>
> ---
>  drivers/iio/adc/at91_adc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 0ac74ad92fca..c7fc06214c05 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -660,6 +660,8 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
>  
>  error_trigger:
>  	for (i--; i >= 0; i--) {
> +		if (!st->trig[i])
> +			continue;
>  		iio_trigger_unregister(st->trig[i]);
>  		iio_trigger_free(st->trig[i]);
>  	}
> @@ -673,6 +675,8 @@ static void at91_adc_trigger_remove(struct iio_dev *idev)
>  	int i;
>  
>  	for (i = 0; i < st->caps->trigger_number; i++) {
> +		if (!st->trig[i])
> +			continue;
>  		iio_trigger_unregister(st->trig[i]);
>  		iio_trigger_free(st->trig[i]);
>  	}