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]

Date: Tue, 30 Apr 2024 14:41:31 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Kenny Levinsen <kl@...wtf>
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 Sat, Apr 27, 2024 at 03:20:07PM +0200, Kenny Levinsen wrote:
> 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...

I actually believe there is. On Chromebooks we may source components
from several vendors and use them in our devices. The components
are electrically compatible with each other, have exactly the same
connector, and therefore interchangeable. Because of that at probe time
we do not quite know if the device is there at given address, or not
(i.e. the touchpad could be from a different vendor and listening on
another address) and we need to make a quick determination whether we
should continue with probe or not.

Once we decided that the probe should continue we commit to it and all
subsequent errors from the device should be treated as unexpected errors
worthy of logging. On resume we do not expect hardware configuration to
change, so again when resuming we also treat errors as errors and log
them and fail resume.

> 
> 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.

I am not sure we can fully unify what Windows does and what Linux does,
mainly because our firmwares are different (I think Windows devices do a
lot more device discovery in firmware, Chrome OS historically tried to
limit amount of code in its firmware). We also need to make sure it
works on non-ACPI systems/ARM.

Thanks.

-- 
Dmitry