linux-kernel - Re: [PATCH v2 3/4] iio: hid-sensor-trigger: Use iio->trig instead trig field internal structure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201121173330.3afd1087@archlinux>
Date:   Sat, 21 Nov 2020 17:33:30 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Ye Xiang <xiang.ye@...el.com>
Cc:     jikos@...nel.org, srinivas.pandruvada@...ux.intel.com,
        linux-input@...r.kernel.org, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] iio: hid-sensor-trigger: Use iio->trig instead
 trig field internal structure

On Thu, 19 Nov 2020 18:03:30 +0800
Ye Xiang <xiang.ye@...el.com> wrote:

> Use iio->trig instead of attrb->trig as parameter of iio_trigger_unregister
> and iio_trigger_free. This allows one HID sensor driver to create
> multiple iio devices. In this case common attributes are shared and
> there can be one instance for the structure containing common attributes
> for all iio devices.
> 
> Signed-off-by: Ye Xiang <xiang.ye@...el.com>
Whilst indio_dev->trig is set to the local trigger at startup,
I'm not seeing the validate callbacks that will be needed to ensure it 
is still set to that trigger when you remove the driver.

Specifically validate_trigger callback for all the hid sensor devices.

It is entirely possible that should be set and these devices can only
use the hid sensor trigger, but I don't think it currently is.

Thus this patch is going to cause some interesting crashes.
Note this is the reason all IIO drivers have to carry an extra copy
of the trig pointer.

So you'll have to do something a bit more interesting to pass that
trigger to the relevant devices that share it.

Arguably if they are actually sharing the trigger, then we shouldn't really
have separate IIO devices in the first place as we loose the
connection between the timing of the data across the different channels.

Jonathan

> ---
>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index 30340abcbc8d..bb5e7c8ff15b 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -236,8 +236,8 @@ void hid_sensor_remove_trigger(struct iio_dev *indio_dev,
>  	pm_runtime_put_noidle(&attrb->pdev->dev);
>  
>  	cancel_work_sync(&attrb->work);
> -	iio_trigger_unregister(attrb->trigger);
> -	iio_trigger_free(attrb->trigger);
> +	iio_trigger_unregister(indio_dev->trig);
> +	iio_trigger_free(indio_dev->trig);

>  	iio_triggered_buffer_cleanup(indio_dev);
>  }
>  EXPORT_SYMBOL(hid_sensor_remove_trigger);