Unfortunately I'm seeing more and more USB device breakages reported the last few days related to the USB data toggle fix which we did commit 2-3 weeks ago.
Since I can't reproduce the issue here with my USB gear, it's very difficult for me to pin point the issue. Therefore I think we should back it out for now. OK? Index: lib/libfido2/src/hid_openbsd.c =================================================================== RCS file: /cvs/src/lib/libfido2/src/hid_openbsd.c,v retrieving revision 1.6 diff -u -p -u -p -r1.6 hid_openbsd.c --- lib/libfido2/src/hid_openbsd.c 5 Feb 2021 14:19:21 -0000 1.6 +++ lib/libfido2/src/hid_openbsd.c 14 Feb 2021 13:58:22 -0000 @@ -88,6 +88,62 @@ fido_hid_manifest(fido_dev_info_t *devli 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) { @@ -101,6 +157,16 @@ fido_hid_open(const char *path) ret->report_in_len = ret->report_out_len = MAX_U2FHID_LEN; 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); } Index: sys/dev/usb/ugen.c =================================================================== RCS file: /cvs/src/sys/dev/usb/ugen.c,v retrieving revision 1.115 diff -u -p -u -p -r1.115 ugen.c --- sys/dev/usb/ugen.c 5 Feb 2021 08:17:22 -0000 1.115 +++ sys/dev/usb/ugen.c 14 Feb 2021 13:58:26 -0000 @@ -117,7 +117,6 @@ int ugen_do_close(struct ugen_softc *, i int ugen_set_config(struct ugen_softc *, int); int ugen_set_interface(struct ugen_softc *, int, int); int ugen_get_alt_index(struct ugen_softc *, int); -void ugen_clear_iface_eps(struct ugen_softc *, struct usbd_interface *); #define UGENUNIT(n) ((minor(n) >> 4) & 0xf) #define UGENENDPOINT(n) (minor(n) & 0xf) @@ -310,8 +309,6 @@ ugenopen(dev_t dev, int flag, int mode, DPRINTFN(5, ("ugenopen: sc=%p, endpt=%d, dir=%d, sce=%p\n", sc, endpt, dir, sce)); edesc = sce->edesc; - /* Clear device endpoint toggle. */ - ugen_clear_iface_eps(sc, sce->iface); switch (UE_GET_XFERTYPE(edesc->bmAttributes)) { case UE_INTERRUPT: if (dir == OUT) { @@ -339,8 +336,6 @@ ugenopen(dev_t dev, int flag, int mode, clfree(&sce->q); return (EIO); } - /* Clear HC endpoint toggle. */ - usbd_clear_endpoint_toggle(sce->pipeh); DPRINTFN(5, ("ugenopen: interrupt open done\n")); break; case UE_BULK: @@ -348,8 +343,6 @@ ugenopen(dev_t dev, int flag, int mode, edesc->bEndpointAddress, 0, &sce->pipeh); if (err) return (EIO); - /* Clear HC endpoint toggle. */ - usbd_clear_endpoint_toggle(sce->pipeh); break; case UE_ISOCHRONOUS: if (dir == OUT) @@ -1431,42 +1424,4 @@ ugenkqfilter(dev_t dev, struct knote *kn splx(s); return (0); -} - -void -ugen_clear_iface_eps(struct ugen_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. */ - for (i = 0; i < USB_MAX_ENDPOINTS; i++) { - if (i == USB_CONTROL_ENDPOINT) - continue; - if (sc->sc_is_open[i] != 0) - 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__); } Index: sys/dev/usb/uhidev.c =================================================================== RCS file: /cvs/src/sys/dev/usb/uhidev.c,v retrieving revision 1.88 diff -u -p -u -p -r1.88 uhidev.c --- sys/dev/usb/uhidev.c 11 Feb 2021 06:55:10 -0000 1.88 +++ sys/dev/usb/uhidev.c 14 Feb 2021 13:58:26 -0000 @@ -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)); @@ -966,40 +958,6 @@ uhidev_ioctl(struct uhidev *sc, u_long c return -1; } 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 Index: sys/dev/usb/usbdi_util.c =================================================================== RCS file: /cvs/src/sys/dev/usb/usbdi_util.c,v retrieving revision 1.45 diff -u -p -u -p -r1.45 usbdi_util.c --- sys/dev/usb/usbdi_util.c 25 Jan 2021 14:05:57 -0000 1.45 +++ sys/dev/usb/usbdi_util.c 14 Feb 2021 13:58:26 -0000 @@ -192,19 +192,6 @@ usbd_clear_port_feature(struct usbd_devi } usbd_status -usbd_clear_endpoint_feature(struct usbd_device *dev, int epaddr, int sel) -{ - usb_device_request_t req; - - req.bmRequestType = UT_WRITE_ENDPOINT; - req.bRequest = UR_CLEAR_FEATURE; - USETW(req.wValue, sel); - USETW(req.wIndex, epaddr); - USETW(req.wLength, 0); - return (usbd_do_request(dev, &req, 0)); -} - -usbd_status usbd_set_port_feature(struct usbd_device *dev, int port, int sel) { usb_device_request_t req; Index: sys/dev/usb/usbdi_util.h =================================================================== RCS file: /cvs/src/sys/dev/usb/usbdi_util.h,v retrieving revision 1.30 diff -u -p -u -p -r1.30 usbdi_util.h --- sys/dev/usb/usbdi_util.h 25 Jan 2021 14:05:57 -0000 1.30 +++ sys/dev/usb/usbdi_util.h 14 Feb 2021 13:58:26 -0000 @@ -41,7 +41,6 @@ usbd_status usbd_set_hub_feature(struct usbd_status usbd_clear_hub_feature(struct usbd_device *, int); usbd_status usbd_set_port_feature(struct usbd_device *dev, int, int); usbd_status usbd_clear_port_feature(struct usbd_device *, int, int); -usbd_status usbd_clear_endpoint_feature(struct usbd_device *, int, int); usbd_status usbd_get_device_status(struct usbd_device *, usb_status_t *); usbd_status usbd_get_hub_status(struct usbd_device *, usb_hub_status_t *); usbd_status usbd_get_hub_descriptor(struct usbd_device *, Index: www/chromium/files/hid_service_fido.cc =================================================================== RCS file: /cvs/ports/www/chromium/files/hid_service_fido.cc,v retrieving revision 1.4 diff -u -p -u -p -r1.4 hid_service_fido.cc --- www/chromium/files/hid_service_fido.cc 6 Feb 2021 05:05:04 -0000 1.4 +++ www/chromium/files/hid_service_fido.cc 14 Feb 2021 14:00:25 -0000 @@ -11,7 +11,10 @@ #include <sys/un.h> #include <unistd.h> +// TODO: remove once the missing guard in fido.h is fixed upstream. +extern "C" { #include <fido.h> +} #include <set> #include <string> @@ -72,6 +75,55 @@ void FinishOpen(std::unique_ptr<ConnectP base::Bind(&CreateConnection, base::Passed(¶ms))); } +bool terrible_ping_kludge(int fd, const std::string &path) { + u_char data[256]; + int i, n; + struct pollfd pfd; + + 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; + HID_LOG(EVENT) << "send ping " << i << " " << path; + if (write(fd, data, 64) == -1) { + HID_PLOG(ERROR) << "write " << path; + return false; + } + HID_LOG(EVENT) << "wait reply " << path; + memset(&pfd, 0, sizeof(pfd)); + pfd.fd = fd; + pfd.events = POLLIN; + if ((n = poll(&pfd, 1, 100)) == -1) { + HID_PLOG(EVENT) << "poll " << path; + return false; + } else if (n == 0) { + HID_LOG(EVENT) << "timed out " << path; + continue; + } + if (read(fd, data, 64) == -1) { + HID_PLOG(ERROR) << "read " << path; + return false; + } + /* + * Ping isn't always supported on the broadcast channel, + * so we might get an error, but we don't care - we're + * synched now. + */ + HID_LOG(EVENT) << "got reply " << path; + return true; + } + HID_LOG(ERROR) << "no response " << path; + return false; +} + void OpenOnBlockingThread(std::unique_ptr<ConnectParams> params) { base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); @@ -85,6 +137,11 @@ void OpenOnBlockingThread(std::unique_pt if (!device_file.IsValid()) { HID_LOG(EVENT) << "Failed to open '" << device_node << "': " << base::File::ErrorToString(device_file.error_details()); + task_runner->PostTask(FROM_HERE, base::BindOnce(std::move(params->callback), nullptr)); + return; + } + if (!terrible_ping_kludge(device_file.GetPlatformFile(), device_node)) { + HID_LOG(EVENT) << "Failed to ping " << device_node; task_runner->PostTask(FROM_HERE, base::BindOnce(std::move(params->callback), nullptr)); return; }