Re: [PATCH v4 3/3] iio: adc: ti-ads112c14: add continuous mode support

[Shuangpeng Bai <shuangpeng.kernel@gmail.com>]

Hi Jonathan,

I took another look at current_trigger_store() following your comment
about the possible TOCTOU there. There may also be a trigger lifetime
issue in the attach path.

I checked mainline commit
fd73f4a6659897191fa0d40695fe370925dd3780 (Linux 7.3-rc3).

The reference acquired by current_trigger_store() becomes the reference
held by indio_dev->trig, while iio_trigger_attach_poll_func() does not
take an additional device reference to the trigger.

For example, hi8435 uses INDIO_EVENT_TRIGGERED and allows changing its
trigger, so these stores reach attach/detach. Assume the consumer stays
registered, initially has no trigger, and T is a sysfs trigger with no
other users. No one writes trigger_now.

With two independently opened current_trigger files, the stores can run
concurrently because kernfs only serializes each open file:

  A: select T via current_trigger_store()
       acquire reference; indio_dev->trig = T
       iio_trigger_attach_poll_func(T, pollfunc_event)
         allocate pf->irq; request_threaded_irq() succeeds and returns
         <preempt before reading trig->ops>

  B: remove T via iio-trig-sysfs's remove_trigger
       iio_trigger_unregister(T)
       irq_work_sync(&t->work)
       iio_trigger_free(T)
     clear current_trigger with "\n"
       oldtrig = T; indio_dev->trig = NULL
       iio_trigger_detach_poll_func(T, pollfunc_event)
       iio_trigger_put(T) -> iio_trig_release() -> kfree(T)

  A: resume in iio_trigger_attach_poll_func()
       if (trig->ops && trig->ops->set_trigger_state && notinuse)
           ^ possible UAF

The consumer reference keeps T alive after removal, but B's clearing
store can drop the last reference while A is still in attach.

At the pause point the IRQ is installed, so B can detach it. T->ops is
NULL, and free_irq() does not wait for the enclosing attach call.

I have only checked this by source review and do not have a reproducer
or KASAN trace.

Does this race look possible to you?

Best,
Shuangpeng