linux-kernel - Re: [PATCH v3 1/3] HID: i2c-hid: Rely on HID descriptor fetch to probe

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: <5aa9f745-7f6a-4873-90ba-79c55335905c@kl.wtf>
Date: Sat, 27 Apr 2024 15:20:07 +0200
From: Kenny Levinsen <kl@...wtf>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: Jiri Kosina <jikos@...nel.org>, Dmitry Torokhov <dtor@...omium.org>,
 Benjamin Tissoires <benjamin.tissoires@...hat.com>,
 Douglas Anderson <dianders@...omium.org>, Hans de Goede
 <hdegoede@...hat.com>, Maxime Ripard <mripard@...nel.org>,
 Kai-Heng Feng <kai.heng.feng@...onical.com>,
 Johan Hovold <johan+linaro@...nel.org>, linux-input@...r.kernel.org,
 linux-kernel@...r.kernel.org, Radoslaw Biernacki <rad@...omium.org>,
 Lukasz Majczak <lma@...omium.org>
Subject: Re: [PATCH v3 1/3] HID: i2c-hid: Rely on HID descriptor fetch to
 probe

On 4/27/24 5:21 AM, Dmitry Torokhov wrote:
> I really think we should differentiate the cases "we do not know if
> there is a device" vs "we do known that there is a device and we have
> strong expectation of what that device is, and we do not expect
> communication to fail".

My reasoning was that there is no difference between looking for address 
acknowledge on a probe read vs. a real command. Unfortunately, I ran 
into some issues with error code consistency that Doug highlighted...

Considering that the smbus probe bails on *any* error, it's really only 
ACK'd address + NACK'd register that remains, and I thought it maybe 
wouldn't be too harmful to just always have a debug log as suggested. 
However, I would still like *more* good errors by being specific about 
the error condition, and I plan to send some patches to get the number 
of drivers sending ENXIO up so we can comfortably rely on it in a future 
i2c-hid patch.

If you don't think it's acceptable to leave this as a pure debug print 
for now, I'll send a patch with just a minor clean-up and Łukasz' delays 
- then I'll try this again later when the driver situation has improved. 
I've been rapid-firing revisions, so I'll await an ACK this time... :)

---

For some context, I started looking at the i2c-hid driver after a recent 
regression around assumed Windows behavior, and found that the actual 
behavior differed a lot from our assumptions. Windows gets the job done 
notably quicker, with fewer messages and with shorter albeit differently 
placed delays.

My plan is to send patches that clean up and aligns our traffic more, 
speeding things up and hopefully deprecating some quirks. To that end, I 
would also like to get rid of the dummy read once we're comfortable with it.