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

[Jonathan Cameron <jic23@kernel.org>]

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
> 
>