linux-kernel - Re: [PATCH] HID: uclogic: Remove useless loop

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: <f2429c78-9189-410d-9c6a-644ae8a4d12c@gmail.com>
Date: Thu, 18 Apr 2024 20:04:11 +0300
From: Nikolai Kondrashov <spbnick@...il.com>
To: Stefan Berzl <stefanberzl@...il.com>, Jiri Kosina <jikos@...nel.org>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: uclogic: Remove useless loop

Hi Jiri, Stefan,

On 4/18/24 4:31 PM, Stefan Berzl wrote:
> 
> On 12/04/2024 17:52, Jiri Kosina wrote:
>> On Mon, 1 Apr 2024, Stefan Berzl wrote:
>>
>>> The while in question does nothing except provide the possibility
>>> to have an infinite loop in case the subreport id is actually the same
>>> as the pen id.
>>>
>>> Signed-off-by: Stefan Berzl <stefanberzl@...il.com>
>>
>> Let me CC Nicolai, the author of the code of question (8b013098be2c9).
> 
> I agree that Nicolai's opinion would be invaluable, but even without it,
> the patch is trivially correct. If we have a subreport that matches the
> packet, we change the report_id accordingly. If we then loop back to the
> beginning, either the report_id is different or we are caught in an
> infinite loop. None of these are hardware registers where the access
> itself would matter.

Yes, Stefan is right. I was trying to implement general rewrite logic, and if
we really had that, then the fix would need to be checking that the new ID is
different. As such there's really no need, and Stefan's fix is fine.

Only perhaps amend that comment to something like

    /* Change to the (non-pen) subreport ID, and continue */

Or at least remove ", and restart".

Thank you, Stefan and Jiri!
Nick