Re: [PATCH] iio: light: gp2ap020a00f: drain irq_work after free_irq

[Jonathan Cameron]

On Thu,  6 Aug 2026 13:29:23 +0000
Fan Wu <fanwu01@xxxxxxxxxx> wrote:

> The threaded IRQ handler queues data->work through irq_work_queue() so
> the trigger is polled from a per-CPU context.  free_irq() does not flush
> an irq_work the handler already queued, so after gp2ap020a00f_remove()
> returns that work may still run and call iio_trigger_poll() on data->trig,
> which the devm cleanup has already freed, causing a use-after-free.
> 
> Add irq_work_sync(&data->work) after free_irq() in remove() and in the
> probe error path, mirroring commit 78601726d4a5 ("iio: trigger: sysfs:
> fix use-after-free on remove").
> 
> Found by an in-house static analysis tool, confirmed by manual review.
> 
> Fixes: bf29fbeaa13d ("iio: gp2ap020a00f: Add a driver for the device")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
Hi,

Thanks for the patch. It looks correct to me but this is an old driver
and we'd not use an irq work at all these days. The reason it was used
was to ensure we could use the trigger from this device to drive capture
on another one that needed a top half.  However the top half (non threaded
bit) of trigger handlers only ever gets a timestamp, so we end up dancing
through
threaded_irq->irq_work->iio_pollfunc_get_timestamp->actualhander.
which almost certainly gives a less useful timestamp than
threaded_irq->actuallhandler without the irq_work part.
The slight risk is someone is actually using that timestamp in
a consumer and will find pf->timestamp is now always 0.  However
I think that's very unlikely with a light sensor (sensor fusion
doesn't really occur much like it does for inertial sensors)

So I think a better solution given you've identified a bug here, would be
to rip out the irq_work dance in favour of a direct call to
iio_trigger_poll_nested() instead of irq_work_queue() in 
gp2ap020a00f_thresh_event_handler()

That would mean also grabbing a timestamp locally in
gp2ap020a00f_trigger_handler() rather than using pf->timestamp()
+ dropping the use of iio_pollfunc_store_time.

A more invasive change, but one that lands us with a more modern solution
and removes the problematic code entirely.

Do you feel like taking that challenge on?  If so do it on top
of this patch because this one is going to be easier to backport.

Applied to the fixes-togreg branch of iio.git but note that won't
go upstream until after rc1 is released (and I'll rebase on that)

Thanks,

Jonathan


> ---
>  drivers/iio/light/gp2ap020a00f.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
> index c7df4b258e2c..20d1aa9e99d7 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -1560,6 +1560,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
>  	iio_trigger_unregister(data->trig);
>  error_free_irq:
>  	free_irq(client->irq, indio_dev);
> +	irq_work_sync(&data->work);
>  error_uninit_buffer:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  error_regulator_disable:
> @@ -1582,6 +1583,7 @@ static void gp2ap020a00f_remove(struct i2c_client *client)
>  	iio_device_unregister(indio_dev);
>  	iio_trigger_unregister(data->trig);
>  	free_irq(client->irq, indio_dev);
> +	irq_work_sync(&data->work);
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	regulator_disable(data->vled_reg);
>  }
>