Re: [PATCH v3] iio: adc: at91_adc: move to devm_* to fix NULL pointer deref

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 11 Nov 2026 12:15:44 +0100
Joshua Crofts <crofts@kernel.org> wrote:

> On Wed, 11 Nov 2026 12:13:45 +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. The error paths and remove function unconditionally called
> > iio_trigger_unregister() and iio_trigger_free() on them, causing a
> > NULL pointer dereference.
> > 
> > Instead of just checking for NULL, refactor the driver to use the
> > device-managed (devm_*) API for registration and allocation. This
> > modernizes the code, eliminates the need for manual cleanup functions
> > (at91_adc_trigger_remove, at91_adc_buffer_remove, at91_ts_unregister,
> > and at91_adc_remove), and inherently fixes the NULL pointer dereference
> > since the core handles the cleanup of properly allocated triggers.
> > 
> > As part of this simplification, the remaining trigger initialization
> > logic has been inlined into at91_adc_probe().
> > 
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Fixes: 0e589d5fb317 ("ARM: AT91: IIO: Add AT91 ADC driver.")
> > Signed-off-by: Abdelnasser Hussein <abdelnasserhussein11@gmail.com>
> > ---
> > v3:
> >  - Addressed review comments from Andy Shevchenko:
> >    - Removed the redundant dev_err() after devm_input_allocate_device().
> >    - Declared a local 'struct device *dev = &pdev->dev' and used it in
> >      at91_adc_probe().
> >    - Declared the loop iterator inside the for-loop.
> > 
> > v2:
> >  - Dropped the simple NULL check approach.
> >  - Refactored the driver to use devm_* API as suggested by Jonathan Cameron.
> >  - Removed manual cleanup functions and error paths entirely.
> >  - Inlined remaining trigger initialization into probe().
> > 
> >  drivers/iio/adc/at91_adc.c | 185 +++++++++++--------------------------
> >  1 file changed, 52 insertions(+), 133 deletions(-)  
> 
> I'm not sure if this is an adequate fix. I'd imagine there would be two patches,
> one that just adds the simple null check so it can be backported and this patch
> that will go upstream.
> 
> The commit that introduces the bug is 14 years old, so it doesn't make sense to
> me to backport an entire devm_* conversion.
> 
> Jonathan, Andy, what do you think?

So, we don't have to backport what we upstream. I could take that null check (which
was v1 of this) and then apply this on top I suppose to make things a little
easier. Alternative is to ask for a backport of the simpler patch with this
referenced as unsuitable for backport.

Jonathan


>