Re: [PATCH] iio: chemical: atlas-sensor: use iio_trigger_poll_nested() to fix remove UAF
Matt Ranostay <[email protected]> Mon, 03 Aug 2026 02:24:38 +0000
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <PT1b4dloegkgrgv26ZJLYsrFQCIfsP236gwpWOn_IxzyZKkz9TV21c4wtPPHKWG6iEfj90YqAQATzTV8yT3T7C6tCRC9egl9PlsU7G0780I=@ranostay.sg> |
On Monday, 3 August 2026 at 02:32, Jonathan Cameron <[email protected]> wrot= e: > On Sun, 2 Aug 2026 07:18:58 +0000 > Fan Wu <[email protected]> wrote: >=20 > > The atlas driver requests its hardware data-ready IRQ with > > devm_request_threaded_irq(); its threaded handler queues an irq_work, > > atlas_work_handler(), that calls iio_trigger_poll(data->trig). > > > > The IRQ is devm-managed, so free_irq() runs from the devres unwind afte= r > > atlas_remove() returns without flushing that irq_work. Once a buffer i= s > > enabled, conversion-complete IRQs keep firing and queueing it; a pendin= g > > irq_work can therefore run after the unwind has freed atlas_data/indio_= dev > > and the trigger, when atlas_work_handler() derives the atlas_data point= er > > via container_of() and dereferences data->trig, a use-after-free. > > > > Call iio_trigger_poll_nested() directly from the threaded handler inste= ad > > of bouncing through irq_work. free_irq() then drains the threaded hand= ler, > > closing the window; other iio drivers with a threaded data-ready IRQ do= the > > same (e.g. bmi270). > > > > This issue was found by an in-house static analysis tool. > > > > Fixes: 7103b99b031c ("iio: chemical: atlas-ph-sensor: reorg driver to a= llow multiple chips") > > Cc: [email protected] # v6.4+ > > Assisted-by: Codex:gpt-5.6 > > Signed-off-by: Fan Wu <[email protected]> >=20 > Added another email address for Matt. >=20 > There are reasons why he might have got the irq_work route but I can't > recall if they applied. What we lose here is the ability to hang > other consumers that need a top half of the trigger. If that doesn't > matter then agreed your solution is the cleanest path forwards. Hello all! Looking at the 'git log' and 10 years ago so it I'm only doing c= onjecture on what I was probably thinking back in 2016. If I recall the reasoning was to allow other consumer iio triggers (e.g. hr= timer timer or sysfs) to map to the atlas sensors. Thanks, Matt >=20 > One day someone will get the time to make combining nested > trigger handling with top halves cleverer than current approach of > just not running them. >=20 > Jonathan >=20 >=20 > > --- > > > > drivers/iio/chemical/atlas-sensor.c | 13 +------------ > > 1 file changed, 1 insertion(+), 12 deletions(-) > > > > diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical= /atlas-sensor.c > > --- a/drivers/iio/chemical/atlas-sensor.c > > +++ b/drivers/iio/chemical/atlas-sensor.c > > @@ -13,7 +13,6 @@ > > #include <linux/mutex.h> > > #include <linux/err.h> > > #include <linux/irq.h> > > -#include <linux/irq_work.h> > > #include <linux/i2c.h> > > #include <linux/mod_devicetable.h> > > #include <linux/regmap.h> > > @@ -88,7 +87,6 @@ struct atlas_data { > > =09struct iio_trigger *trig; > > =09const struct atlas_device *chip; > > =09struct regmap *regmap; > > -=09struct irq_work work; > > =09unsigned int interrupt_enabled; > > =09/* 96-bit data + 32-bit pad + 64-bit timestamp */ > > =09__be32 buffer[6] __aligned(8); > > @@ -437,13 +435,6 @@ static const struct iio_buffer_setup_ops atlas_buf= fer_setup_ops =3D { > > =09.predisable =3D atlas_buffer_predisable, > > }; > > > > -static void atlas_work_handler(struct irq_work *work) > > -{ > > -=09struct atlas_data *data =3D container_of(work, struct atlas_data, w= ork); > > - > > -=09iio_trigger_poll(data->trig); > > -} > > - > > static irqreturn_t atlas_trigger_handler(int irq, void *private) > > { > > =09struct iio_poll_func *pf =3D private; > > @@ -470,7 +461,7 @@ static irqreturn_t atlas_interrupt_handler(int irq,= void *private) > > =09struct iio_dev *indio_dev =3D private; > > =09struct atlas_data *data =3D iio_priv(indio_dev); > > > > -=09irq_work_queue(&data->work); > > +=09iio_trigger_poll_nested(data->trig); > > > > =09return IRQ_HANDLED; > > } > > @@ -666,8 +657,6 @@ static int atlas_probe(struct i2c_client *client) > > =09=09goto unregister_trigger; > > =09} > > > > -=09init_irq_work(&data->work, atlas_work_handler); > > - > > =09if (client->irq > 0) { > > =09=09/* interrupt pin toggles on new conversion */ > > =09=09ret =3D devm_request_threaded_irq(&client->dev, client->irq, > > -- > > 2.43.0 > > > > >=20 >