Re: kern/56145: umb(4): fix kernel panics and make IP traffic actually work

[Reinhard Speyerer <rspmn%arcor.de@localhost>]

The following reply was made to PR kern/56145; it has been noted by GNATS.

From: Reinhard Speyerer <rspmn%arcor.de@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: khorben%netbsd.org@localhost
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