Re: [PATCH v1] iio: health: max30102: fix NULL dereference in interrupt handler
Jonathan Cameron <[email protected]>
| Newsgroups | dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260805002548.01ff7177@jic23-huawei> |
On Mon, 3 Aug 2026 00:13:33 -0400 Marco Chen <[email protected]> wrote: > On Sun, Aug 02, 2026 at 03:26:09AM +0100, Jonathan Cameron wrote: > > > + if (!indio_dev->active_scan_mask) { > > > > That is racy as this could be going on in parallel with the buffer being disabled > > so we check here but it's gone before it is accessed. > > I understand what you mean. I will remove this active_scan_mask check. > > > Which leads me to suggest alternative fix - check INT_STATUS and if nothing > > set it isn't our interrupt. That doesn't get into the potential buffer > > enabled / disabled races but should close your condition I think > > - looks like it is read in the fifo part below. You may need to refactor a > > little to not read it twice. > > This makes a lot of sense. For the v2 patch, I will plan to read > INT_STATUS once at the top of the handler and pass that value into > max30102_fifo_count() so it is not read twice. Consequently I will > remove the INT_STATUS read from max30102_fifo_count(), too. > > For checking INT_STATUS to see if it isn't our interrupt, instead of > checking if nothing is set, I was thinking to check the > MAX30102_REG_INT_STATUS_FIFO_RDY bit specifically because that is the > only interrupt enabled in max30102_chip_init(). Agreed. That is a better solution than mine :) > > This will close the reproducer I was hitting earlier as you said. Since > we only ever reach bitmap_weight() when FIFO_RDY is set, the NULL dereference > is avoided. I think there is a theoretical window if the buffer is disabled > between the FIFO_RDY check and bitmap_weight() call, but as you said, > closing this would need the buffered mode claim. I plan to leave that > out of this fix.