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;
}
