Index | Thread | Search

From:
Ulf Brosziewski <ulf.brosziewski@t-online.de>
Subject:
imt(4)/umt(4): finding HID report IDs
To:
"tech@openbsd.org" <tech@openbsd.org>
Date:
Wed, 2 Jul 2025 01:37:08 +0200

Download raw body.

Thread
The way imt(4) and umt(4) search for report IDs is broken.  Depending
on the structure and distribution of IDs in a report descriptor, it might
succeed, or fail with wrong ID values (as has been reported by Thomas
Dettbarn, see https://marc.info/?l=openbsd-tech&m=174841257723328&w=2 ).

The hid_*_collection functions only provide partial information, which,
in isolation, may be useless if you aren't lucky and deal with simple,
uniform descriptors.

The patch adds another helper function to hid.c, which permits searching
for several report items at once, and filters out "unknown" collections
(see the comment in the diff).  It doesn't put further constraints on the
search and the structure of the descriptor, but the heuristic might be
safe enough for most practical purposes.  However, my experience with the
real HID world is limited, comments and hints would be appreciated.

The patch moves the search for "PTP" reports to hidmt, and adapts imt
and umt accordingly.  Please note that it also contains a fix for another
bug in hidmt (see https://marc.info/?l=openbsd-tech&m=175101436027656&w=2 ).
It may be hidden by the faulty report search; that's why for tests, it
makes sense to combine the patches.

Tests, comments, and OKs would be welcome.


Index: dev/hid/hid.c
===================================================================
RCS file: /cvs/src/sys/dev/hid/hid.c,v
retrieving revision 1.7
diff -u -p -r1.7 hid.c
--- dev/hid/hid.c	10 May 2024 10:49:10 -0000	1.7
+++ dev/hid/hid.c	1 Jul 2025 20:11:57 -0000
@@ -706,3 +706,70 @@ hid_get_id_of_collection(const void *des
 	hid_end_parse(hd);
 	return -1;
 }
+
+/*
+ * Find the first report that contains each of the given "usages"
+ * and belongs to a top-level collection with the 'top_usage' type.
+ * The size of the 'usages' array must be in the range [1..32].
+ *
+ * If 'coll_usages' is NULL, the search will skip collections with
+ * usages from vendor pages (0xFF00 - 0xFFFF).
+ *
+ * If 'coll_usages' is non-NULL, it must point to a 0-terminated
+ * sequence of collection usages, and the search will skip collections
+ * with usages not present in this set. (It isn't necessary to include
+ * the top-level usage here).
+ *
+ * Return Values:
+ *     -1:		No match
+ *     0:		Success (single report without an ID)
+ *     [1..255]:	Report ID
+ */
+int
+hid_find_report(const void *desc, int len, enum hid_kind kind,
+    int32_t top_usage, int n_usages, int32_t *usages, int32_t *coll_usages)
+{
+	struct hid_data *hd;
+	struct hid_item h;
+	uint32_t matches;
+	int i, cur_id, skip;
+
+	hd = hid_start_parse(desc, len, hid_all);
+	for (cur_id = -1, skip = 0;  hid_get_item(hd, &h); ) {
+		if (cur_id != h.report_ID) {
+			matches = 0;
+			cur_id = h.report_ID;
+		}
+		if (h.kind == hid_collection) {
+			if (skip)
+				continue;
+			if (h.collevel == 1) {
+			       if (h.usage != top_usage)
+					skip = 1;
+			} else if (coll_usages != NULL) {
+				for (i = 0; coll_usages[i] != h.usage; i++)
+					if (coll_usages[i] == 0) {
+						skip = h.collevel;
+						break;
+					}
+			} else if (((h.usage >> 16) & 0xffff) >= 0xff00) {
+				skip = h.collevel;
+			}
+		} else if (h.kind == hid_endcollection) {
+			if (h.collevel < skip)
+				skip = 0;
+		}
+		if (h.kind != kind || skip)
+			continue;
+		for (i = 0; i < n_usages; i++)
+			if (h.usage == usages[i] && !(matches & (1 << i))) {
+				matches |= (1 << i);
+				if (matches != (1 << n_usages) - 1)
+					break;
+				hid_end_parse(hd);
+				return (h.report_ID);
+			}
+	}
+	hid_end_parse(hd);
+	return (-1);
+}
Index: dev/hid/hid.h
===================================================================
RCS file: /cvs/src/sys/dev/hid/hid.h,v
retrieving revision 1.11
diff -u -p -r1.11 hid.h
--- dev/hid/hid.h	12 Aug 2023 20:47:06 -0000	1.11
+++ dev/hid/hid.h	1 Jul 2025 20:11:57 -0000
@@ -95,6 +95,8 @@ uint32_t hid_get_udata(const uint8_t *bu
 int	hid_is_collection(const void *, int, uint8_t, int32_t);
 struct hid_data *hid_get_collection_data(const void *, int, int32_t, uint32_t);
 int	hid_get_id_of_collection(const void *, int, int32_t, uint32_t);
+int	hid_find_report(const void *, int len, enum hid_kind, int32_t,
+	    int, int32_t *, int32_t *);

 #endif /* _KERNEL */

Index: dev/hid/hidmt.c
===================================================================
RCS file: /cvs/src/sys/dev/hid/hidmt.c,v
retrieving revision 1.13
diff -u -p -r1.13 hidmt.c
--- dev/hid/hidmt.c	16 Oct 2022 20:17:08 -0000	1.13
+++ dev/hid/hidmt.c	1 Jul 2025 20:11:57 -0000
@@ -42,6 +42,9 @@
 #define DPRINTF(x)
 #endif

+#define IS_TOP_LEVEL_USAGE(u) ((((u) >> 16) & 0xffff) == HUP_BUTTON || \
+    (u) == HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT))
+
 #define HID_UNIT_CM	0x11
 #define HID_UNIT_INCH	0x13

@@ -373,7 +376,7 @@ hidmt_input(struct hidmt *mt, uint8_t *d

 			bzero(&hc, sizeof(struct hidmt_contact));
 		}
-		else if (!firstu)
+		else if (!firstu && !IS_TOP_LEVEL_USAGE(hi->usage))
 			firstu = hi->usage;

 		switch (hi->usage) {
@@ -530,4 +533,41 @@ void
 hidmt_disable(struct hidmt *mt)
 {
 	mt->sc_enabled = 0;
+}
+
+int
+hidmt_find_winptp_reports(const void *desc, int len, int *input_rid,
+    int *config_rid, int *cap_rid)
+{
+	static int32_t ptp_collections[] = {
+		HID_USAGE2(HUP_DIGITIZERS, HUD_FINGER), 0
+	};
+	static int32_t input_usages[] = {
+		/* top-level */
+		HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT),
+		/* contact-level */
+		HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X),
+		HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y),
+		HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIDENCE),
+		HID_USAGE2(HUP_DIGITIZERS, HUD_TIP_SWITCH),
+		HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTID),
+	};
+	static int32_t cfg_usages[] = {
+		HID_USAGE2(HUP_DIGITIZERS, HUD_INPUT_MODE),
+	};
+	static int32_t cap_usages[] = {
+		HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACT_MAX),
+	};
+
+	*input_rid = hid_find_report(desc, len, hid_input,
+	    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD),
+	    nitems(input_usages), input_usages, ptp_collections);
+	*config_rid = hid_find_report(desc, len, hid_feature,
+	    HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG),
+	    nitems(cfg_usages), cfg_usages, ptp_collections);
+	*cap_rid = hid_find_report(desc, len, hid_feature,
+	    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD),
+	    nitems(cap_usages), cap_usages, ptp_collections);
+
+	return (*input_rid > 0 && *config_rid > 0 && *cap_rid > 0);
 }
Index: dev/hid/hidmtvar.h
===================================================================
RCS file: /cvs/src/sys/dev/hid/hidmtvar.h,v
retrieving revision 1.8
diff -u -p -r1.8 hidmtvar.h
--- dev/hid/hidmtvar.h	8 Nov 2019 01:20:22 -0000	1.8
+++ dev/hid/hidmtvar.h	1 Jul 2025 20:11:57 -0000
@@ -77,3 +77,4 @@ int	hidmt_enable(struct hidmt *);
 void	hidmt_input(struct hidmt *, uint8_t *, u_int);
 int	hidmt_ioctl(struct hidmt *, u_long, caddr_t, int, struct proc *);
 int	hidmt_setup(struct device *, struct hidmt *, void *, int);
+int	hidmt_find_winptp_reports(const void *, int, int *, int *, int *);
Index: dev/i2c/imt.c
===================================================================
RCS file: /cvs/src/sys/dev/i2c/imt.c,v
retrieving revision 1.6
diff -u -p -r1.6 imt.c
--- dev/i2c/imt.c	13 May 2024 01:15:50 -0000	1.6
+++ dev/i2c/imt.c	1 Jul 2025 20:11:57 -0000
@@ -56,8 +56,6 @@ const struct wsmouse_accessops imt_acces
 };

 int	imt_match(struct device *, void *, void *);
-int	imt_find_winptp_reports(struct ihidev_softc *, void *, int,
-	    struct imt_softc *);
 void	imt_attach(struct device *, struct device *, void *);
 int	imt_hidev_get_report(struct device *, int, int, void *, int);
 int	imt_hidev_set_report(struct device *, int, int, void *, int);
@@ -78,16 +76,17 @@ int
 imt_match(struct device *parent, void *match, void *aux)
 {
 	struct ihidev_attach_arg *iha = (struct ihidev_attach_arg *)aux;
-	struct imt_softc sc;
+	int input_rid, conf_rid, cap_rid;
 	int size;
 	void *desc;

 	if (iha->reportid == IHIDEV_CLAIM_MULTIPLEID) {
 		ihidev_get_report_desc(iha->parent, &desc, &size);
-		if (imt_find_winptp_reports(iha->parent, desc, size, &sc)) {
-			iha->claims[0] = sc.sc_rep_input;
-			iha->claims[1] = sc.sc_rep_config;
-			iha->claims[2] = sc.sc_rep_cap;
+		if (hidmt_find_winptp_reports(desc, size,
+		    &input_rid, &conf_rid, &cap_rid)) {
+			iha->claims[0] = input_rid;
+			iha->claims[1] = conf_rid;
+			iha->claims[2] = cap_rid;
 			iha->nclaims = 3;
 			return (IMATCH_DEVCLASS_DEVSUBCLASS);
 		}
@@ -96,49 +95,6 @@ imt_match(struct device *parent, void *m
 	return (IMATCH_NONE);
 }

-int
-imt_find_winptp_reports(struct ihidev_softc *parent, void *desc, int size,
-    struct imt_softc *sc)
-{
-	int repid;
-	int input = 0, conf = 0, cap = 0;
-
-	if (sc != NULL) {
-		sc->sc_rep_input = -1;
-		sc->sc_rep_config = -1;
-		sc->sc_rep_cap = -1;
-	}
-
-	for (repid = 0; repid < parent->sc_nrepid; repid++) {
-		if (hid_report_size(desc, size, hid_input, repid) == 0 &&
-		    hid_report_size(desc, size, hid_output, repid) == 0 &&
-		    hid_report_size(desc, size, hid_feature, repid) == 0)
-			continue;
-
-		if (hid_is_collection(desc, size, repid,
-		    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD))) {
-			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))) {
-			conf = 1;
-			if (sc != NULL && sc->sc_rep_config == -1)
-				sc->sc_rep_config = repid;
-		}
-
-		/* capabilities report could be anywhere */
-		if (hid_locate(desc, size, HID_USAGE2(HUP_DIGITIZERS,
-		    HUD_CONTACT_MAX), repid, hid_feature, NULL, NULL)) {
-			cap = 1;
-			if (sc != NULL && sc->sc_rep_cap == -1)
-				sc->sc_rep_cap = repid;
-		}
-	}
-
-	return (conf && input && cap);
-}
-
 void
 imt_attach(struct device *parent, struct device *self, void *aux)
 {
@@ -152,7 +108,8 @@ imt_attach(struct device *parent, struct
 	sc->sc_hdev.sc_parent = iha->parent;

 	ihidev_get_report_desc(iha->parent, &desc, &size);
-	imt_find_winptp_reports(iha->parent, desc, size, sc);
+	hidmt_find_winptp_reports(desc, size, &sc->sc_rep_input,
+	    &sc->sc_rep_config, &sc->sc_rep_cap);

 	memset(mt, 0, sizeof(sc->sc_mt));

Index: dev/usb/umt.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/umt.c,v
retrieving revision 1.8
diff -u -p -r1.8 umt.c
--- dev/usb/umt.c	26 May 2024 20:06:27 -0000	1.8
+++ dev/usb/umt.c	1 Jul 2025 20:11:57 -0000
@@ -60,8 +60,6 @@ const struct wsmouse_accessops umt_acces
 };

 int	umt_match(struct device *, void *, void *);
-int	umt_find_winptp_reports(struct uhidev_softc *, void *, int, int *,
-	    int *, int *);
 void	umt_attach(struct device *, struct device *, void *);
 int	umt_hidev_get_report(struct device *, int, int, void *, int);
 int	umt_hidev_set_report(struct device *, int, int, void *, int);
@@ -88,7 +86,7 @@ umt_match(struct device *parent, void *m

 	if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha)) {
 		uhidev_get_report_desc(uha->parent, &desc, &size);
-		if (umt_find_winptp_reports(uha->parent, desc, size, &input,
+		if (hidmt_find_winptp_reports(desc, size, &input,
 		    &conf, &cap)) {
 			uha->claimed[input] = 1;
 			uha->claimed[conf] = 1;
@@ -100,50 +98,6 @@ umt_match(struct device *parent, void *m
 	return (UMATCH_NONE);
 }

-int
-umt_find_winptp_reports(struct uhidev_softc *parent, void *desc, int size,
-    int *input, int *config, int *cap)
-{
-	int repid;
-	int finput = 0, fconf = 0, fcap = 0;
-
-	if (input != NULL)
-		*input = -1;
-	if (config != NULL)
-		*config = -1;
-	if (cap != NULL)
-		*cap = -1;
-
-	for (repid = 0; repid < parent->sc_nrepid; repid++) {
-		if (hid_report_size(desc, size, hid_input, repid) == 0 &&
-		    hid_report_size(desc, size, hid_output, repid) == 0 &&
-		    hid_report_size(desc, size, hid_feature, repid) == 0)
-			continue;
-
-		if (hid_is_collection(desc, size, repid,
-		    HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD))) {
-			finput = 1;
-			if (input != NULL && *input == -1)
-				*input = repid;
-		} else if (hid_is_collection(desc, size, repid,
-		    HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG))) {
-			fconf = 1;
-			if (config != NULL && *config == -1)
-				*config = repid;
-		}
-
-		/* capabilities report could be anywhere */
-		if (hid_locate(desc, size, HID_USAGE2(HUP_DIGITIZERS,
-		    HUD_CONTACT_MAX), repid, hid_feature, NULL, NULL)) {
-			fcap = 1;
-			if (cap != NULL && *cap == -1)
-				*cap = repid;
-		}
-	}
-
-	return (fconf && finput && fcap);
-}
-
 void
 umt_attach(struct device *parent, struct device *self, void *aux)
 {
@@ -163,7 +117,7 @@ umt_attach(struct device *parent, struct
 	sc->sc_quirks = usbd_get_quirks(sc->sc_hdev.sc_udev)->uq_flags;

 	uhidev_get_report_desc(uha->parent, &desc, &size);
-	umt_find_winptp_reports(uha->parent, desc, size, &sc->sc_rep_input,
+	hidmt_find_winptp_reports(desc, size, &sc->sc_rep_input,
 	    &sc->sc_rep_config, &sc->sc_rep_cap);

 	memset(mt, 0, sizeof(sc->sc_mt));