On Tue, Jun 10, 2025 at 09:34:19AM +0000, Gerhard Roth wrote:
> > > Can (int)dlen be negative??? I am not an USB expert, but it seems
> > > that this length ultimatly comes from sc_rx_bufsz or sc_tx_bufsz.
> > > There I see another UGETDW(np.dwNtbInMaxSize) and
> > > UGETDW(np.dwNtbOutMaxSize).?? Do we need a size check in umb_ncm_setup()?
>
> You probably mean 'len' (instead of 'dlen').
> I agree that an additional check in umb_ncm_setup() would be
> a good idea.
>
> Please look at the proposed patch at the end of this mail.
Now you have DPRINTFN() only in the successful case. Maybe warn
or DPRINTF if maxpktlen lies within unexpected range.
There exists a MBIM_MAXSEGSZ_MINVAL. Should the sanity check also
test maxpktlen >= MBIM_MAXSEGSZ_MINVAL ?
OK bluhm@
> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> diff -u -p -u -p -r1.62 if_umb.c
> --- sys/dev/usb/if_umb.c 10 Jun 2025 08:29:57 -0000 1.62
> +++ sys/dev/usb/if_umb.c 10 Jun 2025 09:31:02 -0000
> @@ -359,6 +359,7 @@ umb_attach(struct device *parent, struct
> int altnum;
> int s;
> struct ifnet *ifp;
> + int maxpktlen;
>
> sc->sc_udev = uaa->device;
> sc->sc_ctrl_ifaceno = uaa->ifaceno;
> @@ -461,10 +462,14 @@ umb_attach(struct device *parent, struct
> MBIM_CTRLMSG_MAXLEN);
> /* cont. anyway */
> }
> - sc->sc_maxpktlen = UGETW(md->wMaxSegmentSize);
> - DPRINTFN(2, "%s: ctrl_len=%d, maxpktlen=%d, cap=0x%x\n",
> - DEVNAM(sc), sc->sc_ctrl_len, sc->sc_maxpktlen,
> - md->bmNetworkCapabilities);
> + maxpktlen = UGETW(md->wMaxSegmentSize);
> + if (maxpktlen > 0 && maxpktlen <= INT32_MAX) {
> + sc->sc_maxpktlen = maxpktlen;
> + DPRINTFN(2, "%s: ctrl_len=%d, maxpktlen=%d, "
> + "cap=0x%x\n", DEVNAM(sc), sc->sc_ctrl_len,
> + sc->sc_maxpktlen,
> + md->bmNetworkCapabilities);
> + }
> break;
> default:
> break;
> @@ -709,8 +714,10 @@ umb_ncm_setup(struct umb_softc *sc)
> USETW(req.wLength, sizeof (np));
> if (usbd_do_request(sc->sc_udev, &req, &np) == USBD_NORMAL_COMPLETION &&
> UGETW(np.wLength) == sizeof (np)) {
> - sc->sc_rx_bufsz = UGETDW(np.dwNtbInMaxSize);
> - sc->sc_tx_bufsz = UGETDW(np.dwNtbOutMaxSize);
> + sc->sc_rx_bufsz = MIN(UGETDW(np.dwNtbInMaxSize),
> + (uint32_t)sc->sc_maxpktlen);
> + sc->sc_tx_bufsz = MIN(UGETDW(np.dwNtbOutMaxSize),
> + (uint32_t)sc->sc_maxpktlen);
> sc->sc_maxdgram = UGETW(np.wNtbOutMaxDatagrams);
> sc->sc_align = UGETW(np.wNdpOutAlignment);
> sc->sc_ndp_div = UGETW(np.wNdpOutDivisor);
>
>
