* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).