[PATCH] iio: chemical: atlas-sensor: use iio_trigger_poll_nested() to fix remove UAF

[PATCH] iio: chemical: atlas-sensor: use iio_trigger_poll_nested() to fix remove UAF

[Fan Wu]

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 after
atlas_remove() returns without flushing that irq_work.  Once a buffer is
enabled, conversion-complete IRQs keep firing and queueing it; a pending
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 pointer
via container_of() and dereferences data->trig, a use-after-free.

Call iio_trigger_poll_nested() directly from the threaded handler instead
of bouncing through irq_work.  free_irq() then drains the threaded handler,
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 allow multiple chips")
Cc: stable@vger.kernel.org # v6.4+
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---

 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 {
 	struct iio_trigger *trig;
 	const struct atlas_device *chip;
 	struct regmap *regmap;
-	struct irq_work work;
 	unsigned int interrupt_enabled;
 	/* 96-bit data + 32-bit pad + 64-bit timestamp */
 	__be32 buffer[6] __aligned(8);
@@ -437,13 +435,6 @@ static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
 	.predisable = atlas_buffer_predisable,
 };

-static void atlas_work_handler(struct irq_work *work)
-{
-	struct atlas_data *data = container_of(work, struct atlas_data, work);
-
-	iio_trigger_poll(data->trig);
-}
-
 static irqreturn_t atlas_trigger_handler(int irq, void *private)
 {
 	struct iio_poll_func *pf = private;
@@ -470,7 +461,7 @@ static irqreturn_t atlas_interrupt_handler(int irq, void *private)
 	struct iio_dev *indio_dev = private;
 	struct atlas_data *data = iio_priv(indio_dev);

-	irq_work_queue(&data->work);
+	iio_trigger_poll_nested(data->trig);

 	return IRQ_HANDLED;
 }
@@ -666,8 +657,6 @@ static int atlas_probe(struct i2c_client *client)
 		goto unregister_trigger;
 	}

-	init_irq_work(&data->work, atlas_work_handler);
-
 	if (client->irq > 0) {
 		/* interrupt pin toggles on new conversion */
 		ret = devm_request_threaded_irq(&client->dev, client->irq,
--
2.43.0

Re: [PATCH] iio: chemical: atlas-sensor: use iio_trigger_poll_nested() to fix remove UAF

[Jonathan Cameron]

On Sun,  2 Aug 2026 07:18:58 +0000
Fan Wu <fanwu01@zju.edu.cn> wrote:

> 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 after
> atlas_remove() returns without flushing that irq_work.  Once a buffer is
> enabled, conversion-complete IRQs keep firing and queueing it; a pending
> 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 pointer
> via container_of() and dereferences data->trig, a use-after-free.
> 
> Call iio_trigger_poll_nested() directly from the threaded handler instead
> of bouncing through irq_work.  free_irq() then drains the threaded handler,
> 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 allow multiple chips")
> Cc: stable@vger.kernel.org # v6.4+
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

Added another email address for Matt.

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.

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.

Jonathan


> ---
> 
>  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 {
>  	struct iio_trigger *trig;
>  	const struct atlas_device *chip;
>  	struct regmap *regmap;
> -	struct irq_work work;
>  	unsigned int interrupt_enabled;
>  	/* 96-bit data + 32-bit pad + 64-bit timestamp */
>  	__be32 buffer[6] __aligned(8);
> @@ -437,13 +435,6 @@ static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
>  	.predisable = atlas_buffer_predisable,
>  };
> 
> -static void atlas_work_handler(struct irq_work *work)
> -{
> -	struct atlas_data *data = container_of(work, struct atlas_data, work);
> -
> -	iio_trigger_poll(data->trig);
> -}
> -
>  static irqreturn_t atlas_trigger_handler(int irq, void *private)
>  {
>  	struct iio_poll_func *pf = private;
> @@ -470,7 +461,7 @@ static irqreturn_t atlas_interrupt_handler(int irq, void *private)
>  	struct iio_dev *indio_dev = private;
>  	struct atlas_data *data = iio_priv(indio_dev);
> 
> -	irq_work_queue(&data->work);
> +	iio_trigger_poll_nested(data->trig);
> 
>  	return IRQ_HANDLED;
>  }
> @@ -666,8 +657,6 @@ static int atlas_probe(struct i2c_client *client)
>  		goto unregister_trigger;
>  	}
> 
> -	init_irq_work(&data->work, atlas_work_handler);
> -
>  	if (client->irq > 0) {
>  		/* interrupt pin toggles on new conversion */
>  		ret = devm_request_threaded_irq(&client->dev, client->irq,
> --
> 2.43.0
> 
> 

Re: [PATCH] iio: chemical: atlas-sensor: use iio_trigger_poll_nested() to fix remove UAF

[Matt Ranostay]

On Monday, 3 August 2026 at 02:32, Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun,  2 Aug 2026 07:18:58 +0000
> Fan Wu <fanwu01@zju.edu.cn> wrote:
> 
> > 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 after
> > atlas_remove() returns without flushing that irq_work.  Once a buffer is
> > enabled, conversion-complete IRQs keep firing and queueing it; a pending
> > 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 pointer
> > via container_of() and dereferences data->trig, a use-after-free.
> >
> > Call iio_trigger_poll_nested() directly from the threaded handler instead
> > of bouncing through irq_work.  free_irq() then drains the threaded handler,
> > 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 allow multiple chips")
> > Cc: stable@vger.kernel.org # v6.4+
> > Assisted-by: Codex:gpt-5.6
> > Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> 
> Added another email address for Matt.
> 
> 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 conjecture on what I was probably thinking back in 2016.
If I recall the reasoning was to allow other consumer iio triggers (e.g. hrtimer timer or sysfs)
to map to the atlas sensors.

Thanks,

Matt

> 
> 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.
> 
> Jonathan
> 
> 
> > ---
> >
> >  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 {
> >  	struct iio_trigger *trig;
> >  	const struct atlas_device *chip;
> >  	struct regmap *regmap;
> > -	struct irq_work work;
> >  	unsigned int interrupt_enabled;
> >  	/* 96-bit data + 32-bit pad + 64-bit timestamp */
> >  	__be32 buffer[6] __aligned(8);
> > @@ -437,13 +435,6 @@ static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
> >  	.predisable = atlas_buffer_predisable,
> >  };
> >
> > -static void atlas_work_handler(struct irq_work *work)
> > -{
> > -	struct atlas_data *data = container_of(work, struct atlas_data, work);
> > -
> > -	iio_trigger_poll(data->trig);
> > -}
> > -
> >  static irqreturn_t atlas_trigger_handler(int irq, void *private)
> >  {
> >  	struct iio_poll_func *pf = private;
> > @@ -470,7 +461,7 @@ static irqreturn_t atlas_interrupt_handler(int irq, void *private)
> >  	struct iio_dev *indio_dev = private;
> >  	struct atlas_data *data = iio_priv(indio_dev);
> >
> > -	irq_work_queue(&data->work);
> > +	iio_trigger_poll_nested(data->trig);
> >
> >  	return IRQ_HANDLED;
> >  }
> > @@ -666,8 +657,6 @@ static int atlas_probe(struct i2c_client *client)
> >  		goto unregister_trigger;
> >  	}
> >
> > -	init_irq_work(&data->work, atlas_work_handler);
> > -
> >  	if (client->irq > 0) {
> >  		/* interrupt pin toggles on new conversion */
> >  		ret = devm_request_threaded_irq(&client->dev, client->irq,
> > --
> > 2.43.0
> >
> >
> 
>

Re: [PATCH] iio: chemical: atlas-sensor: use iio_trigger_poll_nested() to fix remove UAF

[Jonathan Cameron]

On Mon, 03 Aug 2026 02:24:38 +0000
Matt Ranostay <matt@ranostay.sg> wrote:

> On Monday, 3 August 2026 at 02:32, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Sun,  2 Aug 2026 07:18:58 +0000
> > Fan Wu <fanwu01@zju.edu.cn> wrote:
> >   
> > > 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 after
> > > atlas_remove() returns without flushing that irq_work.  Once a buffer is
> > > enabled, conversion-complete IRQs keep firing and queueing it; a pending
> > > 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 pointer
> > > via container_of() and dereferences data->trig, a use-after-free.
> > >
> > > Call iio_trigger_poll_nested() directly from the threaded handler instead
> > > of bouncing through irq_work.  free_irq() then drains the threaded handler,
> > > 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 allow multiple chips")
> > > Cc: stable@vger.kernel.org # v6.4+
> > > Assisted-by: Codex:gpt-5.6
> > > Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>  
> > 
> > Added another email address for Matt.
> > 
> > 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 conjecture on what I was probably thinking back in 2016.
> If I recall the reasoning was to allow other consumer iio triggers (e.g. hrtimer timer or sysfs)
> to map to the atlas sensors.
> 

That way around shouldn't require this.  Using this to trigger
others would.  Meh, long time back as you note!

Anyhow, let's rip it out and see if anyone screams :)

Applied to the fixes-togreg branch of iio.git.

Jonathan


> Thanks,
> 
> Matt
> 
> > 
> > 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.
> > 
> > Jonathan
> > 
> >   
> > > ---
> > >
> > >  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 {
> > >  	struct iio_trigger *trig;
> > >  	const struct atlas_device *chip;
> > >  	struct regmap *regmap;
> > > -	struct irq_work work;
> > >  	unsigned int interrupt_enabled;
> > >  	/* 96-bit data + 32-bit pad + 64-bit timestamp */
> > >  	__be32 buffer[6] __aligned(8);
> > > @@ -437,13 +435,6 @@ static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
> > >  	.predisable = atlas_buffer_predisable,
> > >  };
> > >
> > > -static void atlas_work_handler(struct irq_work *work)
> > > -{
> > > -	struct atlas_data *data = container_of(work, struct atlas_data, work);
> > > -
> > > -	iio_trigger_poll(data->trig);
> > > -}
> > > -
> > >  static irqreturn_t atlas_trigger_handler(int irq, void *private)
> > >  {
> > >  	struct iio_poll_func *pf = private;
> > > @@ -470,7 +461,7 @@ static irqreturn_t atlas_interrupt_handler(int irq, void *private)
> > >  	struct iio_dev *indio_dev = private;
> > >  	struct atlas_data *data = iio_priv(indio_dev);
> > >
> > > -	irq_work_queue(&data->work);
> > > +	iio_trigger_poll_nested(data->trig);
> > >
> > >  	return IRQ_HANDLED;
> > >  }
> > > @@ -666,8 +657,6 @@ static int atlas_probe(struct i2c_client *client)
> > >  		goto unregister_trigger;
> > >  	}
> > >
> > > -	init_irq_work(&data->work, atlas_work_handler);
> > > -
> > >  	if (client->irq > 0) {
> > >  		/* interrupt pin toggles on new conversion */
> > >  		ret = devm_request_threaded_irq(&client->dev, client->irq,
> > > --
> > > 2.43.0
> > >
> > >  
> > 
> >