[PATCH v3 1/2] iio: accel: kxsd9: fix Use-After-Free in remove()

[PATCH v3 1/2] iio: accel: kxsd9: fix Use-After-Free in remove()

[Biren Pandya]

The kxsd9 driver currently calls iio_triggered_buffer_cleanup() before
iio_device_unregister() in the remove() function. This order creates a
race condition where userspace can still access sysfs or ioctl interfaces
while the triggered buffers are being torn down, potentially leading to
a use-after-free.

Fix this by swapping the cleanup order. Unregister the IIO device first
to guarantee that all userspace interfaces are destroyed and no new
accesses can occur before cleaning up the triggered buffers.

This vulnerability was flagged by the Sashiko automated review system.

Link: https://sashiko.dev/#/patchset/20260621193036.78549-2-birenpandya@gmail.com
Fixes: 0427a106a98a ("iio: accel: kxsd9: Add triggered buffer handling")
Cc: stable@vger.kernel.org
Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
 drivers/iio/accel/kxsd9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 4717d80fc24af..7569201ed3c75 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -477,8 +477,8 @@ void kxsd9_common_remove(struct device *dev)
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct kxsd9_state *st = iio_priv(indio_dev);
 
-	iio_triggered_buffer_cleanup(indio_dev);
 	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
 	pm_runtime_get_sync(dev);
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
-- 
2.50.1 (Apple Git-155)

[PATCH v3 2/2] iio: accel: kxsd9: fix runtime PM leaks and unchecked returns

[Biren Pandya]

The kxsd9 driver previously used manual pm_runtime_get_sync() and
pm_runtime_put_autosuspend() calls in various return paths, which is
prone to memory leaks if a return path is missed. Specifically, in
kxsd9_write_raw(), if the user provides a non-zero integer component
for scale, the function previously returned -EINVAL directly, leaking
the runtime PM usage counter.

Switch to using the new scoped-based PM_RUNTIME_ACQUIRE_AUTOSUSPEND()
and PM_RUNTIME_ACQUIRE_ERR() macros to automatically handle dropping
the PM usage counter upon return. This simplifies the logic by allowing
early returns in error paths.

In remove(), use pm_runtime_resume_and_get() with an error check to
prevent unchecked return issues, and perform a best-effort power down.

Fixes: 9a9a369d6178 ("iio: accel: kxsd9: Deploy system and runtime PM")
Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
 drivers/iio/accel/kxsd9.c | 59 ++++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 7569201ed3c75..a71b473cbfa03 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -139,68 +139,65 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
 			   int val2,
 			   long mask)
 {
-	int ret = -EINVAL;
 	struct kxsd9_state *st = iio_priv(indio_dev);
+	int ret;
 
-	pm_runtime_get_sync(st->dev);
+	if (mask != IIO_CHAN_INFO_SCALE)
+		return -EINVAL;
 
-	if (mask == IIO_CHAN_INFO_SCALE) {
-		/* Check no integer component */
-		if (val)
-			return -EINVAL;
-		ret = kxsd9_write_scale(indio_dev, val2);
-	}
+	if (val)
+		return -EINVAL;
 
-	pm_runtime_put_autosuspend(st->dev);
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(st->dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
 
-	return ret;
+	return kxsd9_write_scale(indio_dev, val2);
 }
 
 static int kxsd9_read_raw(struct iio_dev *indio_dev,
 			  struct iio_chan_spec const *chan,
 			  int *val, int *val2, long mask)
 {
-	int ret = -EINVAL;
 	struct kxsd9_state *st = iio_priv(indio_dev);
 	unsigned int regval;
 	__be16 raw_val;
 	u16 nval;
+	int ret;
 
-	pm_runtime_get_sync(st->dev);
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(st->dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		ret = regmap_bulk_read(st->map, chan->address, &raw_val,
 				       sizeof(raw_val));
 		if (ret)
-			goto error_ret;
+			return ret;
 		nval = be16_to_cpu(raw_val);
 		/* Only 12 bits are valid */
 		nval >>= 4;
 		*val = nval;
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
 		/* This has a bias of -2048 */
 		*val = KXSD9_ZERO_G_OFFSET;
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		ret = regmap_read(st->map,
 				  KXSD9_REG_CTRL_C,
 				  &regval);
 		if (ret < 0)
-			goto error_ret;
+			return ret;
 		*val = 0;
 		*val2 = kxsd9_micro_scales[regval & KXSD9_CTRL_C_FS_MASK];
-		ret = IIO_VAL_INT_PLUS_MICRO;
-		break;
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 
-error_ret:
-	pm_runtime_put_autosuspend(st->dev);
-
-	return ret;
+	return -EINVAL;
 };
 
 static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
@@ -239,9 +236,7 @@ static int kxsd9_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct kxsd9_state *st = iio_priv(indio_dev);
 
-	pm_runtime_get_sync(st->dev);
-
-	return 0;
+	return pm_runtime_resume_and_get(st->dev);
 }
 
 static int kxsd9_buffer_postdisable(struct iio_dev *indio_dev)
@@ -476,12 +471,18 @@ void kxsd9_common_remove(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct kxsd9_state *st = iio_priv(indio_dev);
+	int ret;
 
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
-	pm_runtime_get_sync(dev);
-	pm_runtime_put_noidle(dev);
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		dev_warn(dev, "Failed to resume device (%d)\n", ret);
+
 	pm_runtime_disable(dev);
+	if (ret >= 0)
+		pm_runtime_put_noidle(dev);
 	kxsd9_power_down(st);
 }
 EXPORT_SYMBOL_NS(kxsd9_common_remove, "IIO_KXSD9");
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3 1/2] iio: accel: kxsd9: fix Use-After-Free in remove()

[Andy Shevchenko]

On Mon, Jul 06, 2026 at 01:16:51PM +0530, Biren Pandya wrote:
> The kxsd9 driver currently calls iio_triggered_buffer_cleanup() before
> iio_device_unregister() in the remove() function. This order creates a
> race condition where userspace can still access sysfs or ioctl interfaces
> while the triggered buffers are being torn down, potentially leading to
> a use-after-free.
> 
> Fix this by swapping the cleanup order. Unregister the IIO device first
> to guarantee that all userspace interfaces are destroyed and no new
> accesses can occur before cleaning up the triggered buffers.
> 
> This vulnerability was flagged by the Sashiko automated review system.

> Link: https://sashiko.dev/#/patchset/20260621193036.78549-2-birenpandya@gmail.com

I believe you wanted Closes: and Reported-by: tags instead of simple Link.

> Fixes: 0427a106a98a ("iio: accel: kxsd9: Add triggered buffer handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>

...

You have a series, where is the cover letter and the changelog, please?
Do not abuse the process.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] iio: accel: kxsd9: fix runtime PM leaks and unchecked returns

[Andy Shevchenko]

On Mon, Jul 06, 2026 at 01:16:52PM +0530, Biren Pandya wrote:
> The kxsd9 driver previously used manual pm_runtime_get_sync() and
> pm_runtime_put_autosuspend() calls in various return paths, which is
> prone to memory leaks if a return path is missed. Specifically, in
> kxsd9_write_raw(), if the user provides a non-zero integer component
> for scale, the function previously returned -EINVAL directly, leaking
> the runtime PM usage counter.
> 
> Switch to using the new scoped-based PM_RUNTIME_ACQUIRE_AUTOSUSPEND()
> and PM_RUNTIME_ACQUIRE_ERR() macros to automatically handle dropping
> the PM usage counter upon return. This simplifies the logic by allowing
> early returns in error paths.
> 
> In remove(), use pm_runtime_resume_and_get() with an error check to
> prevent unchecked return issues, and perform a best-effort power down.

...

> void kxsd9_common_remove(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	iio_device_unregister(indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
> -	pm_runtime_get_sync(dev);
> -	pm_runtime_put_noidle(dev);
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret < 0)
> +		dev_warn(dev, "Failed to resume device (%d)\n", ret);
> +
>  	pm_runtime_disable(dev);
> +	if (ret >= 0)
> +		pm_runtime_put_noidle(dev);

Has this been discussed already? Because it looks to me a bit strange to have
_disable() be called before _put_noidle(). Also the commit message is unclear
about the flow change.

>  	kxsd9_power_down(st);
>  }

-- 
With Best Regards,
Andy Shevchenko