From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-f54.google.com (mail-oa1-f54.google.com [209.85.160.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C13612DF68 for ; Sat, 1 Aug 2026 15:18:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785597530; cv=none; b=pkNf2VBnqLh800WTvfdmdAqJ0QPkUtM78ENDzlhUPLmCMQ872l8UqtWqzExubNwSFyLEIZ12Hy8/5Be3qrp56tiRg0AJgnxlakBSELnJwiZ5HHnpsuq0pO6VpyN+nzQDGlckHQfRtcV2/T04rtzPlBWCK6ZhQRe2yChNkXDV27U= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785597530; c=relaxed/simple; bh=ZD6WFg3E7WMeQJ/6siAitoDwF8pIbMsF6e9YFtv4qQU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=e8TZziBKTSGBnwBvIFTjKSi7n99+QpSLpVjyYy70EIN1s5uHrd3TIG/H6q7nnq/pbQx+kUHe4bLeflVqgi/LcoAugOIMuX8zg6A2P7DPTmn9DEHOQee9LyOJNPFy9lDYP1o+wrHXTf8sdmqTzf03jNMIS0WH0cZoKwJTUtesOLE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b=SGB9wNMP; arc=none smtp.client-ip=209.85.160.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b="SGB9wNMP" Received: by mail-oa1-f54.google.com with SMTP id 586e51a60fabf-4472500e25fso967172fac.1 for ; Sat, 01 Aug 2026 08:18:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre.com; s=google; t=1785597527; x=1786202327; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=BkrJvX7ruNCnFu6Nri8nnZ3AJjvtpseq0lUs6PITb4A=; b=SGB9wNMPrwNV6STeICjDvuvmS0Tz43gI2S8f3hFDRHNKRit95HMjcCZYFVBcQZ8WuR whOh0rEpv1YJEO3rv4ZZyQWj8HMA+/J6Wiu398kps58D4p9uyrAjthWEU2tSq2ALHUPM X3TaKf7JnTsKrJbanKRkfR+vvXYdPArupGkxjS/EzrvY/5/XyULFdEOEJgn4UC5m6WYW nGig1PZ8yNJteP2hD2TvSKV9HSWqRLLgpLIb5O4unktGeTXVvpGACCANkE5L6dZ5LMIN tZWFWGnh/+l1meZxYHD7FGQgJIoCOxP4/bybPBfPtJNA30figDndaVc2p3h5N0c8HyCB 1e9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785597527; x=1786202327; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=BkrJvX7ruNCnFu6Nri8nnZ3AJjvtpseq0lUs6PITb4A=; b=S7WCqEk6lL2ERmJuoW88PwJ34WwE/9uxx2v02U7sxed7XPtjwao11LqYflIz41FdSS IGVuvT8upY1+tCugbb2Apy0YHX1pCFCuRVEw5Mk9cf0upxcP+3FARvIUQhOWAmkdZOHk XPKrtXnLXZfccsjBCWTBqqNo/zlO4LfLMtZ4lPhkr/FltTCyEBHyzK7rEggNpKv3kfEr eo+yf3wgxqXq1XlaAjyJEEv6QWp3nE3J+25tDdjy6M5dS21fVulDQTvFvspw3ZV5m3xe pUqwIBmUR3DpWq9KnpdB3TI06OihB4z6asq6nDD6RKHeBTXgAKN5IJH6RCw4sn3JUNX9 QOBA== X-Forwarded-Encrypted: i=1; AHgh+RpkeNTpgPuA4yxxboBQLgLbSv3xobXrcyfHFCjkkwaE9jk8EiijnKVw5VoO/0nQYCKiI5YvxM+Wie85NzA=@vger.kernel.org X-Gm-Message-State: AOJu0Yyi7ql4dMCFxIipjTYEPboulv4uuu8cAWX22pNc82g49WXKklcr KwET8Co0QbIjUo3ZSX2VdfGwwUQ4JKhwymcJzJU9/0Zy67iJfVEiEgi0mPK8UdN4eTQ= X-Gm-Gg: AR+sD11/bCN3PD6sSSnQVUm4xfBdzutQzqZS6q7QHLmxCXQwB/i+U/2ghACy2GPQndc pY3mjh9obW5/IuGTUgxEU6XGc/XEIs7w50QwMQ4pBXee5yPQpfO26uh7Qu83atAUgE3Y2ndWXzL 4aEApGPG7ti9r6C+Qa4xptQcYCGsMfzRri+8bznW8RY1PBBbpxG7w77Cu7xlnyIlCBzQrqOU2R0 hK0SmQzkb+JlYu9xsGQKbAIiKHnbnh4yZi2xjMeGYqEJ8BVJghX9ZdSAEytLfoY+ftQHaxaVQQw I8t36sWtVhexawRwIVOxU2f+bpACyP3oNZiKJiFltc9zv6VVWv+U1zLtgxRSRsFL8gs35CQ5dqi xiZZOGPI1whmfGSn9gEhKEpXQ4SfwXksIfd8tuUDQXagJ9V/Vb91Vv4Q4Ofpqbiske2SenGkTot KsYOIeFcK5QyAXs7fiLKDK1o+r35myF2r7Bi89CS4xfh2cwc6gRUIf3KHHZz1bgHUbxS/Ybc5JK K0fms/gDnpAFd+kxH8dXwquXuQHqhMlaHo/lnw= X-Received: by 2002:a05:6808:1b98:b0:492:973e:1c23 with SMTP id 5614622812f47-4af5e30f381mr7473132b6e.15.1785597526617; Sat, 01 Aug 2026 08:18:46 -0700 (PDT) Received: from ?IPV6:2600:8803:e7e4:500:359b:17f1:d4f9:4949? ([2600:8803:e7e4:500:359b:17f1:d4f9:4949]) by smtp.gmail.com with ESMTPSA id 5614622812f47-4af58933884sm3013821b6e.0.2026.08.01.08.18.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 01 Aug 2026 08:18:44 -0700 (PDT) Message-ID: Date: Sat, 1 Aug 2026 10:18:42 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] iio: health: max30102: fix NULL dereference in interrupt handler To: Marco Chen , jic23@kernel.org Cc: nuno.sa@analog.com, andy@kernel.org, pmeerw@pmeerw.net, matt@ranostay.sg, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, linux-kernel-mentees@lists.linux.dev References: <20260731184124.112124-1-marcochen.dev@gmail.com> Content-Language: en-US From: David Lechner In-Reply-To: <20260731184124.112124-1-marcochen.dev@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 7/31/26 1:41 PM, Marco Chen wrote: > The interrupt is requested in max30102_probe() and stays enabled I've always wondered why most IIO drivers seem to do this. It seems to me that requesting the irq with IRQF_NO_AUTOEN and only enabling it when everything is set up for reading data would avoid spurious interrupts like this. > for the lifetime of the device, but indio_dev->active_scan_mask is only > valid while a buffer is enabled. When an interrupt arrives while no > buffer is enabled, the handler dereferences the NULL active_scan_mask: > > Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000000 > pc : __bitmap_weight+0x64/0x98 > lr : max30102_interrupt_handler+0x48/0x160 [max30102] > Call trace: > __bitmap_weight+0x64/0x98 (P) > max30102_interrupt_handler+0x48/0x160 [max30102] > irq_thread_fn+0x28/0xa8 > irq_thread+0x184/0x30c > kthread+0x118/0x124 > ret_from_fork+0x10/0x20 > > Return early when no buffer is enabled. The interrupt status register is > read before returning, since reading it deasserts the chip's active-low > interrupt pin. Otherwise the pin would stay asserted and no further > edges would be delivered. > > Fixes: 90579b69e94b ("iio: health: max30102: Add MAX30105 support") > Signed-off-by: Marco Chen > --- > I ran into this while interfacing with the MAX30102 over I2C on a > Raspberry Pi 4 running the IIO subsystem tree testing branch and > learning the IIO sysfs interface for the first time. When physically > rearranging INT pin wiring with no buffer enabled, the kernel oopsed. This is one of the reasons why you are supposed to turn off power before moving wires. ;-) > This happened because max30102_interrupt_handler() attempted to > dereference active_scan_mask, which is NULL because no buffer is > enabled. With this patch, the same situation no longer oopses > and the buffered capture was tested to work normally afterward. > > Some things I am unsure about though: > - Is IRQ_HANDLED or IRQ_NONE preferred here? I chose IRQ_HANDLED > because of the status register read to deassert the INT pin, but > I am not 100% confident on this choice. IRQ_NONE would cause the handler to just run again as soon as it exits, so it is almost never the right value to return. IRQ_HANDLED is correct. > - Should the regmap_read() return value be checked? I did not add a > check because I do not see a useful recovery path from this I2C failure, > but I can add a check in a v2 if it is better. There isn't anything we could do other than log the error. > Thank you. > > drivers/iio/health/max30102.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/health/max30102.c b/drivers/iio/health/max30102.c > index c37316c86f14..c30b029ba8aa 100644 > --- a/drivers/iio/health/max30102.c > +++ b/drivers/iio/health/max30102.c > @@ -290,10 +290,23 @@ static irqreturn_t max30102_interrupt_handler(int irq, void *private) > { > struct iio_dev *indio_dev = private; > struct max30102_data *data = iio_priv(indio_dev); > - unsigned int measurements = bitmap_weight(indio_dev->active_scan_mask, > - iio_get_masklength(indio_dev)); > + unsigned int measurements, val; > int ret, cnt = 0; > > + if (!indio_dev->active_scan_mask) { > + /* > + * No buffer is enabled so there is nothing to read. Read the > + * status register anyway to deassert the max30102's interrupt > + * pin; otherwise it would stay asserted and further edges > + * would not be delivered. > + */ > + regmap_read(data->regmap, MAX30102_REG_INT_STATUS, &val); > + return IRQ_HANDLED; > + } > + > + measurements = bitmap_weight(indio_dev->active_scan_mask, > + iio_get_masklength(indio_dev)); > + > mutex_lock(&data->lock); > > while (cnt || (cnt = max30102_fifo_count(data)) > 0) {