Re: [PATCH v1] iio: health: max30102: fix NULL dereference in interrupt handler

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 3 Aug 2026 00:13:33 -0400
Marco Chen <marcochen.dev@gmail.com> 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.