[PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Fabio Cesari]

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@xxxxxxxxxxxxxxx
Assisted-by: Claude:claude-opus-5 coccinelle
Signed-off-by: Fabio Cesari <fabio.cesari@xxxxxxxxx>
---

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