linux-kernel - Re: [PATCH] HID: i2c-hid: override HID descriptors for some Haptick 5288 touchpads

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: <aV6d5mt2veL-vEvf@anonymous>
Date: Thu, 8 Jan 2026 02:14:29 +0800
From: kenkinming2002@...il.com
To: Benjamin Tissoires <bentiss@...nel.org>
Cc: jikos@...nel.org, linux-input@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: i2c-hid: override HID descriptors for some Haptick
 5288 touchpads

On Wed, Jan 07, 2026 at 06:45:45PM +0100, Benjamin Tissoires wrote:
> I'm really not found of this patch.
Me neither and would be happy with a cleaner solution.

> I'm really not found of this patch. AFAICT, from the archlinux bug, the
> device is presenting itself to HID, and we "just" have a truncated
> report descriptor. I'm not sure if you can trigger that bug at the HID
> descriptor level, without scripting it (so in real case scenarios).
At least I can trigger it, because I have an full disk encryption setup
and my theory is that I might unintentionally touch the trackpad while
typing the password. That would explain how I come across the bug.

> The simplest "solution" following what you are doing is making a HID-BPF
> fixup which checks whether the device properly sent the report
> descriptor and if not puts the one here. The HID-BPF has the advantage
> of being compatible with hid-multitouch so you won't get into troubles
> with a separate module.
This might be a solution but would that not only fix it just for me? I
would have to look into how to do HID-BPF fixup.

> The proper solution should be to detect those situations. But you
> mentioned above that the detection of the interrupts wasn't working.
> Could you tell us more?
This is the diff that is lying in my filesystem. I do not see a nice way
to do it without any race. The call to msleep(1) is just a best effort
to make sure any interrupt that have fired would be seen but there is no
guarantee. Now that I think about it, there might be some memory
visiblity issue with multicore but I am not sure.

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index f9b9dd0d9bb8..161eb8301763 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -66,6 +66,7 @@
 /* flags */
 #define I2C_HID_STARTED		0
 #define I2C_HID_RESET_PENDING	1
+#define I2C_HID_IRQ_HANDLED     2

 #define I2C_HID_PWR_ON		0x00
 #define I2C_HID_PWR_SLEEP	0x01
@@ -219,7 +220,25 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
 		n++;
 	}

+	// On some devices, what we received will may become corrupted if we
+	// are also supposed to receive input reports asynchronously at the
+	// same time, persumably because the hardware buffer in the device is
+	// shared.
+	clear_bit(I2C_HID_IRQ_HANDLED, &ihid->flags);
 	ret = i2c_transfer(client->adapter, msgs, n);
+	if(recv_len != 0)
+		while(ret == n)
+		{
+			msleep(1);
+			if(!test_and_clear_bit(I2C_HID_IRQ_HANDLED, &ihid->flags))
+				break;
+
+			i2c_hid_dbg(ihid, "%s: interrupted transfer: res=%*ph\n",
+					__func__, recv_len, recv_buf);
+
+			clear_bit(I2C_HID_IRQ_HANDLED, &ihid->flags);
+			ret = i2c_transfer(client->adapter, msgs, n);
+		}

 	if (ret != n)
 		return ret < 0 ? ret : -EIO;
@@ -585,6 +604,7 @@ static irqreturn_t i2c_hid_irq(int irq, void *dev_id)
 {
 	struct i2c_hid *ihid = dev_id;

+	set_bit(I2C_HID_IRQ_HANDLED, &ihid->flags);
 	i2c_hid_get_input(ihid);

 	return IRQ_HANDLED;

Simply disable and enabling interrupt would not work since we would be
just masking the interrupt on the cpu side. In fact, that is another fix
that I have attempted:

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 63f46a2e5..28ba480e4 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -219,7 +219,9 @@ static int i2c_hid_xfer(struct i2c_hid *ihid,
 		n++;
 	}

+	disable_irq(client->irq);
 	ret = i2c_transfer(client->adapter, msgs, n);
+	enable_irq(client->irq);

 	if (ret != n)
 		return ret < 0 ? ret : -EIO;

> I'd very much not have a report descriptor written in stone in the
> kernel when the device returns a correct one. Especially not at the
> i2c-hid level (I would be happier with a HID module, but this might
> break mutltiouch functionality).
Same, which is why I am asking for suggestion to alternative fixes.

Yours sincerly,
Ken Kwok