From: Jan Schreiber <jes@posteo.de>
Subject: Re: uncm(4): process all notifications in one interrupt transfer
To: Mikolaj Kucharski <mikolaj@kucharski.name>
Cc: tech@openbsd.org, stsp@stsp.name
Date: Mon, 12 Oct 2026 18:39:52 +0000

On Mon, Oct 12, 2026 at 05:36:17PM +0000, Mikolaj Kucharski wrote:
> Jan.
>
> Thank you looking into this. See my comments below.

You are correct, I only copied the header. Thank you!
Added the missing memcpy to msg.data below.

diff --git sys/dev/usb/if_uncm.c sys/dev/usb/if_uncm.c
index a003841a7b7..3723459bb2f 100644
--- sys/dev/usb/if_uncm.c
+++ sys/dev/usb/if_uncm.c
@@ -1174,7 +1174,7 @@ uncm_intr(struct usbd_xfer *xfer, void *priv, usbd_status status)
 {
 	struct uncm_softc *sc = priv;
 	struct ifnet *ifp = GET_IFP(sc);
-	struct usb_cdc_notification *msg;
+	struct usb_cdc_notification msg;
 	uint32_t up, down;
 	uint8_t *buf;
 	int	total_len, off, dlen;
@@ -1213,8 +1213,8 @@ uncm_intr(struct usbd_xfer *xfer, void *priv, usbd_status status)
 	buf = (uint8_t *)&sc->sc_intr_msg;
 	for (off = 0; off + UCDC_NOTIFICATION_LENGTH <= total_len;
 	    off += UCDC_NOTIFICATION_LENGTH + dlen) {
-		msg = (struct usb_cdc_notification *)(buf + off);
-		dlen = UGETW(msg->wLength);
+		memcpy(&msg, buf + off, UCDC_NOTIFICATION_LENGTH);
+		dlen = UGETW(msg.wLength);

 		if (off + UCDC_NOTIFICATION_LENGTH + dlen > total_len) {
 			DPRINTF("%s: truncated notification at offset %d\n",
@@ -1223,26 +1223,29 @@ uncm_intr(struct usbd_xfer *xfer, void *priv, usbd_status status)
 		}

 		/* Framing is lost; do not parse the rest. */
-		if (msg->bmRequestType != UCDC_NOTIFICATION) {
+		if (msg.bmRequestType != UCDC_NOTIFICATION) {
 			DPRINTF("%s: unexpected notification (type=0x%02x)\n",
-			    DEVNAM(sc), msg->bmRequestType);
+			    DEVNAM(sc), msg.bmRequestType);
 			break;
 		}

-		switch (msg->bNotification) {
+		memcpy(msg.data, buf + off + UCDC_NOTIFICATION_LENGTH,
+	    	    MIN(dlen, sizeof(msg.data)));
+
+		switch (msg.bNotification) {
 		case UCDC_N_NETWORK_CONNECTION:
 			if (ifp->if_flags & IFF_DEBUG)
 				log(LOG_DEBUG, "%s: network %sconnected\n",
 				    DEVNAM(sc),
-				    UGETW(msg->wValue) ? "" : "dis");
-			sc->sc_link = UGETW(msg->wValue) ?
+				    UGETW(msg.wValue) ? "" : "dis");
+			sc->sc_link = UGETW(msg.wValue) ?
 			    LINK_STATE_UP : LINK_STATE_DOWN;
 			usb_add_task(sc->sc_udev, &sc->sc_link_task);
 			break;
 		case UCDC_N_CONNECTION_SPEED_CHANGE:
 			if (dlen >= 8) {
-				down = UGETDW(&msg->data[0]);
-				up = UGETDW(&msg->data[4]);
+				down = UGETDW(&msg.data[0]);
+				up = UGETDW(&msg.data[4]);
 				sc->sc_baudrate = MIN(up, down);
 				DPRINTFN(2, "%s: speed down %u up %u\n",
 				    DEVNAM(sc), down, up);
@@ -1251,7 +1254,7 @@ uncm_intr(struct usbd_xfer *xfer, void *priv, usbd_status status)
 			break;
 		default:
 			DPRINTF("%s: unexpected notification (0x%02x)\n",
-			    DEVNAM(sc), msg->bNotification);
+			    DEVNAM(sc), msg.bNotification);
 			break;
 		}
 	}