On Sat, 24 May 2025 12:34:09 +0200 Sean Nyekjaer sean@geanix.com 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 iio_device_claim_buffer_mode() to ensure indio_dev can't exit buffer mode during the flush.
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 Signed-off-by: Sean Nyekjaer sean@geanix.com
That's nasty and a case I'd never thought about. Most of the races around disabling end up with an extra sample or two which then gets dropped because there are no buffers enabled.
We need to consider the active scan mask as part of the buffer state. So effectively taking mlock if we enter this code will delay the state transition (and change of active_scan_mask until after this interrupt is done).
If David's synchronize_irq() is enough maybe that's a lighter weight path?
Jonathan
drivers/iio/accel/fxls8962af-core.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c index 6d23da3e7aa22c61f2d9348bb91d70cc5719a732..7db83ebeea823173d79bf8ff484add16f575edfc 100644 --- a/drivers/iio/accel/fxls8962af-core.c +++ b/drivers/iio/accel/fxls8962af-core.c @@ -973,6 +973,9 @@ static int fxls8962af_fifo_flush(struct iio_dev *indio_dev) if (ret) return ret;
- if (iio_device_claim_buffer_mode(indio_dev) < 0)
return 0;- /* Demux hw FIFO into kfifo. */ for (i = 0; i < count; i++) { int j, bit;
@@ -989,6 +992,8 @@ static int fxls8962af_fifo_flush(struct iio_dev *indio_dev) tstamp += sample_period; }
- iio_device_release_buffer_mode(indio_dev);
- return count;
}
base-commit: 5c3fcb36c92443a9a037683626a2e43d8825f783 change-id: 20250524-fxlsrace-f4d20e29fb29
Best regards,