From: Thomas Dettbarn <dettus@dettus.net>
Subject: Re: hidmt: separate contact inputs correctly
To: Ulf Brosziewski <ulf.brosziewski@t-online.de>
Cc: "tech@openbsd.org" <tech@openbsd.org>
Date: Fri, 27 Jun 2025 10:39:44 +0200

Thank you Ulf!

Please find my patch at the end of this email.


Thomas


On Thu, Jun 26, 2025 at 10:22:03PM +0200, Ulf Brosziewski wrote:
> Thomas Dettbarn has found, analyzed und reported two bugs in imt(4) (see
>     https://marc.info/?l=openbsd-tech&m=174841257723328&w=2
> and the related messages).
> ...
> (The second bug is in the *_find_winptp_reports functions in imt(4)
> and umt(4), which match wrong report IDs under certain conditions.
> Another patch will follow soon; it might be the short workaround from
> Thomas - see the imt.c part of his patch - or, perhaps, a more
> systematic solution.  I haven't made up my mind yet.)

And here is the short workaround: (Ulf helped me a lot in refining it.)

 
This patch reduces the amount of false positives in the functions 
imt_find_winptp_reports() and umt_find_winptp_reports() (in imt.c
and umt.c respectively).
The report which is detected as the Input report now requires not
only to be in the Physical Collection DIGITIZERS/TOUCHPAD; in 
addition to this, it now has to contain the usages for coordinates
GENERIC_DESKTOP/HUGX and GENERIC_DESKTOP/HUG_Y as well. 
The report for the configuration is now the one which is not only
in DIGITIZERS/HUD_CONFIG, buut also has DIGITIZERS/HUD_INPUT_MODE.

Side note: Even though this patch consists of two diffs, both of
them are independant. So feel free to split them up.


diff --git sys/dev/i2c/imt.c sys/dev/i2c/imt.c
index 5dcdbed57e9..f287674c832 100644
--- sys/dev/i2c/imt.c
+++ sys/dev/i2c/imt.c
@@ -116,12 +116,18 @@ imt_find_winptp_reports(struct ihidev_softc *parent, void *desc, int size,
 			continue;
 
 		if (hid_is_collection(desc, size, repid,
-		    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD))) {
+		    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD)) &&
+		    hid_locate(desc, size, HID_USAGE2(HUP_GENERIC_DESKTOP,
+		    HUG_X), repid, hid_input, NULL, NULL) &&
+		    hid_locate(desc, size, HID_USAGE2(HUP_GENERIC_DESKTOP,
+		    HUG_Y), repid, hid_input, NULL, NULL)) {
 			input = 1;
 			if (sc != NULL && sc->sc_rep_input == -1)
 				sc->sc_rep_input = repid;
 		} else if (hid_is_collection(desc, size, repid,
-		    HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG))) {
+		    HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG)) &&
+		    hid_locate(desc, size, HID_USAGE2(HUP_DIGITIZERS,
+		    HUD_INPUT_MODE), repid, hid_feature, NULL, NULL)) {
 			conf = 1;
 			if (sc != NULL && sc->sc_rep_config == -1)
 				sc->sc_rep_config = repid;
diff --git sys/dev/usb/umt.c sys/dev/usb/umt.c
index 35ebfb59668..0712fdc10bc 100644
--- sys/dev/usb/umt.c
+++ sys/dev/usb/umt.c
@@ -121,12 +121,18 @@ umt_find_winptp_reports(struct uhidev_softc *parent, void *desc, int size,
 			continue;
 
 		if (hid_is_collection(desc, size, repid,
-		    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD))) {
+		    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD)) &&
+		    hid_locate(desc, size, HID_USAGE2(HUP_GENERIC_DESKTOP,
+		    HUG_X), repid, hid_input, NULL, NULL) &&
+		    hid_locate(desc, size, HID_USAGE2(HUP_GENERIC_DESKTOP,
+		    HUG_Y), repid, hid_input, NULL, NULL)) {
 			finput = 1;
 			if (input != NULL && *input == -1)
 				*input = repid;
 		} else if (hid_is_collection(desc, size, repid,
-		    HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG))) {
+		    HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG)) &&
+		    hid_locate(desc, size, HID_USAGE2(HUP_DIGITIZERS,
+		    HUD_INPUT_MODE), repid, hid_feature, NULL, NULL)) {
 			fconf = 1;
 			if (config != NULL && *config == -1)
 				*config = repid;