NetBSD Problem Report #56145

From www@netbsd.org  Tue May  4 21:00:54 2021
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id AF7401A9241
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  4 May 2021 21:00:54 +0000 (UTC)
Message-Id: <20210504210053.3404C1A9244@mollari.NetBSD.org>
Date: Tue,  4 May 2021 21:00:53 +0000 (UTC)
From: rspmn@arcor.de
Reply-To: rspmn@arcor.de
To: gnats-bugs@NetBSD.org
Subject: umb(4): fix kernel panics and make IP traffic actually work
X-Send-Pr-Version: www-1.0

>Number:         56145
>Category:       kern
>Synopsis:       umb(4): fix kernel panics and make IP traffic actually work
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue May 04 21:05:00 +0000 2021
>Last-Modified:  Fri Aug 09 22:05:01 +0000 2024
>Originator:     Reinhard Speyerer
>Release:        NetBSD-9.1
>Organization:
>Environment:
NetBSD netbsd 9.1 NetBSD 9.1 (GENERIC) #0: Sun Oct 18 19:24:30 UTC 2020  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/i386/compile/GENERIC i386
>Description:
[This PR supersedes kern/55956 which can be closed.]

umb(4) has several problems which either cause kernel panics, failed
kernel assertions or prevent IP traffic from working properly:

1. Improper initialization of sc in umb_detach() causes a kernel panic
   on device disconnect. This has been fixed in if_umb.c version 1.19
   on the MAIN branch but is still present on the netbsd-9 branch.

2. Connecting a device which has a MBIM interface and serial ports
   supported by another kernel driver like u3g(4) causes a failed
   kernel assertion in usbd_get_interface_descriptor() because
   umb_attach() passes a NULL pointer.

3. The reception of an IP packet from the device causes a kernel
   panic in umb_decap() due to a missing ifp->if_percpuq assignment
   in umb_attach().

4. Buffer allocation in umb_alloc_xfers() does not take MBIM headers
   into account.

5. IP packets are not sent to the device since umb(4) interfaces have
   the <DETACHED> flag set. Since umb(4) does currently not support
   IPv6 iftshould also be marked as EAFNOSUPPORT in umb_ioctl().

6. The MBIM implementation on the device ignores the IP packet sent
   to it due to a missing initialization of hdr->wNdpIndex in
   umb_encap().
>How-To-Repeat:
Build a kernel with support for umb(4), connect a USB device with a MBIM
interface, set parameters with umbctl(8), perform a ifconfig umb0 up,
wait until umbctl(8)/ifconfig(8) output indicates that there is
an active data connection and then try to pass IP traffic over umb0.

>Fix:
--- if_umb.c.orig	2019-12-17 17:12:53.000000000 +0100
+++ if_umb.c	2021-05-03 22:31:14.332847796 +0200
@@ -396,5 +396,8 @@ umb_attach(device_t parent, device_t sel
 	for (i = 0; i < uiaa->uiaa_nifaces; i++) {
-		id = usbd_get_interface_descriptor(uiaa->uiaa_ifaces[i]);
-		if (id != NULL && id->bInterfaceNumber == data_ifaceno) {
-			sc->sc_data_iface = uiaa->uiaa_ifaces[i];
+		if (uiaa->uiaa_ifaces[i] != NULL) {
+			id = usbd_get_interface_descriptor(uiaa->uiaa_ifaces[i]);
+			if (id != NULL && id->bInterfaceNumber == data_ifaceno) {
+				sc->sc_data_iface = uiaa->uiaa_ifaces[i];
+				uiaa->uiaa_ifaces[i] = NULL;
+			}
 		}
@@ -540,2 +543,3 @@ umb_attach(device_t parent, device_t sel
 	}
+	ifp->if_percpuq = if_percpuq_create(ifp);
 	if_register(ifp);
@@ -658,3 +662,3 @@ umb_alloc_xfers(struct umb_softc *sc)
 		err |= usbd_create_xfer(sc->sc_rx_pipe,
-		    sc->sc_rx_bufsz,
+		    sc->sc_rx_bufsz + MBIM_HDR32_LEN,
 		    0, 0, &sc->sc_rx_xfer);
@@ -663,3 +667,3 @@ umb_alloc_xfers(struct umb_softc *sc)
 		err |= usbd_create_xfer(sc->sc_tx_pipe,
-		    sc->sc_tx_bufsz,
+		    sc->sc_tx_bufsz + MBIM_HDR16_LEN,
 		    0, 0, &sc->sc_tx_xfer);
@@ -746,2 +750,3 @@ umb_ioctl(struct ifnet *ifp, u_long cmd,
 	struct umb_softc *sc = ifp->if_softc;
+	struct in_ifaddr *ia;
 	struct ifaddr *ifa = (struct ifaddr *)data;
@@ -762,2 +767,4 @@ umb_ioctl(struct ifnet *ifp, u_long cmd,
 		case AF_INET:
+			ia = ifatoia(ifa);
+			ia->ia4_flags &= ~IN_IFF_DETACHED;
 			break;
@@ -766,2 +773,3 @@ umb_ioctl(struct ifnet *ifp, u_long cmd,
 		case AF_INET6:
+			error = EAFNOSUPPORT;
 			break;
@@ -1850,2 +1858,3 @@ umb_encap(struct umb_softc *sc, struct m
 	sc->sc_tx_seq++;
+	USETW(hdr->wNdpIndex, sizeof (*hdr));


>Audit-Trail:
From: Reinhard Speyerer <rspmn@arcor.de>
To: gnats-bugs@netbsd.org
Cc: khorben@netbsd.org
Subject: Re: kern/56145: umb(4): fix kernel panics and make IP traffic
 actually work
Date: Fri, 9 Aug 2024 22:41:26 +0200

 Here is an updated patch for if_umb.c 1.26 which fixes the following:

 1. Connecting a device which has a MBIM interface and serial ports
    supported by another kernel driver like u3g(4) causes a failed
    kernel assertion in usbd_get_interface_descriptor() because
    umb_attach() passes a NULL pointer.

 2. The reception of a packet from the device may cause a kernel
    panic in umb_decap() due to a missing ifp->if_percpuq assignment
    in umb_attach().

 3. The MBIM implementation on the device ignores the IP packet sent
    to it due to a missing initialization of hdr->wNdpIndex in
    umb_encap().

 --- if_umb.c.orig	2024-07-24 22:14:17.939793557 +0200
 +++ if_umb.c	2024-08-09 18:53:05.387540279 +0200
 @@ -405,9 +405,12 @@ umb_attach(device_t parent, device_t sel
  	}

  	for (i = 0; i < uiaa->uiaa_nifaces; i++) {
 -		id = usbd_get_interface_descriptor(uiaa->uiaa_ifaces[i]);
 -		if (id != NULL && id->bInterfaceNumber == data_ifaceno) {
 -			sc->sc_data_iface = uiaa->uiaa_ifaces[i];
 +		if (uiaa->uiaa_ifaces[i] != NULL) {
 +			id = usbd_get_interface_descriptor(uiaa->uiaa_ifaces[i]);
 +			if (id != NULL && id->bInterfaceNumber == data_ifaceno) {
 +				sc->sc_data_iface = uiaa->uiaa_ifaces[i];
 +				uiaa->uiaa_ifaces[i] = NULL;
 +			}
  		}
  	}
  	if (sc->sc_data_iface == NULL) {
 @@ -544,6 +547,7 @@ umb_attach(device_t parent, device_t sel

  	/* attach the interface */
  	if_initialize(ifp);
 +	ifp->if_percpuq = if_percpuq_create(ifp);
  	if_register(ifp);
  	if_alloc_sadl(ifp);

 @@ -1854,6 +1858,7 @@ umb_encap(struct umb_softc *sc, struct m
  	USETW(hdr->wHeaderLength, sizeof(*hdr));
  	USETW(hdr->wSequence, sc->sc_tx_seq);
  	sc->sc_tx_seq++;
 +	USETW(hdr->wNdpIndex, sizeof (*hdr));

  	len = m->m_pkthdr.len;


 More work may be needed to make umb(4) useful in practice as the
 umb0 addresses assigned are still marked as <DETACHED> on NetBSD 10.0:

   # umbctl umb0 pin <PIN> apn internet
   # ifconfig umb0 up
   # umbctl umb0
   umb0: state up, mode automatic, registration home network
           provider "o2 - de", dataclass GPRS, signal #99
           phone number "xxxxxxxxxxxxx", roaming "" (denied)
           APN "internet", TX 236800, RX 236800
           firmware "SWI9X15C_05.05.78.00", hardware "MC7304"
   # ifconfig umb0
   umb0: flags=0x8851<UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST> mtu 1430
   ifconfig: umb0: no media types?
           inet6 fe80::208:9bff:feb4:3524%umb0/64 ->  flags 0x8<DETACHED> scopeid 0x4
           inet 10.x.y.71/28 -> 10.x.y.72 flags 0x4<DETACHED>
   # route -n add 8.8.8.8 10.x.y.72
   add host 8.8.8.8: gateway 10.x.y.72
   # ping -n -c 10 -i 1 8.8.8.8
   PING 8.8.8.8 (8.8.8.8): 56 data bytes

   ----8.8.8.8 PING Statistics----
   10 packets transmitted, 0 packets received, 100.0% packet loss

 Reinhard

Home	PR Database Search