From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Fix dumping ISOCHRONOUS IN transfers
To: OpenBSD tech <tech@openbsd.org>
Date: Thu, 19 Dec 2024 02:27:23 +0100

tech@,

tcpdump can be used to capture USB traffic, and it works well with one
exception: ISOCHRONOUS IN transfers.

While OUT transfers work as expected, IN transfers only capture the first
packet that can be read.

The root cause is that the same array is being used to store both the
offsets in the data buffer and the resulting lengths. As a result, the dump
records the length of the packets read from the device as offsets in the
data buffer.

When the device returns full packets, everything works correctly. However,
when it returns partial packets, only the first packet is useful, and all
subsequent packets are lost.

I’d like to suggest a potential fix for this issue.

Any feedback or testing would be greatly appreciated.

Index: sys/dev/usb/usb.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/usb.c,v
diff -u -p -r1.133 usb.c
--- sys/dev/usb/usb.c	4 Sep 2024 07:54:52 -0000	1.133
+++ sys/dev/usb/usb.c	19 Dec 2024 01:25:24 -0000
@@ -952,7 +952,7 @@ usb_tap(struct usbd_bus *bus, struct usb
 		struct usbpcap_iso_hdr_full	uih;
 	} h;
 	struct usbpcap_pkt_hdr *uph = &h.uch.uch_hdr;
-	uint32_t nframes, offset;
+	uint32_t nframes;
 	unsigned int bpfdir;
 	void *data = NULL;
 	size_t flen;
@@ -970,7 +970,6 @@ usb_tap(struct usbd_bus *bus, struct usb
 		uph->uph_xfertype = USBPCAP_TRANSFER_CONTROL;
 		break;
 	case UE_ISOCHRONOUS:
-		offset = 0;
 		nframes = xfer->nframes;
 #ifdef DIAGNOSTIC
 		if (nframes > _USBPCAP_MAX_ISOFRAMES) {
@@ -987,11 +986,10 @@ usb_tap(struct usbd_bus *bus, struct usb
 		h.uih.uih_nframes = nframes;
 		h.uih.uih_errors = 0; /* we don't have per-frame error */
 		for (i = 0; i < nframes; i++) {
-			h.uih.uih_frames[i].uip_offset = offset;
+			h.uih.uih_frames[i].uip_offset = xfer->froffsets[i];
 			h.uih.uih_frames[i].uip_length = xfer->frlengths[i];
 			/* See above, we don't have per-frame error */
 			h.uih.uih_frames[i].uip_status = 0;
-			offset += xfer->frlengths[i];
 		}
 		break;
 	case UE_BULK:
@@ -1049,6 +1047,12 @@ usb_tap(struct usbd_bus *bus, struct usb
 			if (xfer->rqflags & URQ_REQUEST)
 				h.uch.uch_stage = USBPCAP_CONTROL_STAGE_STATUS;
 		}
+	}
+
+	/* ISOCHRONOUS IN from device may have gaps, use full buffer */
+	if (bpfdir == BPF_DIRECTION_IN && uph->uph_dlen > 0 &&
+	    uph->uph_xfertype == USBPCAP_TRANSFER_ISOCHRONOUS) {
+		uph->uph_dlen = xfer->length;
 	}
 
 	/* Dump bulk/intr/iso data, ctrl DATA or STATUS stage. */
Index: sys/dev/usb/usbdi.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/usbdi.c,v
diff -u -p -r1.111 usbdi.c
--- sys/dev/usb/usbdi.c	23 May 2024 03:21:09 -0000	1.111
+++ sys/dev/usb/usbdi.c	18 Dec 2024 22:35:39 -0000
@@ -482,8 +482,10 @@ usbd_setup_isoc_xfer(struct usbd_xfer *x
 	xfer->priv = priv;
 	xfer->buffer = 0;
 	xfer->length = 0;
-	for (i = 0; i < nframes; i++)
+	for (i = 0; i < nframes; i++) {
+		xfer->froffsets[i] = xfer->length;
 		xfer->length += frlengths[i];
+	}
 	xfer->actlen = 0;
 	xfer->flags = flags;
 	xfer->timeout = USBD_NO_TIMEOUT;
Index: sys/dev/usb/usbdivar.h
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/usbdivar.h,v
diff -u -p -r1.84 usbdivar.h
--- sys/dev/usb/usbdivar.h	8 Oct 2024 19:42:31 -0000	1.84
+++ sys/dev/usb/usbdivar.h	18 Dec 2024 22:35:42 -0000
@@ -223,6 +223,12 @@ struct usbd_xfer {
 	/* For isoc */
 	u_int16_t		*frlengths;
 	int			nframes;
+/*
+ * OpenBSD specific, maximum number of frames per transfer used across
+ * all USB drivers.  This allows us to dump ISOC IN.
+ */
+#define _USBD_MAX_ISOFRAMES	40
+	u_int16_t		froffsets[_USBD_MAX_ISOFRAMES];
 
 	/* For memory allocation */
 	struct usbd_device     *device;

-- 
wbr, Kirill