Second attempt to resolve UHIDEV_CLAIM_MULTIPLE_REPORTID conflict, this

time without using sentinel that cannot be represented using a single
byte. Instead, use 0 as this report ID is reserved according to the USB
HID specification. Fixes attachment of some upd devices which exposes up
to 256 report IDs.

Thanks to Damien Couderc <openbsd at petrocore dot eu> for reporting and
testing.

diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index 1aafa12..59e8949 100644 (file)
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uhidev.c,v 1.97 2021/11/09 06:25:42 anton Exp $       */
+/*     $OpenBSD: uhidev.c,v 1.98 2021/11/10 06:33:30 anton Exp $       */
 /*     $NetBSD: uhidev.c,v 1.14 2003/03/11 16:44:00 augustss Exp $     */
 
 /*
@@ -246,29 +246,36 @@ uhidev_attach(struct device *parent, struct device *self, void *aux)
        sc->sc_isize += (nrepid != 1);  /* one byte for the report ID */
        DPRINTF(("uhidev_attach: isize=%d\n", sc->sc_isize));
 
+       memset(&uha, 0, sizeof(uha));
        uha.uaa = uaa;
        uha.parent = sc;
-       uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
-       uha.nreports = nrepid;
-       uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
 
        /* Look for a driver claiming multiple report IDs first. */
-       dev = config_found_sm(self, &uha, NULL, NULL);
-       if (dev != NULL) {
-               for (repid = 0; repid < nrepid; repid++) {
-                       /*
-                        * Could already be assigned by uhidev_set_report_dev().
-                        */
-                       if (sc->sc_subdevs[repid] != NULL)
-                               continue;
-
-                       if (uha.claimed[repid])
+       if (nrepid > 1) {
+               uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
+               uha.nreports = nrepid;
+               uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
+
+               dev = config_found_sm(self, &uha, NULL, NULL);
+               if (dev != NULL) {
+                       for (repid = 0; repid < nrepid; repid++) {
+                               /*
+                                * Could already be assigned by
+                                * uhidev_set_report_dev().
+                                */
+                               if (sc->sc_subdevs[repid] != NULL)
+                                       continue;
+
+                               if (!uha.claimed[repid])
+                                       continue;
                                sc->sc_subdevs[repid] = (struct uhidev *)dev;
+                       }
                }
-       }
 
-       free(uha.claimed, M_TEMP, nrepid);
-       uha.claimed = NULL;
+               free(uha.claimed, M_TEMP, nrepid);
+               uha.nreports = 0;
+               uha.claimed = NULL;
+       }
 
        for (repid = 0; repid < nrepid; repid++) {
                DPRINTF(("%s: try repid=%d\n", __func__, repid));

diff --git a/sys/dev/usb/uhidev.h b/sys/dev/usb/uhidev.h
index abc78dc..d825a08 100644 (file)
--- a/sys/dev/usb/uhidev.h
+++ b/sys/dev/usb/uhidev.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uhidev.h,v 1.35 2021/11/09 06:25:42 anton Exp $       */
+/*     $OpenBSD: uhidev.h,v 1.36 2021/11/10 06:33:30 anton Exp $       */
 /*     $NetBSD: uhidev.h,v 1.3 2002/10/08 09:56:17 dan Exp $   */
 
 /*
@@ -75,8 +75,8 @@ struct uhidev_attach_arg {
        struct usb_attach_arg   *uaa;
        struct uhidev_softc     *parent;
        uint8_t                  reportid;
-#define        UHIDEV_CLAIM_MULTIPLE_REPORTID  255
-       uint8_t                  nreports;
+#define        UHIDEV_CLAIM_MULTIPLE_REPORTID  0
+       u_int                    nreports;
        uint8_t                 *claimed;
 };