mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths
@ 2026-09-06 13:11 Fabio Cesari
  2026-09-06 13:55 ` Joshua Crofts
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Fabio Cesari @ 2026-09-06 13:11 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Brian Masney, linux-iio, linux-kernel

Both isl29028_read_raw() and isl29028_write_raw() take a runtime PM
reference with pm_runtime_resume_and_get() and are supposed to drop it
again with pm_runtime_put_autosuspend() before returning. On their error
paths they return directly instead, leaking the reference.

The usage count is then never balanced, so the device stops entering
autosuspend for the rest of its lifetime. The effect accumulates: every
failed access leaks another reference.

In isl29028_write_raw() this is reachable from userspace with a single
rejected sysfs write, for example

  echo 200 > in_proximity_sampling_frequency

which is outside the accepted [1:100] range, or

  echo 999 > in_illuminance_scale

which is not one of the two accepted scales. Both return -EINVAL with
the reference still held. In isl29028_read_raw() the leak is reached
when the underlying regmap access fails.

Drop the reference before checking the error, reusing the pm_ret
pattern already present in isl29028_read_raw().

Found by auditing IIO drivers for runtime PM acquire/release imbalances
with a Coccinelle semantic patch that models pm_runtime_resume_and_get()
and pm_runtime_put_autosuspend() along the control flow graph, flagging
functions that take a reference and then reach a return without dropping
it.

Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5 coccinelle
Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
---

Compile-tested only: arm64 (native) and x86_64 (cross), defconfig plus
CONFIG_SENSORS_ISL29028=m, with gcc 15.2.0, W=1 and sparse v0.6.5-rc1:
no warnings. I have no isl29028 hardware, so this is untested at
runtime.

I also have a version that takes the runtime PM reference only where it
is needed: isl29028_write_raw() validates its arguments first, and
isl29028_read_raw() acquires it only for the reads that reach the
hardware, the sampling frequency and lux scale being cached. It also
stops propagating the pm_runtime_put_autosuspend() return value to
userspace, which fixes a second problem: with CONFIG_PM=n that call
returns -ENOSYS, so every read and write fails today even when the
access itself succeeded.

I kept this patch to the one bug, since the rest changes what userspace
sees. Happy to send that version on top once this lands, or instead of
this one if you would rather have it that way.

 drivers/iio/light/isl29028.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
index 33deb1726689..c5146c1d9f39 100644
--- a/drivers/iio/light/isl29028.c
+++ b/drivers/iio/light/isl29028.c
@@ -340,7 +340,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 {
 	struct isl29028_chip *chip = iio_priv(indio_dev);
 	struct device *dev = regmap_get_device(chip->regmap);
-	int ret;
+	int ret, pm_ret;
 
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
@@ -392,12 +392,11 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->lock);
 
+	pm_ret = pm_runtime_put_autosuspend(dev);
 	if (ret < 0)
 		return ret;
-
-	ret = pm_runtime_put_autosuspend(dev);
-	if (ret < 0)
-		return ret;
+	if (pm_ret < 0)
+		return pm_ret;
 
 	return 0;
 }
@@ -461,15 +460,14 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->lock);
 
-	if (ret < 0)
-		return ret;
-
 	/**
 	 * Preserve the ret variable if the call to
 	 * pm_runtime_put_autosuspend() is successful so the reading
 	 * (if applicable) is returned to user space.
 	 */
 	pm_ret = pm_runtime_put_autosuspend(dev);
+	if (ret < 0)
+		return ret;
 	if (pm_ret < 0)
 		return pm_ret;
 

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.53.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-09 16:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-06 13:11 [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths Fabio Cesari
2026-09-06 13:55 ` Joshua Crofts
2026-09-06 22:09   ` Fabio Cesari
2026-09-06 17:43 ` Jonathan Cameron
2026-09-06 22:15   ` Fabio Cesari
2026-09-07  2:07     ` Jonathan Cameron
2026-09-07  7:29       ` Joshua Crofts
2026-09-09 16:21         ` Fabio Cesari
2026-09-06 22:37 ` [PATCH v2] " Fabio Cesari
2026-09-07  7:19   ` Joshua Crofts
2026-09-07 11:02     ` Fabio Cesari

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®