[PATCH 0/2] iio: accel: kxsd9: fix use-after-free and PM leaks

[PATCH 0/2] iio: accel: kxsd9: fix use-after-free and PM leaks

[Biren Pandya]

This series fixes a use-after-free during device removal and resolves 
multiple runtime PM reference leaks.

Changes in v3:
- Split the fixes into two patches (UAF fix and PM leaks fix) as 
  requested by Jonathan Cameron.
- remove(): Dropped the early return on PM resume failure. The driver now 
  makes a best-effort attempt to power down the device unconditionally, 
  addressing feedback from Andy Shevchenko and Jonathan Cameron.
- read_raw(): Mirrored the -EINVAL reset fix from write_raw() to ensure 
  symmetric error handling on invalid masks.
- Dropped redundant pm_runtime_mark_last_busy() calls, relying instead
  on pm_runtime_put_autosuspend().

Link to v2: https://lore.kernel.org/linux-iio/20260621193036.78549-2-birenpandya@gmail.com/

---
Biren Pandya (2):
      iio: accel: kxsd9: fix use-after-free on remove
      iio: accel: kxsd9: fix runtime PM leaks and unchecked returns

 drivers/iio/accel/kxsd9.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)
---
base-commit: 7de6ae9e12207ec146f2f3f1e58d1a99317e88bc
change-id: 20260703-kxsd9-v3-proper-51a1f05c4951

Best regards,
--  
Biren Pandya <birenpandya@gmail.com>

[PATCH 1/2] iio: accel: kxsd9: fix use-after-free on 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: 9a9a369d6178 ("iio: accel: kxsd9: Deploy system and runtime PM")
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 7ac885d94d7f4..27adcdd312014 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -478,8 +478,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);

-- 
Git-155)

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

[Biren Pandya]

The driver fails to check the return value of pm_runtime_get_sync(),
leading to potential silent failures. Additionally, error paths leak the
runtime PM usage counter by returning without decrementing it.

Fix this by transitioning to pm_runtime_resume_and_get():

- In kxsd9_write_raw() and kxsd9_read_raw(), check the resume status and
  guarantee pm_runtime_put_autosuspend() is called on all return paths.
  Simultaneously, refactor the control flow to align with standard IIO
  idioms (removing the dead -EINVAL initializer in favor of explicit
  assignments).
- In kxsd9_buffer_preenable(), propagate the resume error code.
- In kxsd9_common_remove(), only decrement the usage counter on a
  successful resume, while retaining the unconditional power-down for a
  best-effort shutdown.

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

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 27adcdd312014..e9a0790e2eea2 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -139,18 +139,17 @@ 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);
+	ret = pm_runtime_resume_and_get(st->dev);
+	if (ret < 0)
+		return ret;
 
-	if (mask == IIO_CHAN_INFO_SCALE) {
-		/* Check no integer component */
-		if (val)
-			ret = -EINVAL;
-		else
-			ret = kxsd9_write_scale(indio_dev, val2);
-	}
+	if (mask == IIO_CHAN_INFO_SCALE && !val)
+		ret = kxsd9_write_scale(indio_dev, val2);
+	else
+		ret = -EINVAL;
 
 	pm_runtime_put_autosuspend(st->dev);
 
@@ -161,20 +160,22 @@ 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);
+	ret = pm_runtime_resume_and_get(st->dev);
+	if (ret < 0)
+		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;
+			break;
 		nval = be16_to_cpu(raw_val);
 		/* Only 12 bits are valid */
 		nval >>= 4;
@@ -191,18 +192,20 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 				  KXSD9_REG_CTRL_C,
 				  &regval);
 		if (ret < 0)
-			goto error_ret;
+			break;
 		*val = 0;
 		*val2 = kxsd9_micro_scales[regval & KXSD9_CTRL_C_FS_MASK];
 		ret = IIO_VAL_INT_PLUS_MICRO;
 		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
 
-error_ret:
 	pm_runtime_put_autosuspend(st->dev);
 
 	return ret;
-};
+}
 
 static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
 {
@@ -240,9 +243,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)
@@ -480,8 +481,9 @@ void kxsd9_common_remove(struct device *dev)
 
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
-	pm_runtime_get_sync(dev);
-	pm_runtime_put_noidle(dev);
+
+	if (pm_runtime_resume_and_get(dev) >= 0)
+		pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
 	kxsd9_power_down(st);
 }

-- 
Git-155)

Re: [PATCH 0/2] iio: accel: kxsd9: fix use-after-free and PM leaks

[Jonathan Cameron]

On Fri, 03 Jul 2026 22:53:21 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> This series fixes a use-after-free during device removal and resolves 
> multiple runtime PM reference leaks.
> 
> Changes in v3:
> - Split the fixes into two patches (UAF fix and PM leaks fix) as 
>   requested by Jonathan Cameron.
> - remove(): Dropped the early return on PM resume failure. The driver now 
>   makes a best-effort attempt to power down the device unconditionally, 
>   addressing feedback from Andy Shevchenko and Jonathan Cameron.
> - read_raw(): Mirrored the -EINVAL reset fix from write_raw() to ensure 
>   symmetric error handling on invalid masks.
> - Dropped redundant pm_runtime_mark_last_busy() calls, relying instead
>   on pm_runtime_put_autosuspend().
> 
> Link to v2: https://lore.kernel.org/linux-iio/20260621193036.78549-2-birenpandya@gmail.com/

[PATCH v3 0/2] etc

Don't resend for this but please check future series for missing versions
before sending out.

Jonathan

> 
> ---
> Biren Pandya (2):
>       iio: accel: kxsd9: fix use-after-free on remove
>       iio: accel: kxsd9: fix runtime PM leaks and unchecked returns
> 
>  drivers/iio/accel/kxsd9.c | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
> ---
> base-commit: 7de6ae9e12207ec146f2f3f1e58d1a99317e88bc
> change-id: 20260703-kxsd9-v3-proper-51a1f05c4951
> 
> Best regards,
> --  
> Biren Pandya <birenpandya@gmail.com>
> 

Re: [PATCH 1/2] iio: accel: kxsd9: fix use-after-free on remove

[Jonathan Cameron]

On Fri, 03 Jul 2026 22:53:22 +0530
Biren Pandya <birenpandya@gmail.com> 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
> Fixes: 9a9a369d6178 ("iio: accel: kxsd9: Deploy system and runtime PM")
That tag touches the pm runtime stuff just below, but nothing to do with the
bug reported here.

Should be:
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 7ac885d94d7f4..27adcdd312014 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -478,8 +478,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);
> 

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

[Jonathan Cameron]

On Fri, 03 Jul 2026 22:53:23 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> The driver fails to check the return value of pm_runtime_get_sync(),
> leading to potential silent failures. Additionally, error paths leak the
> runtime PM usage counter by returning without decrementing it.
> 
> Fix this by transitioning to pm_runtime_resume_and_get():
> 
> - In kxsd9_write_raw() and kxsd9_read_raw(), check the resume status and
>   guarantee pm_runtime_put_autosuspend() is called on all return paths.
>   Simultaneously, refactor the control flow to align with standard IIO
>   idioms (removing the dead -EINVAL initializer in favor of explicit
>   assignments).
> - In kxsd9_buffer_preenable(), propagate the resume error code.
> - In kxsd9_common_remove(), only decrement the usage counter on a
>   successful resume, while retaining the unconditional power-down for a
>   best-effort shutdown.
> 
> Fixes: 9a9a369d6178 ("iio: accel: kxsd9: Deploy system and runtime PM")
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>
> ---
>  drivers/iio/accel/kxsd9.c | 42 ++++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 27adcdd312014..e9a0790e2eea2 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -139,18 +139,17 @@ 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);
> +	ret = pm_runtime_resume_and_get(st->dev);
> +	if (ret < 0)
> +		return ret;
>  
> -	if (mask == IIO_CHAN_INFO_SCALE) {
> -		/* Check no integer component */
> -		if (val)
> -			ret = -EINVAL;
> -		else
> -			ret = kxsd9_write_scale(indio_dev, val2);
> -	}
> +	if (mask == IIO_CHAN_INFO_SCALE && !val)
This is a mix of checking for two very different conditions. I'd rather
they were handled separately as before - that will require you to do
	if (mask == IIO_CHAN_INFO_SCALE) {
		if (val)
			ret = -EINVAL;
		else
			ret = kxsd9_write_scale(indio_dev, val2);
	} else {
		ret = -EINVAL;

Given how ugly this is we should probably switch this to the ACQUIRE stuff.
Maybe it would be better to just do that as part of this fix rather
than as a follow up.  PM_RUNTIME_ACQUIRE_AUTOSUSPEND() and
PM_RUNTIME_ACQUIRE_ERR()

Then this can all be eaerly returns.
	if (mask != IIO_CHAN_INFO_SCALE)
		return -EINVAL;

	if (val)
		return -EINVAL;

	return kxsd9_write_scale();


	}
> +		ret = kxsd9_write_scale(indio_dev, val2);
> +	else
> +		ret = -EINVAL;
>  
>  	pm_runtime_put_autosuspend(st->dev);
>  
> @@ -161,20 +160,22 @@ 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);
> +	ret = pm_runtime_resume_and_get(st->dev);
> +	if (ret < 0)
> +		return ret;

Similar to above, switching to the ACQUIRE magic will simplify this
flow a lot as well as dealing with resource leaks.

>  
>  	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;
> +			break;
>  		nval = be16_to_cpu(raw_val);
>  		/* Only 12 bits are valid */
>  		nval >>= 4;
> @@ -191,18 +192,20 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  				  KXSD9_REG_CTRL_C,
>  				  &regval);
>  		if (ret < 0)
> -			goto error_ret;
> +			break;
>  		*val = 0;
>  		*val2 = kxsd9_micro_scales[regval & KXSD9_CTRL_C_FS_MASK];
>  		ret = IIO_VAL_INT_PLUS_MICRO;
>  		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
>  	}
>  
> -error_ret:
>  	pm_runtime_put_autosuspend(st->dev);
>  
>  	return ret;
> -};
> +}
>  
>  static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
>  {
> @@ -240,9 +243,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)
> @@ -480,8 +481,9 @@ void kxsd9_common_remove(struct device *dev)
>  
>  	iio_device_unregister(indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
> -	pm_runtime_get_sync(dev);
> -	pm_runtime_put_noidle(dev);
> +
> +	if (pm_runtime_resume_and_get(dev) >= 0)
> +		pm_runtime_put_noidle(dev);

I'd just let it underflow in this case.   Trying to cleanly 
handle errors in a driver unbind is rarely worth the effort
and despite what Sashiko thinks, runtime pm underflow isn't
a real problem - it's just messy and generally indicative
of something being less than ideal.

Jonathan

>  	pm_runtime_disable(dev);
>  	kxsd9_power_down(st);
>  }
>