linux-kernel - Re: [PATCH] HID: i2c-hid: remove I2C_HID_READ

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] [day] [month] [year] [list]

Date: Thu, 21 Mar 2024 13:31:31 +0100 (CET)
From: Jiri Kosina <jikos@...nel.org>
To: Nam Cao <namcao@...utronix.de>
cc: 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, Eva Kurchatova <nyandarknessgirl@...il.com>, 
    stable@...r.kernel.org
Subject: Re: [PATCH] HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent
 lock-up

On Mon, 18 Mar 2024, Nam Cao wrote:

> The flag I2C_HID_READ_PENDING is used to serialize I2C operations.
> However, this is not necessary, because I2C core already has its own
> locking for that.
> 
> More importantly, this flag can cause a lock-up: if the flag is set in
> i2c_hid_xfer() and an interrupt happens, the interrupt handler
> (i2c_hid_irq) will check this flag and return immediately without doing
> anything, then the interrupt handler will be invoked again in an
> infinite loop.
> 
> Since interrupt handler is an RT task, it takes over the CPU and the
> flag-clearing task never gets scheduled, thus we have a lock-up.
> 
> Delete this unnecessary flag.

Hmm, right, good catch, I can't figure out what extra semantic 
I2C_HID_READ_PENDING would be adding (rather than deadlock :) ).
Why RT throttling didn't happen and let the system in a completely locked 
up state is something I don't understand, but that's separate.

I have now queued this in hid.git#for-6.9/upstream-fixes

Thanks,

-- 
Jiri Kosina
SUSE Labs