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

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(-)
>

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

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