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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAO-hwJK3Q1Bu+6JVbXmbZ2-JqaPzX=dEVPfue=v2LRJsU7FmGA@mail.gmail.com>
Date:   Fri, 18 Oct 2019 17:38:14 +0200
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     Mazin Rezk <mnrzk@...tonmail.com>
Cc:     "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "jikos@...nel.org" <jikos@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Filipe LaĆ­ns <lains@...hlinux.org>
Subject: Re: [PATCH v6 2/2] HID: logitech: Support WirelessDeviceStatus
 connect events

On Mon, Oct 14, 2019 at 8:36 PM Mazin Rezk <mnrzk@...tonmail.com> wrote:
>
> This patch allows WirelessDeviceStatus (0x1d4b) events to be detected as
> connection events in the hid-logitech-hidpp module.
>
> Devices with HIDPP_QUIRK_WIRELESS_DEVICE_STATUS use WirelessDeviceStatus
> instead of traditional connect events. Since all Bluetooth LE devices seem
> to act this way, HIDPP_QUIRK_CLASS_BLUETOOTH_LE aliases this quirk.
>
> Signed-off-by: Mazin Rezk <mnrzk@...tonmail.com>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 42 ++++++++++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 997b1056850a..9b3df57ca857 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -60,6 +60,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
>  #define HIDPP_QUIRK_CLASS_K750                 BIT(4)
>
>  /* bits 2..15 are reserved for classes */
> +#define HIDPP_QUIRK_WIRELESS_DEVICE_STATUS     BIT(19)
>  #define HIDPP_QUIRK_MISSING_SHORT_REPORTS      BIT(20)
>  /* #define HIDPP_QUIRK_CONNECT_EVENTS          BIT(21) disabled */
>  #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS       BIT(22)
> @@ -82,7 +83,8 @@ MODULE_PARM_DESC(disable_tap_to_click,
>                                          HIDPP_QUIRK_HI_RES_SCROLL_X2120 | \
>                                          HIDPP_QUIRK_HI_RES_SCROLL_X2121)
>
> -#define HIDPP_QUIRK_CLASS_BLUETOOTH_LE HIDPP_QUIRK_MISSING_SHORT_REPORTS
> +#define HIDPP_QUIRK_CLASS_BLUETOOTH_LE (HIDPP_QUIRK_MISSING_SHORT_REPORTS | \
> +                                        HIDPP_QUIRK_WIRELESS_DEVICE_STATUS)
>
>  #define HIDPP_QUIRK_DELAYED_INIT               HIDPP_QUIRK_NO_HIDINPUT
>
> @@ -189,6 +191,8 @@ struct hidpp_device {
>
>         struct hidpp_battery battery;
>         struct hidpp_scroll_counter vertical_wheel_counter;
> +
> +       u8 wireless_feature_index;
>  };
>
>  /* HID++ 1.0 error codes */
> @@ -402,10 +406,14 @@ static inline bool hidpp_match_error(struct hidpp_report *question,
>             (answer->fap.params[0] == question->fap.funcindex_clientid);
>  }
>
> -static inline bool hidpp_report_is_connect_event(struct hidpp_report *report)
> +static inline bool hidpp_report_is_connect_event(struct hidpp_device *hidpp,
> +                                                struct hidpp_report *report)
>  {
> -       return (report->report_id == REPORT_ID_HIDPP_SHORT) &&
> -               (report->rap.sub_id == 0x41);
> +       return ((hidpp->quirks & HIDPP_QUIRK_WIRELESS_DEVICE_STATUS) &&
> +               (report->fap.feature_index == hidpp->wireless_feature_index)) ||
> +             (((report->report_id == REPORT_ID_HIDPP_SHORT) ||
> +               (hidpp->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS)) &&
> +               (report->rap.sub_id == 0x41));

I wonder if we need a quirk after all: if
hidpp->wireless_feature_index is non null, that means that we have the
quirk.
Unless the feature is present for non BLE devices, in which case, we
might want to enable them one by one, for now.

Cheers,
Benjamin

>  }
>
>  /**
> @@ -1282,6 +1290,24 @@ static int hidpp_battery_get_property(struct power_supply *psy,
>         return ret;
>  }
>
> +/* -------------------------------------------------------------------------- */
> +/* 0x1d4b: Wireless device status                                             */
> +/* -------------------------------------------------------------------------- */
> +#define HIDPP_PAGE_WIRELESS_DEVICE_STATUS                      0x1d4b
> +
> +static int hidpp_set_wireless_feature_index(struct hidpp_device *hidpp)
> +{
> +       u8 feature_type;
> +       int ret;
> +
> +       ret = hidpp_root_get_feature(hidpp,
> +                                    HIDPP_PAGE_WIRELESS_DEVICE_STATUS,
> +                                    &hidpp->wireless_feature_index,
> +                                    &feature_type);
> +
> +       return ret;
> +}
> +
>  /* -------------------------------------------------------------------------- */
>  /* 0x2120: Hi-resolution scrolling                                            */
>  /* -------------------------------------------------------------------------- */
> @@ -3077,7 +3103,7 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
>                 }
>         }
>
> -       if (unlikely(hidpp_report_is_connect_event(report))) {
> +       if (unlikely(hidpp_report_is_connect_event(hidpp, report))) {
>                 atomic_set(&hidpp->connected,
>                                 !(report->rap.params[0] & (1 << 6)));
>                 if (schedule_work(&hidpp->work) == 0)
> @@ -3624,6 +3650,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 hidpp_overwrite_name(hdev);
>         }
>
> +       if (connected && (hidpp->quirks & HIDPP_QUIRK_WIRELESS_DEVICE_STATUS)) {
> +               ret = hidpp_set_wireless_feature_index(hidpp);
> +               if (ret)
> +                       goto hid_hw_init_fail;
> +       }
> +
>         if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
>                 ret = wtp_get_config(hidpp);
>                 if (ret)
> --
> 2.23.0
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ