All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register
@ 2022-05-24 18:14 Dmitry Rokosov
  2022-05-24 18:14 ` [PATCH v2 1/5] iio:accel:bma180: rearrange iio trigger get and register Dmitry Rokosov
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-24 18:14 UTC (permalink / raw)
  To: robh+dt, jic23, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12
  Cc: linux-iio, kernel, linux-kernel, Dmitry Rokosov

The following patchset resolves problems with iio_trigger_get() and
iio_trigger_register() call order in the different IIO drivers.

IIO trigger interface function iio_trigger_get() should be called after
iio_trigger_register() (or its devm analogue) strictly, because of
iio_trigger_get() acquires module refcnt based on the trigger->owner
pointer, which is initialized inside iio_trigger_register() to
THIS_MODULE.
If this call order is wrong, the next iio_trigger_put() (from sysfs
callback or "delete module" path) will dereference "default" module
refcnt, which is incorrect behaviour.

Changes v1->v2:
    - provide tag Fixes: for all patches

Dmitry Rokosov (5):
  iio:accel:bma180: rearrange iio trigger get and register
  iio:accel:kxcjk-1013: rearrange iio trigger get and register
  iio:accel:mxc4005: rearrange iio trigger get and register
  iio:chemical:ccs811: rearrange iio trigger get and register
  iio:humidity:hts221: rearrange iio trigger get and register

 drivers/iio/accel/bma180.c           | 3 ++-
 drivers/iio/accel/kxcjk-1013.c       | 4 ++--
 drivers/iio/accel/mxc4005.c          | 4 ++--
 drivers/iio/chemical/ccs811.c        | 4 ++--
 drivers/iio/humidity/hts221_buffer.c | 5 ++++-
 5 files changed, 12 insertions(+), 8 deletions(-)

-- 
2.36.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/5] iio:accel:bma180: rearrange iio trigger get and register
  2022-05-24 18:14 [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Dmitry Rokosov
@ 2022-05-24 18:14 ` Dmitry Rokosov
  2022-05-24 19:53   ` Andy Shevchenko
  2022-05-24 18:14 ` [PATCH v2 2/5] iio:accel:kxcjk-1013: " Dmitry Rokosov
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-24 18:14 UTC (permalink / raw)
  To: robh+dt, jic23, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12
  Cc: linux-iio, kernel, linux-kernel, Dmitry Rokosov

IIO trigger interface function iio_trigger_get() should be called after
iio_trigger_register() (or its devm analogue) strictly, because of
iio_trigger_get() acquires module refcnt based on the trigger->owner
pointer, which is initialized inside iio_trigger_register() to
THIS_MODULE.
If this call order is wrong, the next iio_trigger_put() (from sysfs
callback or "delete module" path) will dereference "default" module
refcnt, which is incorrect behaviour.

Fixes: 0668a4e4d297 ("iio: accel: bma180: Fix indio_dev->trig assignment")
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/iio/accel/bma180.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
index d8a454c266d5..5d0bd0fc3018 100644
--- a/drivers/iio/accel/bma180.c
+++ b/drivers/iio/accel/bma180.c
@@ -1006,11 +1006,12 @@ static int bma180_probe(struct i2c_client *client,
 
 		data->trig->ops = &bma180_trigger_ops;
 		iio_trigger_set_drvdata(data->trig, indio_dev);
-		indio_dev->trig = iio_trigger_get(data->trig);
 
 		ret = iio_trigger_register(data->trig);
 		if (ret)
 			goto err_trigger_free;
+
+		indio_dev->trig = iio_trigger_get(data->trig);
 	}
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-- 
2.36.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/5] iio:accel:kxcjk-1013: rearrange iio trigger get and register
  2022-05-24 18:14 [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Dmitry Rokosov
  2022-05-24 18:14 ` [PATCH v2 1/5] iio:accel:bma180: rearrange iio trigger get and register Dmitry Rokosov
@ 2022-05-24 18:14 ` Dmitry Rokosov
  2022-05-24 19:49   ` Andy Shevchenko
  2022-05-24 18:14 ` [PATCH v2 3/5] iio:accel:mxc4005: " Dmitry Rokosov
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-24 18:14 UTC (permalink / raw)
  To: robh+dt, jic23, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12
  Cc: linux-iio, kernel, linux-kernel, Dmitry Rokosov

IIO trigger interface function iio_trigger_get() should be called after
iio_trigger_register() (or its devm analogue) strictly, because of
iio_trigger_get() acquires module refcnt based on the trigger->owner
pointer, which is initialized inside iio_trigger_register() to
THIS_MODULE.
If this call order is wrong, the next iio_trigger_put() (from sysfs
callback or "delete module" path) will dereference "default" module
refcnt, which is incorrect behaviour.

Fixes: c1288b833881 ("iio: accel: kxcjk-1013: Increment ref counter for indio_dev->trig")
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/iio/accel/kxcjk-1013.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index ac74cdcd2bc8..748b35c2f0c3 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1554,12 +1554,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
 
 		data->dready_trig->ops = &kxcjk1013_trigger_ops;
 		iio_trigger_set_drvdata(data->dready_trig, indio_dev);
-		indio_dev->trig = data->dready_trig;
-		iio_trigger_get(indio_dev->trig);
 		ret = iio_trigger_register(data->dready_trig);
 		if (ret)
 			goto err_poweroff;
 
+		indio_dev->trig = iio_trigger_get(data->dready_trig);
+
 		data->motion_trig->ops = &kxcjk1013_trigger_ops;
 		iio_trigger_set_drvdata(data->motion_trig, indio_dev);
 		ret = iio_trigger_register(data->motion_trig);
-- 
2.36.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/5] iio:accel:mxc4005: rearrange iio trigger get and register
  2022-05-24 18:14 [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Dmitry Rokosov
  2022-05-24 18:14 ` [PATCH v2 1/5] iio:accel:bma180: rearrange iio trigger get and register Dmitry Rokosov
  2022-05-24 18:14 ` [PATCH v2 2/5] iio:accel:kxcjk-1013: " Dmitry Rokosov
@ 2022-05-24 18:14 ` Dmitry Rokosov
  2022-05-24 19:50   ` Andy Shevchenko
  2022-05-24 18:14 ` [PATCH v2 4/5] iio:chemical:ccs811: " Dmitry Rokosov
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-24 18:14 UTC (permalink / raw)
  To: robh+dt, jic23, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12
  Cc: linux-iio, kernel, linux-kernel, Dmitry Rokosov

IIO trigger interface function iio_trigger_get() should be called after
iio_trigger_register() (or its devm analogue) strictly, because of
iio_trigger_get() acquires module refcnt based on the trigger->owner
pointer, which is initialized inside iio_trigger_register() to
THIS_MODULE.
If this call order is wrong, the next iio_trigger_put() (from sysfs
callback or "delete module" path) will dereference "default" module
refcnt, which is incorrect behaviour.

Fixes: 47196620c82f ("iio: mxc4005: add data ready trigger for mxc4005")
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/iio/accel/mxc4005.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
index b3afbf064915..df600d2917c0 100644
--- a/drivers/iio/accel/mxc4005.c
+++ b/drivers/iio/accel/mxc4005.c
@@ -456,8 +456,6 @@ static int mxc4005_probe(struct i2c_client *client,
 
 		data->dready_trig->ops = &mxc4005_trigger_ops;
 		iio_trigger_set_drvdata(data->dready_trig, indio_dev);
-		indio_dev->trig = data->dready_trig;
-		iio_trigger_get(indio_dev->trig);
 		ret = devm_iio_trigger_register(&client->dev,
 						data->dready_trig);
 		if (ret) {
@@ -465,6 +463,8 @@ static int mxc4005_probe(struct i2c_client *client,
 				"failed to register trigger\n");
 			return ret;
 		}
+
+		indio_dev->trig = iio_trigger_get(data->dready_trig);
 	}
 
 	return devm_iio_device_register(&client->dev, indio_dev);
-- 
2.36.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/5] iio:chemical:ccs811: rearrange iio trigger get and register
  2022-05-24 18:14 [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Dmitry Rokosov
                   ` (2 preceding siblings ...)
  2022-05-24 18:14 ` [PATCH v2 3/5] iio:accel:mxc4005: " Dmitry Rokosov
@ 2022-05-24 18:14 ` Dmitry Rokosov
  2022-05-24 19:51   ` Andy Shevchenko
  2022-05-24 18:14 ` [PATCH v2 5/5] iio:humidity:hts221: " Dmitry Rokosov
  2022-05-28 17:10 ` [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Jonathan Cameron
  5 siblings, 1 reply; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-24 18:14 UTC (permalink / raw)
  To: robh+dt, jic23, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12
  Cc: linux-iio, kernel, linux-kernel, Dmitry Rokosov

IIO trigger interface function iio_trigger_get() should be called after
iio_trigger_register() (or its devm analogue) strictly, because of
iio_trigger_get() acquires module refcnt based on the trigger->owner
pointer, which is initialized inside iio_trigger_register() to
THIS_MODULE.
If this call order is wrong, the next iio_trigger_put() (from sysfs
callback or "delete module" path) will dereference "default" module
refcnt, which is incorrect behaviour.

Fixes: f1f065d7ac30 ("iio: chemical: ccs811: Add support for data ready trigger")
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/iio/chemical/ccs811.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 847194fa1e46..80ef1aa9aae3 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -499,11 +499,11 @@ static int ccs811_probe(struct i2c_client *client,
 
 		data->drdy_trig->ops = &ccs811_trigger_ops;
 		iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
-		indio_dev->trig = data->drdy_trig;
-		iio_trigger_get(indio_dev->trig);
 		ret = iio_trigger_register(data->drdy_trig);
 		if (ret)
 			goto err_poweroff;
+
+		indio_dev->trig = iio_trigger_get(data->drdy_trig);
 	}
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-- 
2.36.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/5] iio:humidity:hts221: rearrange iio trigger get and register
  2022-05-24 18:14 [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Dmitry Rokosov
                   ` (3 preceding siblings ...)
  2022-05-24 18:14 ` [PATCH v2 4/5] iio:chemical:ccs811: " Dmitry Rokosov
@ 2022-05-24 18:14 ` Dmitry Rokosov
  2022-05-24 19:52   ` Andy Shevchenko
  2022-05-28 17:10 ` [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Jonathan Cameron
  5 siblings, 1 reply; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-24 18:14 UTC (permalink / raw)
  To: robh+dt, jic23, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12
  Cc: linux-iio, kernel, linux-kernel, Dmitry Rokosov

IIO trigger interface function iio_trigger_get() should be called after
iio_trigger_register() (or its devm analogue) strictly, because of
iio_trigger_get() acquires module refcnt based on the trigger->owner
pointer, which is initialized inside iio_trigger_register() to
THIS_MODULE.
If this call order is wrong, the next iio_trigger_put() (from sysfs
callback or "delete module" path) will dereference "default" module
refcnt, which is incorrect behaviour.

Fixes: e4a70e3e7d84 ("iio: humidity: add support to hts221 rh/temp combo device")
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/iio/humidity/hts221_buffer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index f29692b9d2db..66b32413cf5e 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -135,9 +135,12 @@ int hts221_allocate_trigger(struct iio_dev *iio_dev)
 
 	iio_trigger_set_drvdata(hw->trig, iio_dev);
 	hw->trig->ops = &hts221_trigger_ops;
+
+	err = devm_iio_trigger_register(hw->dev, hw->trig);
+
 	iio_dev->trig = iio_trigger_get(hw->trig);
 
-	return devm_iio_trigger_register(hw->dev, hw->trig);
+	return err;
 }
 
 static int hts221_buffer_preenable(struct iio_dev *iio_dev)
-- 
2.36.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/5] iio:accel:kxcjk-1013: rearrange iio trigger get and register
  2022-05-24 18:14 ` [PATCH v2 2/5] iio:accel:kxcjk-1013: " Dmitry Rokosov
@ 2022-05-24 19:49   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2022-05-24 19:49 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: robh+dt, jic23, lars, lorenzo.bianconi83, srinivas.pandruvada,
	teodora.baluta, narcisaanamaria12, linux-iio, kernel,
	linux-kernel

On Tue, May 24, 2022 at 8:14 PM Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:
>
> IIO trigger interface function iio_trigger_get() should be called after
> iio_trigger_register() (or its devm analogue) strictly, because of
> iio_trigger_get() acquires module refcnt based on the trigger->owner
> pointer, which is initialized inside iio_trigger_register() to
> THIS_MODULE.
> If this call order is wrong, the next iio_trigger_put() (from sysfs
> callback or "delete module" path) will dereference "default" module
> refcnt, which is incorrect behaviour.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: c1288b833881 ("iio: accel: kxcjk-1013: Increment ref counter for indio_dev->trig")
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  drivers/iio/accel/kxcjk-1013.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index ac74cdcd2bc8..748b35c2f0c3 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1554,12 +1554,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
>
>                 data->dready_trig->ops = &kxcjk1013_trigger_ops;
>                 iio_trigger_set_drvdata(data->dready_trig, indio_dev);
> -               indio_dev->trig = data->dready_trig;
> -               iio_trigger_get(indio_dev->trig);
>                 ret = iio_trigger_register(data->dready_trig);
>                 if (ret)
>                         goto err_poweroff;
>
> +               indio_dev->trig = iio_trigger_get(data->dready_trig);
> +
>                 data->motion_trig->ops = &kxcjk1013_trigger_ops;
>                 iio_trigger_set_drvdata(data->motion_trig, indio_dev);
>                 ret = iio_trigger_register(data->motion_trig);
> --
> 2.36.0



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] iio:accel:mxc4005: rearrange iio trigger get and register
  2022-05-24 18:14 ` [PATCH v2 3/5] iio:accel:mxc4005: " Dmitry Rokosov
@ 2022-05-24 19:50   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2022-05-24 19:50 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: robh+dt, jic23, lars, lorenzo.bianconi83, srinivas.pandruvada,
	teodora.baluta, narcisaanamaria12, linux-iio, kernel,
	linux-kernel

On Tue, May 24, 2022 at 8:14 PM Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:
>
> IIO trigger interface function iio_trigger_get() should be called after
> iio_trigger_register() (or its devm analogue) strictly, because of
> iio_trigger_get() acquires module refcnt based on the trigger->owner
> pointer, which is initialized inside iio_trigger_register() to
> THIS_MODULE.
> If this call order is wrong, the next iio_trigger_put() (from sysfs
> callback or "delete module" path) will dereference "default" module
> refcnt, which is incorrect behaviour.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: 47196620c82f ("iio: mxc4005: add data ready trigger for mxc4005")
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  drivers/iio/accel/mxc4005.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
> index b3afbf064915..df600d2917c0 100644
> --- a/drivers/iio/accel/mxc4005.c
> +++ b/drivers/iio/accel/mxc4005.c
> @@ -456,8 +456,6 @@ static int mxc4005_probe(struct i2c_client *client,
>
>                 data->dready_trig->ops = &mxc4005_trigger_ops;
>                 iio_trigger_set_drvdata(data->dready_trig, indio_dev);
> -               indio_dev->trig = data->dready_trig;
> -               iio_trigger_get(indio_dev->trig);
>                 ret = devm_iio_trigger_register(&client->dev,
>                                                 data->dready_trig);
>                 if (ret) {
> @@ -465,6 +463,8 @@ static int mxc4005_probe(struct i2c_client *client,
>                                 "failed to register trigger\n");
>                         return ret;
>                 }
> +
> +               indio_dev->trig = iio_trigger_get(data->dready_trig);
>         }
>
>         return devm_iio_device_register(&client->dev, indio_dev);
> --
> 2.36.0



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/5] iio:chemical:ccs811: rearrange iio trigger get and register
  2022-05-24 18:14 ` [PATCH v2 4/5] iio:chemical:ccs811: " Dmitry Rokosov
@ 2022-05-24 19:51   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2022-05-24 19:51 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: robh+dt, jic23, lars, lorenzo.bianconi83, srinivas.pandruvada,
	teodora.baluta, narcisaanamaria12, linux-iio, kernel,
	linux-kernel

On Tue, May 24, 2022 at 8:14 PM Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:
>
> IIO trigger interface function iio_trigger_get() should be called after
> iio_trigger_register() (or its devm analogue) strictly, because of
> iio_trigger_get() acquires module refcnt based on the trigger->owner
> pointer, which is initialized inside iio_trigger_register() to
> THIS_MODULE.
> If this call order is wrong, the next iio_trigger_put() (from sysfs
> callback or "delete module" path) will dereference "default" module
> refcnt, which is incorrect behaviour.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: f1f065d7ac30 ("iio: chemical: ccs811: Add support for data ready trigger")
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  drivers/iio/chemical/ccs811.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
> index 847194fa1e46..80ef1aa9aae3 100644
> --- a/drivers/iio/chemical/ccs811.c
> +++ b/drivers/iio/chemical/ccs811.c
> @@ -499,11 +499,11 @@ static int ccs811_probe(struct i2c_client *client,
>
>                 data->drdy_trig->ops = &ccs811_trigger_ops;
>                 iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
> -               indio_dev->trig = data->drdy_trig;
> -               iio_trigger_get(indio_dev->trig);
>                 ret = iio_trigger_register(data->drdy_trig);
>                 if (ret)
>                         goto err_poweroff;
> +
> +               indio_dev->trig = iio_trigger_get(data->drdy_trig);
>         }
>
>         ret = iio_triggered_buffer_setup(indio_dev, NULL,
> --
> 2.36.0



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/5] iio:humidity:hts221: rearrange iio trigger get and register
  2022-05-24 18:14 ` [PATCH v2 5/5] iio:humidity:hts221: " Dmitry Rokosov
@ 2022-05-24 19:52   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2022-05-24 19:52 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: robh+dt, jic23, lars, lorenzo.bianconi83, srinivas.pandruvada,
	teodora.baluta, narcisaanamaria12, linux-iio, kernel,
	linux-kernel

On Tue, May 24, 2022 at 8:14 PM Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:
>
> IIO trigger interface function iio_trigger_get() should be called after
> iio_trigger_register() (or its devm analogue) strictly, because of
> iio_trigger_get() acquires module refcnt based on the trigger->owner
> pointer, which is initialized inside iio_trigger_register() to
> THIS_MODULE.
> If this call order is wrong, the next iio_trigger_put() (from sysfs
> callback or "delete module" path) will dereference "default" module
> refcnt, which is incorrect behaviour.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: e4a70e3e7d84 ("iio: humidity: add support to hts221 rh/temp combo device")
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  drivers/iio/humidity/hts221_buffer.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
> index f29692b9d2db..66b32413cf5e 100644
> --- a/drivers/iio/humidity/hts221_buffer.c
> +++ b/drivers/iio/humidity/hts221_buffer.c
> @@ -135,9 +135,12 @@ int hts221_allocate_trigger(struct iio_dev *iio_dev)
>
>         iio_trigger_set_drvdata(hw->trig, iio_dev);
>         hw->trig->ops = &hts221_trigger_ops;
> +
> +       err = devm_iio_trigger_register(hw->dev, hw->trig);
> +
>         iio_dev->trig = iio_trigger_get(hw->trig);
>
> -       return devm_iio_trigger_register(hw->dev, hw->trig);
> +       return err;
>  }
>
>  static int hts221_buffer_preenable(struct iio_dev *iio_dev)
> --
> 2.36.0



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/5] iio:accel:bma180: rearrange iio trigger get and register
  2022-05-24 18:14 ` [PATCH v2 1/5] iio:accel:bma180: rearrange iio trigger get and register Dmitry Rokosov
@ 2022-05-24 19:53   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2022-05-24 19:53 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: robh+dt, jic23, lars, lorenzo.bianconi83, srinivas.pandruvada,
	teodora.baluta, narcisaanamaria12, linux-iio, kernel,
	linux-kernel

On Tue, May 24, 2022 at 8:14 PM Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:
>
> IIO trigger interface function iio_trigger_get() should be called after
> iio_trigger_register() (or its devm analogue) strictly, because of
> iio_trigger_get() acquires module refcnt based on the trigger->owner
> pointer, which is initialized inside iio_trigger_register() to
> THIS_MODULE.
> If this call order is wrong, the next iio_trigger_put() (from sysfs
> callback or "delete module" path) will dereference "default" module
> refcnt, which is incorrect behaviour.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: 0668a4e4d297 ("iio: accel: bma180: Fix indio_dev->trig assignment")
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  drivers/iio/accel/bma180.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> index d8a454c266d5..5d0bd0fc3018 100644
> --- a/drivers/iio/accel/bma180.c
> +++ b/drivers/iio/accel/bma180.c
> @@ -1006,11 +1006,12 @@ static int bma180_probe(struct i2c_client *client,
>
>                 data->trig->ops = &bma180_trigger_ops;
>                 iio_trigger_set_drvdata(data->trig, indio_dev);
> -               indio_dev->trig = iio_trigger_get(data->trig);
>
>                 ret = iio_trigger_register(data->trig);
>                 if (ret)
>                         goto err_trigger_free;
> +
> +               indio_dev->trig = iio_trigger_get(data->trig);
>         }
>
>         ret = iio_triggered_buffer_setup(indio_dev, NULL,
> --
> 2.36.0



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register
  2022-05-24 18:14 [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Dmitry Rokosov
                   ` (4 preceding siblings ...)
  2022-05-24 18:14 ` [PATCH v2 5/5] iio:humidity:hts221: " Dmitry Rokosov
@ 2022-05-28 17:10 ` Jonathan Cameron
  2022-05-30 15:29   ` Dmitry Rokosov
  2022-05-31 18:20   ` Dmitry Rokosov
  5 siblings, 2 replies; 14+ messages in thread
From: Jonathan Cameron @ 2022-05-28 17:10 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: robh+dt, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12,
	linux-iio, kernel, linux-kernel

On Tue, 24 May 2022 18:14:37 +0000
Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:

> The following patchset resolves problems with iio_trigger_get() and
> iio_trigger_register() call order in the different IIO drivers.
> 
> IIO trigger interface function iio_trigger_get() should be called after
> iio_trigger_register() (or its devm analogue) strictly, because of
> iio_trigger_get() acquires module refcnt based on the trigger->owner
> pointer, which is initialized inside iio_trigger_register() to
> THIS_MODULE.
> If this call order is wrong, the next iio_trigger_put() (from sysfs
> callback or "delete module" path) will dereference "default" module
> refcnt, which is incorrect behaviour.

Hi Dmitry,

Series applied to the fixes-togreg branch of iio.git and marked for stable.

Do you think it's also worth adding a runtime warning in iio_trigger_get()
on !trig->owner so that we catch any cases of this introduced in the future?

Thanks,

Jonathan

> 
> Changes v1->v2:
>     - provide tag Fixes: for all patches
> 
> Dmitry Rokosov (5):
>   iio:accel:bma180: rearrange iio trigger get and register
>   iio:accel:kxcjk-1013: rearrange iio trigger get and register
>   iio:accel:mxc4005: rearrange iio trigger get and register
>   iio:chemical:ccs811: rearrange iio trigger get and register
>   iio:humidity:hts221: rearrange iio trigger get and register
> 
>  drivers/iio/accel/bma180.c           | 3 ++-
>  drivers/iio/accel/kxcjk-1013.c       | 4 ++--
>  drivers/iio/accel/mxc4005.c          | 4 ++--
>  drivers/iio/chemical/ccs811.c        | 4 ++--
>  drivers/iio/humidity/hts221_buffer.c | 5 ++++-
>  5 files changed, 12 insertions(+), 8 deletions(-)
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register
  2022-05-28 17:10 ` [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Jonathan Cameron
@ 2022-05-30 15:29   ` Dmitry Rokosov
  2022-05-31 18:20   ` Dmitry Rokosov
  1 sibling, 0 replies; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-30 15:29 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: robh+dt, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12,
	linux-iio, kernel, linux-kernel

Jonathan,

On Sat, May 28, 2022 at 06:10:04PM +0100, Jonathan Cameron wrote:
> > If this call order is wrong, the next iio_trigger_put() (from sysfs
> > callback or "delete module" path) will dereference "default" module
> > refcnt, which is incorrect behaviour.
> 
> Hi Dmitry,
> 
> Series applied to the fixes-togreg branch of iio.git and marked for stable.
> 

Thank you!

> Do you think it's also worth adding a runtime warning in iio_trigger_get()
> on !trig->owner so that we catch any cases of this introduced in the future?
> 

Good point, it will help other kernel developers to avoid such mistake.
I'll prepare new patchset with that.

-- 
Thank you,
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register
  2022-05-28 17:10 ` [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Jonathan Cameron
  2022-05-30 15:29   ` Dmitry Rokosov
@ 2022-05-31 18:20   ` Dmitry Rokosov
  1 sibling, 0 replies; 14+ messages in thread
From: Dmitry Rokosov @ 2022-05-31 18:20 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: robh+dt, lars, andy.shevchenko, lorenzo.bianconi83,
	srinivas.pandruvada, teodora.baluta, narcisaanamaria12,
	linux-iio, kernel, linux-kernel

Jonathan,

I've submitted the patch with runtime WARN() as you suggested:

https://lore.kernel.org/linux-iio/20220531181457.26034-1-ddrokosov@sberdevices.ru/

On Sat, May 28, 2022 at 06:10:04PM +0100, Jonathan Cameron wrote:
> > If this call order is wrong, the next iio_trigger_put() (from sysfs
> > callback or "delete module" path) will dereference "default" module
> > refcnt, which is incorrect behaviour.
> 
> Hi Dmitry,
> 
> Series applied to the fixes-togreg branch of iio.git and marked for stable.
> 
> Do you think it's also worth adding a runtime warning in iio_trigger_get()
> on !trig->owner so that we catch any cases of this introduced in the future?
> 

-- 
Thank you,
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-05-31 18:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-24 18:14 [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Dmitry Rokosov
2022-05-24 18:14 ` [PATCH v2 1/5] iio:accel:bma180: rearrange iio trigger get and register Dmitry Rokosov
2022-05-24 19:53   ` Andy Shevchenko
2022-05-24 18:14 ` [PATCH v2 2/5] iio:accel:kxcjk-1013: " Dmitry Rokosov
2022-05-24 19:49   ` Andy Shevchenko
2022-05-24 18:14 ` [PATCH v2 3/5] iio:accel:mxc4005: " Dmitry Rokosov
2022-05-24 19:50   ` Andy Shevchenko
2022-05-24 18:14 ` [PATCH v2 4/5] iio:chemical:ccs811: " Dmitry Rokosov
2022-05-24 19:51   ` Andy Shevchenko
2022-05-24 18:14 ` [PATCH v2 5/5] iio:humidity:hts221: " Dmitry Rokosov
2022-05-24 19:52   ` Andy Shevchenko
2022-05-28 17:10 ` [PATCH v2 0/5] iio: treewide: rearrange iio trig get/register Jonathan Cameron
2022-05-30 15:29   ` Dmitry Rokosov
2022-05-31 18:20   ` Dmitry Rokosov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.