iio: potentiostat: lmp91000: fix NULL deref in probe by reordering setup

[PATCH] iio: potentiostat: lmp91000: fix NULL deref in probe by reordering setup

Posted by Stepan Ionichev 4 days, 2 hours ago

lmp91000_probe() calls iio_trigger_set_immutable() with
iio_channel_cb_get_iio_dev(data->cb_buffer) before data->cb_buffer is
assigned. The struct is zero-initialised by devm_iio_device_alloc(), so
cb_buffer is NULL on entry, and iio_channel_cb_get_iio_dev() does an
unconditional cb_buffer->indio_dev which dereferences NULL.

Reorder probe to acquire cb_buffer first (handling -EPROBE_DEFER) and
only then set the immutable trigger, register the trigger, set up the
triggered buffer, and register the iio device. Move the cb_buffer
release to the end of the cleanup chain so a late failure properly
unwinds in reverse order.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/iio/potentiostat/lmp91000.c | 36 +++++++++++++----------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index eccc2a343..7a938a023 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -330,17 +330,27 @@ static int lmp91000_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	data->cb_buffer = iio_channel_get_all_cb(dev, &lmp91000_buffer_cb,
+						 indio_dev);
+	if (IS_ERR(data->cb_buffer)) {
+		if (PTR_ERR(data->cb_buffer) == -ENODEV)
+			return -EPROBE_DEFER;
+		return PTR_ERR(data->cb_buffer);
+	}
+
+	data->adc_chan = iio_channel_cb_get_channels(data->cb_buffer);
+
 	ret = iio_trigger_set_immutable(iio_channel_cb_get_iio_dev(data->cb_buffer),
 					data->trig);
 	if (ret) {
 		dev_err(dev, "cannot set immutable trigger.\n");
-		return ret;
+		goto error_release_cb;
 	}
 
 	ret = iio_trigger_register(data->trig);
 	if (ret) {
 		dev_err(dev, "cannot register iio trigger.\n");
-		return ret;
+		goto error_release_cb;
 	}
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
@@ -349,35 +359,21 @@ static int lmp91000_probe(struct i2c_client *client)
 	if (ret)
 		goto error_unreg_trigger;
 
-	data->cb_buffer = iio_channel_get_all_cb(dev, &lmp91000_buffer_cb,
-						 indio_dev);
-
-	if (IS_ERR(data->cb_buffer)) {
-		if (PTR_ERR(data->cb_buffer) == -ENODEV)
-			ret = -EPROBE_DEFER;
-		else
-			ret = PTR_ERR(data->cb_buffer);
-
-		goto error_unreg_buffer;
-	}
-
-	data->adc_chan = iio_channel_cb_get_channels(data->cb_buffer);
-
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_unreg_cb_buffer;
+		goto error_unreg_buffer;
 
 	return 0;
 
-error_unreg_cb_buffer:
-	iio_channel_release_all_cb(data->cb_buffer);
-
 error_unreg_buffer:
 	iio_triggered_buffer_cleanup(indio_dev);
 
 error_unreg_trigger:
 	iio_trigger_unregister(data->trig);
 
+error_release_cb:
+	iio_channel_release_all_cb(data->cb_buffer);
+
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH] iio: potentiostat: lmp91000: fix NULL deref in probe by reordering setup

Posted by Stepan Ionichev 1 day, 7 hours ago

Sorry for the duplication -- I'll drop this one. Looking forward to
Salah's version.

Stepan

Re: [PATCH] iio: potentiostat: lmp91000: fix NULL deref in probe by reordering setup

Posted by Jonathan Cameron 2 days, 6 hours ago

On Wed, 20 May 2026 23:51:41 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> lmp91000_probe() calls iio_trigger_set_immutable() with
> iio_channel_cb_get_iio_dev(data->cb_buffer) before data->cb_buffer is
> assigned. The struct is zero-initialised by devm_iio_device_alloc(), so
> cb_buffer is NULL on entry, and iio_channel_cb_get_iio_dev() does an
> unconditional cb_buffer->indio_dev which dereferences NULL.
> 
> Reorder probe to acquire cb_buffer first (handling -EPROBE_DEFER) and
> only then set the immutable trigger, register the trigger, set up the
> triggered buffer, and register the iio device. Move the cb_buffer
> release to the end of the cleanup chain so a late failure properly
> unwinds in reverse order.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>

Hi Stephan

Make sure to check the list before sending out a patch.
+CC Salah who is working on a fix for the same thing.

Jonathan

> ---
>  drivers/iio/potentiostat/lmp91000.c | 36 +++++++++++++----------------
>  1 file changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> index eccc2a343..7a938a023 100644
> --- a/drivers/iio/potentiostat/lmp91000.c
> +++ b/drivers/iio/potentiostat/lmp91000.c
> @@ -330,17 +330,27 @@ static int lmp91000_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;
>  
> +	data->cb_buffer = iio_channel_get_all_cb(dev, &lmp91000_buffer_cb,
> +						 indio_dev);
> +	if (IS_ERR(data->cb_buffer)) {
> +		if (PTR_ERR(data->cb_buffer) == -ENODEV)
> +			return -EPROBE_DEFER;
> +		return PTR_ERR(data->cb_buffer);
> +	}
> +
> +	data->adc_chan = iio_channel_cb_get_channels(data->cb_buffer);
> +
>  	ret = iio_trigger_set_immutable(iio_channel_cb_get_iio_dev(data->cb_buffer),
>  					data->trig);
>  	if (ret) {
>  		dev_err(dev, "cannot set immutable trigger.\n");
> -		return ret;
> +		goto error_release_cb;
>  	}
>  
>  	ret = iio_trigger_register(data->trig);
>  	if (ret) {
>  		dev_err(dev, "cannot register iio trigger.\n");
> -		return ret;
> +		goto error_release_cb;
>  	}
>  
>  	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> @@ -349,35 +359,21 @@ static int lmp91000_probe(struct i2c_client *client)
>  	if (ret)
>  		goto error_unreg_trigger;
>  
> -	data->cb_buffer = iio_channel_get_all_cb(dev, &lmp91000_buffer_cb,
> -						 indio_dev);
> -
> -	if (IS_ERR(data->cb_buffer)) {
> -		if (PTR_ERR(data->cb_buffer) == -ENODEV)
> -			ret = -EPROBE_DEFER;
> -		else
> -			ret = PTR_ERR(data->cb_buffer);
> -
> -		goto error_unreg_buffer;
> -	}
> -
> -	data->adc_chan = iio_channel_cb_get_channels(data->cb_buffer);
> -
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> -		goto error_unreg_cb_buffer;
> +		goto error_unreg_buffer;
>  
>  	return 0;
>  
> -error_unreg_cb_buffer:
> -	iio_channel_release_all_cb(data->cb_buffer);
> -
>  error_unreg_buffer:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  
>  error_unreg_trigger:
>  	iio_trigger_unregister(data->trig);
>  
> +error_release_cb:
> +	iio_channel_release_all_cb(data->cb_buffer);
> +
>  	return ret;
>  }
>