linux-kernel - Re: [PATCH] HID: logitech-hidpp: rework one more time the retries attempts

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]

Date:   Wed, 21 Jun 2023 12:50:21 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:     Filipe Laíns <lains@...eup.net>,
        Bastien Nocera <hadess@...ess.net>,
        Jiri Kosina <jikos@...nel.org>, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] HID: logitech-hidpp: rework one more time the retries
 attempts

On Wed, Jun 21, 2023 at 11:42:30AM +0200, Benjamin Tissoires wrote:
> Make the code looks less like Pascal.
> 
> Extract the internal code inside a helper function, fix the
> initialization of the parameters used in the helper function
> (`hidpp->answer_available` was not reset and `*response` wasn't too),
> and use a `do {...} while();` loop.
> 
> Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
> Cc: stable@...r.kernel.org
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> ---
> as requested by https://lore.kernel.org/all/CAHk-=wiMbF38KCNhPFiargenpSBoecSXTLQACKS2UMyo_Vu2ww@mail.gmail.com/
> This is a rewrite of that particular piece of code.
> ---
>  drivers/hid/hid-logitech-hidpp.c | 102 +++++++++++++++++++++++----------------
>  1 file changed, 61 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index dfe8e09a18de..3d1ffe199f08 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -275,21 +275,20 @@ static int __hidpp_send_report(struct hid_device *hdev,
>  }
>  
>  /*
> - * hidpp_send_message_sync() returns 0 in case of success, and something else
> - * in case of a failure.
> - * - If ' something else' is positive, that means that an error has been raised
> - *   by the protocol itself.
> - * - If ' something else' is negative, that means that we had a classic error
> - *   (-ENOMEM, -EPIPE, etc...)
> + * Effectively send the message to the device, waiting for its answer.
> + *
> + * Must be called with hidpp->send_mutex locked
> + *
> + * Same return protocol than hidpp_send_message_sync():
> + * - success on 0
> + * - negative error means transport error
> + * - positive value means protocol error
>   */
> -static int hidpp_send_message_sync(struct hidpp_device *hidpp,
> +static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp,
>  	struct hidpp_report *message,
>  	struct hidpp_report *response)

__must_hold(&hidpp->send_mutex)  ?