[PATCH v2] iio: accel: fxls8962af: Fix use after free in fxls8962af_fifo_flush

[PATCH v2] iio: accel: fxls8962af: Fix use after free in fxls8962af_fifo_flush

[Sean Nyekjaer]

fxls8962af_fifo_flush() uses indio_dev->active_scan_mask (with
iio_for_each_active_channel()) without making sure the indio_dev
stays in buffer mode.
There is a race if indio_dev exits buffer mode in the middle of the
interrupt that flushes the fifo. Fix this by calling
synchronize_irq() to ensure that no interrupt is currently running when
disabling buffer mode.

Unable to handle kernel NULL pointer dereference at virtual address 00000000 when read
[...]
_find_first_bit_le from fxls8962af_fifo_flush+0x17c/0x290
fxls8962af_fifo_flush from fxls8962af_interrupt+0x80/0x178
fxls8962af_interrupt from irq_thread_fn+0x1c/0x7c
irq_thread_fn from irq_thread+0x110/0x1f4
irq_thread from kthread+0xe0/0xfc
kthread from ret_from_fork+0x14/0x2c

Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
Cc: stable@vger.kernel.org
Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes in v2:
- As per David's suggestion; switched to use synchronize_irq() instead.
- Link to v1: https://lore.kernel.org/r/20250524-fxlsrace-v1-1-dec506dc87ae@geanix.com
---
 drivers/iio/accel/fxls8962af-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
index 6d23da3e7aa22c61f2d9348bb91d70cc5719a732..f2558fba491dffa78b26d47d2cd9f1f4d9811f54 100644
--- a/drivers/iio/accel/fxls8962af-core.c
+++ b/drivers/iio/accel/fxls8962af-core.c
@@ -866,6 +866,8 @@ static int fxls8962af_buffer_predisable(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
+	synchronize_irq(data->irq);
+
 	ret = __fxls8962af_fifo_set_mode(data, false);
 
 	if (data->enable_event)

---
base-commit: 5c3fcb36c92443a9a037683626a2e43d8825f783
change-id: 20250524-fxlsrace-f4d20e29fb29

Best regards,
-- 
Sean Nyekjaer <sean@geanix.com>

Re: [PATCH v2] iio: accel: fxls8962af: Fix use after free in fxls8962af_fifo_flush

[David Lechner]

On 6/3/25 7:25 AM, Sean Nyekjaer wrote:
> fxls8962af_fifo_flush() uses indio_dev->active_scan_mask (with
> iio_for_each_active_channel()) without making sure the indio_dev
> stays in buffer mode.
> There is a race if indio_dev exits buffer mode in the middle of the
> interrupt that flushes the fifo. Fix this by calling
> synchronize_irq() to ensure that no interrupt is currently running when
> disabling buffer mode.
> 
> Unable to handle kernel NULL pointer dereference at virtual address 00000000 when read
> [...]
> _find_first_bit_le from fxls8962af_fifo_flush+0x17c/0x290
> fxls8962af_fifo_flush from fxls8962af_interrupt+0x80/0x178
> fxls8962af_interrupt from irq_thread_fn+0x1c/0x7c
> irq_thread_fn from irq_thread+0x110/0x1f4
> irq_thread from kthread+0xe0/0xfc
> kthread from ret_from_fork+0x14/0x2c
> 
> Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
> Cc: stable@vger.kernel.org
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes in v2:
> - As per David's suggestion; switched to use synchronize_irq() instead.
> - Link to v1: https://lore.kernel.org/r/20250524-fxlsrace-v1-1-dec506dc87ae@geanix.com
Were you able to find a way to reproduce the bug well enough to
test this?

Re: [PATCH v2] iio: accel: fxls8962af: Fix use after free in fxls8962af_fifo_flush

[Sean Nyekjaer]

On Tue, Jun 03, 2025 at 08:29:51AM +0100, David Lechner wrote:
> On 6/3/25 7:25 AM, Sean Nyekjaer wrote:
> > fxls8962af_fifo_flush() uses indio_dev->active_scan_mask (with
> > iio_for_each_active_channel()) without making sure the indio_dev
> > stays in buffer mode.
> > There is a race if indio_dev exits buffer mode in the middle of the
> > interrupt that flushes the fifo. Fix this by calling
> > synchronize_irq() to ensure that no interrupt is currently running when
> > disabling buffer mode.
> >
> > Unable to handle kernel NULL pointer dereference at virtual address 00000000 when read
> > [...]
> > _find_first_bit_le from fxls8962af_fifo_flush+0x17c/0x290
> > fxls8962af_fifo_flush from fxls8962af_interrupt+0x80/0x178
> > fxls8962af_interrupt from irq_thread_fn+0x1c/0x7c
> > irq_thread_fn from irq_thread+0x110/0x1f4
> > irq_thread from kthread+0xe0/0xfc
> > kthread from ret_from_fork+0x14/0x2c
> >
> > Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
> > Cc: stable@vger.kernel.org
> > Suggested-by: David Lechner <dlechner@baylibre.com>
> > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> > ---
> > Changes in v2:
> > - As per David's suggestion; switched to use synchronize_irq() instead.
> > - Link to v1: https://lore.kernel.org/r/20250524-fxlsrace-v1-1-dec506dc87ae@geanix.com
> Were you able to find a way to reproduce the bug well enough to
> test this?
> 

Yeah. Sorry I didn't included the reproducer :)

I added:
diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
index f2558fba491d..ce9a14245f83 100644
--- a/drivers/iio/accel/fxls8962af-core.c
+++ b/drivers/iio/accel/fxls8962af-core.c
@@ -1040,6 +1040,8 @@ static irqreturn_t fxls8962af_interrupt(int irq, void *p)
 	unsigned int reg;
 	int ret;
 
+	usleep_range(10000, 15000);
+
 	ret = regmap_read(data->regmap, FXLS8962AF_INT_STATUS, &reg);
 	if (ret)
 		return IRQ_NONE;

And it was reproduceable within 10 secs:
root@localhost:/sys/bus/iio/devices/iio:device0/buffer0#
while true; do echo 1 > enable; sleep 1; echo 0 > enable; sleep 1; done

With synchronize_irq(data->irq); I have not been able to reproduce it.

/Sean

Re: [PATCH v2] iio: accel: fxls8962af: Fix use after free in fxls8962af_fifo_flush

[Jonathan Cameron]

On Tue, 03 Jun 2025 13:36:15 +0000
Sean Nyekjaer <sean@geanix.com> wrote:

> On Tue, Jun 03, 2025 at 08:29:51AM +0100, David Lechner wrote:
> > On 6/3/25 7:25 AM, Sean Nyekjaer wrote:  
> > > fxls8962af_fifo_flush() uses indio_dev->active_scan_mask (with
> > > iio_for_each_active_channel()) without making sure the indio_dev
> > > stays in buffer mode.
> > > There is a race if indio_dev exits buffer mode in the middle of the
> > > interrupt that flushes the fifo. Fix this by calling
> > > synchronize_irq() to ensure that no interrupt is currently running when
> > > disabling buffer mode.
> > >
> > > Unable to handle kernel NULL pointer dereference at virtual address 00000000 when read
> > > [...]
> > > _find_first_bit_le from fxls8962af_fifo_flush+0x17c/0x290
> > > fxls8962af_fifo_flush from fxls8962af_interrupt+0x80/0x178
> > > fxls8962af_interrupt from irq_thread_fn+0x1c/0x7c
> > > irq_thread_fn from irq_thread+0x110/0x1f4
> > > irq_thread from kthread+0xe0/0xfc
> > > kthread from ret_from_fork+0x14/0x2c
> > >
> > > Fixes: 79e3a5bdd9ef ("iio: accel: fxls8962af: add hw buffered sampling")
> > > Cc: stable@vger.kernel.org
> > > Suggested-by: David Lechner <dlechner@baylibre.com>
> > > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> > > ---
> > > Changes in v2:
> > > - As per David's suggestion; switched to use synchronize_irq() instead.
> > > - Link to v1: https://lore.kernel.org/r/20250524-fxlsrace-v1-1-dec506dc87ae@geanix.com  
> > Were you able to find a way to reproduce the bug well enough to
> > test this?
> >   
> 
> Yeah. Sorry I didn't included the reproducer :)
> 
> I added:
> diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
> index f2558fba491d..ce9a14245f83 100644
> --- a/drivers/iio/accel/fxls8962af-core.c
> +++ b/drivers/iio/accel/fxls8962af-core.c
> @@ -1040,6 +1040,8 @@ static irqreturn_t fxls8962af_interrupt(int irq, void *p)
>  	unsigned int reg;
>  	int ret;
>  
> +	usleep_range(10000, 15000);
> +
>  	ret = regmap_read(data->regmap, FXLS8962AF_INT_STATUS, &reg);
>  	if (ret)
>  		return IRQ_NONE;
> 
> And it was reproduceable within 10 secs:
> root@localhost:/sys/bus/iio/devices/iio:device0/buffer0#
> while true; do echo 1 > enable; sleep 1; echo 0 > enable; sleep 1; done
> 
> With synchronize_irq(data->irq); I have not been able to reproduce it.
Nice.

Applied to a temporary tree that I'll rebase on rc1 once available.

Thanks,

Jonathan

> 
> /Sean
>