From: mglocker <mglocker@openbsd.org>
Date: Mon, 15 Feb 2021 11:26:00 +0000 (+0000)
Subject: Back-out USB data toggle fix for HID devices, since we received multiple
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=65f50905169ac9865a865e567e90c5d9c01d42c1;p=openbsd

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 38edc41d4c6..58b8c3e9475 100644
--- 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 ed072c741d0..24b7841d837 100644
--- 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)
 {