Back-out USB data toggle fix for HID devices, since we received multiple

reports about broken devices, e.g. for ukbd(4) and fido(4).

ok mpi@

diff --git a/lib/libfido2/src/hid_openbsd.c b/lib/libfido2/src/hid_openbsd.c
index 38edc41..58b8c3e 100644 (file)
--- a/lib/libfido2/src/hid_openbsd.c
+++ b/lib/libfido2/src/hid_openbsd.c
@@ -88,6 +88,62 @@ fido_hid_manifest(fido_dev_info_t *devlist, size_t ilen, size_t *olen)
        return FIDO_OK;
 }
 
+/*
+ * Workaround for OpenBSD <=6.6-current (as of 201910) bug that loses
+ * sync of DATA0/DATA1 sequence bit across uhid open/close.
+ * Send pings until we get a response - early pings with incorrect
+ * sequence bits will be ignored as duplicate packets by the device.
+ */
+static int
+terrible_ping_kludge(struct hid_openbsd *ctx)
+{
+       u_char data[256];
+       int i, n;
+       struct pollfd pfd;
+
+       if (sizeof(data) < ctx->report_out_len + 1)
+               return -1;
+       for (i = 0; i < 4; i++) {
+               memset(data, 0, sizeof(data));
+               /* broadcast channel ID */
+               data[1] = 0xff;
+               data[2] = 0xff;
+               data[3] = 0xff;
+               data[4] = 0xff;
+               /* Ping command */
+               data[5] = 0x81;
+               /* One byte ping only, Vasili */
+               data[6] = 0;
+               data[7] = 1;
+               fido_log_debug("%s: send ping %d", __func__, i);
+               if (fido_hid_write(ctx, data, ctx->report_out_len + 1) == -1)
+                       return -1;
+               fido_log_debug("%s: wait reply", __func__);
+               memset(&pfd, 0, sizeof(pfd));
+               pfd.fd = ctx->fd;
+               pfd.events = POLLIN;
+               if ((n = poll(&pfd, 1, 100)) == -1) {
+                       fido_log_debug("%s: poll: %s", __func__, strerror(errno));
+                       return -1;
+               } else if (n == 0) {
+                       fido_log_debug("%s: timed out", __func__);
+                       continue;
+               }
+               if (fido_hid_read(ctx, data, ctx->report_out_len, 250) == -1)
+                       return -1;
+               /*
+                * Ping isn't always supported on the broadcast channel,
+                * so we might get an error, but we don't care - we're
+                * synched now.
+                */
+               fido_log_debug("%s: got reply", __func__);
+               fido_log_xxd(data, ctx->report_out_len);
+               return 0;
+       }
+       fido_log_debug("%s: no response", __func__);
+       return -1;
+}
+
 void *
 fido_hid_open(const char *path)
 {
@@ -102,6 +158,16 @@ fido_hid_open(const char *path)
        fido_log_debug("%s: inlen = %zu outlen = %zu", __func__,
            ret->report_in_len, ret->report_out_len);
 
+       /*
+        * OpenBSD (as of 201910) has a bug that causes it to lose
+        * track of the DATA0/DATA1 sequence toggle across uhid device
+        * open and close. This is a terrible hack to work around it.
+        */
+       if (terrible_ping_kludge(ret) != 0) {
+               fido_hid_close(ret);
+               return NULL;
+       }
+
        return (ret);
 }
 

diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index ed072c7..24b7841 100644 (file)
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uhidev.c,v 1.88 2021/02/11 06:55:10 anton Exp $       */
+/*     $OpenBSD: uhidev.c,v 1.89 2021/02/15 11:26:00 mglocker Exp $    */
 /*     $NetBSD: uhidev.c,v 1.14 2003/03/11 16:44:00 augustss Exp $     */
 
 /*
@@ -98,7 +98,6 @@ int uhidev_activate(struct device *, int);
 
 void uhidev_get_report_async_cb(struct usbd_xfer *, void *, usbd_status);
 void uhidev_set_report_async_cb(struct usbd_xfer *, void *, usbd_status);
-void uhidev_clear_iface_eps(struct uhidev_softc *, struct usbd_interface *);
 
 struct cfdriver uhidev_cd = {
        NULL, "uhidev", DV_DULL
@@ -518,9 +517,6 @@ uhidev_open(struct uhidev *scd)
        DPRINTF(("uhidev_open: isize=%d, ep=0x%02x\n", sc->sc_isize,
            sc->sc_iep_addr));
 
-       /* Clear device endpoint toggle. */
-       uhidev_clear_iface_eps(sc, sc->sc_iface);
-
        err = usbd_open_pipe_intr(sc->sc_iface, sc->sc_iep_addr,
                  USBD_SHORT_XFER_OK, &sc->sc_ipipe, sc, sc->sc_ibuf,
                  sc->sc_isize, uhidev_intr, USBD_DEFAULT_INTERVAL);
@@ -530,8 +526,6 @@ uhidev_open(struct uhidev *scd)
                error = EIO;
                goto out1;
        }
-       /* Clear HC endpoint toggle. */
-       usbd_clear_endpoint_toggle(sc->sc_ipipe);
 
        DPRINTF(("uhidev_open: sc->sc_ipipe=%p\n", sc->sc_ipipe));
 
@@ -557,8 +551,6 @@ uhidev_open(struct uhidev *scd)
                        error = EIO;
                        goto out2;
                }
-               /* Clear HC endpoint toggle. */
-               usbd_clear_endpoint_toggle(sc->sc_opipe);
 
                DPRINTF(("uhidev_open: sc->sc_opipe=%p\n", sc->sc_opipe));
 
@@ -968,40 +960,6 @@ uhidev_ioctl(struct uhidev *sc, u_long cmd, caddr_t addr, int flag,
        return 0;
 }
 
-void
-uhidev_clear_iface_eps(struct uhidev_softc *sc, struct usbd_interface *iface)
-{
-       usb_interface_descriptor_t *id;
-       usb_endpoint_descriptor_t *ed;
-       uint8_t xfertype;
-       int i;
-
-       /* Only clear interface endpoints when none are in use. */
-       if (sc->sc_ipipe || sc->sc_opipe)
-               return;
-       DPRINTFN(1,("%s: clear interface eps\n", __func__));
-
-       id = usbd_get_interface_descriptor(iface);
-       if (id == NULL)
-               goto bad;
-
-       for (i = 0; i < id->bNumEndpoints; i++) {
-               ed = usbd_interface2endpoint_descriptor(iface, i);
-               if (ed == NULL)
-                       goto bad;
-
-               xfertype = UE_GET_XFERTYPE(ed->bmAttributes);
-               if (xfertype == UE_BULK || xfertype == UE_INTERRUPT) {
-                       if (usbd_clear_endpoint_feature(sc->sc_udev,
-                           ed->bEndpointAddress, UF_ENDPOINT_HALT))
-                               goto bad;
-               }
-       }
-       return;
-bad:
-       printf("%s: clear endpoints failed!\n", __func__);
-}
-
 int
 uhidev_set_report_dev(struct uhidev_softc *sc, struct uhidev *dev, int repid)
 {