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

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

[Fabio Cesari]

isl29028_read_raw() and isl29028_write_raw() take a runtime PM reference
with pm_runtime_resume_and_get() but return directly on their error
paths without dropping it. The usage count never balances again and the
device stops entering autosuspend for good. In isl29028_read_raw() this
needs a regmap access to fail; in isl29028_write_raw() one rejected
sysfs write is enough, for example

  echo 200 > in_proximity_sampling_frequency

which is outside the [1:100] range and returns -EINVAL with the
reference still held.

Take the reference with PM_RUNTIME_ACQUIRE_AUTOSUSPEND() instead, so it
is released on every return path.

This also stops the return value of pm_runtime_put_autosuspend() from
reaching userspace. That value only says whether the device could be
suspended right away, so -EAGAIN or -EPERM turns a successful access
into a failure, and with CONFIG_PM=n the stub returns -ENOSYS on every
access.

PM_RUNTIME_ACQUIRE_AUTOSUSPEND() exists since v6.19. Older trees need
the manual form instead: keep pm_runtime_resume_and_get() and drop the
reference on all paths with an unchecked pm_runtime_put_autosuspend().

Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
Suggested-by: Joshua Crofts <joshua.crofts1@gmail.com>
Cc: <stable@vger.kernel.org> # see patch description, needs adjustments for < 6.19
Assisted-by: LLM coccinelle
Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
---

Changes in v3, from the review of v2:
  - use PM_RUNTIME_ACQUIRE_AUTOSUSPEND() rather than the _IF_ENABLED_
    variant
  - sent as its own thread rather than as a reply to v2

v1: https://lore.kernel.org/linux-iio/20260906131203.125407-1-fabio.cesari@gmail.com/
v2: https://lore.kernel.org/linux-iio/20260906223737.206730-1-fabio.cesari@gmail.com/

Found by auditing IIO drivers with a Coccinelle semantic patch for
runtime PM acquire/release imbalances.

Compile-tested only: arm64 (native) and x86_64 (cross), defconfig plus
CONFIG_SENSORS_ISL29028=m, plus an arm64 CONFIG_PM=n build to cover the
stubs, 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.

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

diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
index 33deb1726689..e481ac908fc1 100644
--- a/drivers/iio/light/isl29028.c
+++ b/drivers/iio/light/isl29028.c
@@ -342,8 +342,9 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 	struct device *dev = regmap_get_device(chip->regmap);
 	int ret;
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret < 0)
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
 		return ret;
 
 	mutex_lock(&chip->lock);
@@ -392,14 +393,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->lock);
 
-	if (ret < 0)
-		return ret;
-
-	ret = pm_runtime_put_autosuspend(dev);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return ret;
 }
 
 static int isl29028_read_raw(struct iio_dev *indio_dev,
@@ -408,10 +402,11 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 {
 	struct isl29028_chip *chip = iio_priv(indio_dev);
 	struct device *dev = regmap_get_device(chip->regmap);
-	int ret, pm_ret;
+	int ret;
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret < 0)
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
 		return ret;
 
 	mutex_lock(&chip->lock);
@@ -461,18 +456,6 @@ 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 (pm_ret < 0)
-		return pm_ret;
-
 	return ret;
 }
 

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.53.0

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

[Joshua Crofts]

On Thu, 10 Sep 2026 08:24:35 +0200
Fabio Cesari <fabio.cesari@gmail.com> wrote:

> isl29028_read_raw() and isl29028_write_raw() take a runtime PM reference
> with pm_runtime_resume_and_get() but return directly on their error
> paths without dropping it. The usage count never balances again and the
> device stops entering autosuspend for good. In isl29028_read_raw() this
> needs a regmap access to fail; in isl29028_write_raw() one rejected
> sysfs write is enough, for example
> 
>   echo 200 > in_proximity_sampling_frequency
> 
> which is outside the [1:100] range and returns -EINVAL with the
> reference still held.
> 
> Take the reference with PM_RUNTIME_ACQUIRE_AUTOSUSPEND() instead, so it
> is released on every return path.
> 
> This also stops the return value of pm_runtime_put_autosuspend() from
> reaching userspace. That value only says whether the device could be
> suspended right away, so -EAGAIN or -EPERM turns a successful access
> into a failure, and with CONFIG_PM=n the stub returns -ENOSYS on every
> access.
> 
> PM_RUNTIME_ACQUIRE_AUTOSUSPEND() exists since v6.19. Older trees need
> the manual form instead: keep pm_runtime_resume_and_get() and drop the
> reference on all paths with an unchecked pm_runtime_put_autosuspend().
> 
> Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
> Suggested-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Cc: <stable@vger.kernel.org> # see patch description, needs adjustments for < 6.19
> Assisted-by: LLM coccinelle
> Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
> ---

LGTM now.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards,
Joshua Crofts