On Sun, Jan 31, 2021 at 07:05:29PM +0100, Thomas Jeunet wrote:
> Hello tech,
>
> in ugen_set_config, the cached config descriptor (ugen.c:213) is
> obsolete after the call to usbd_set_config_no (ugen.c:220). Use a
> refreshed value so the next loop account for the correct number of
> interfaces.
>
> I also believe the dev->cdesc is leaked inside usbd_set_config_index but
> I haven't yet fully audited how the variable is used.
Hmm, I don't think this is the right place to call
usbd_get_config_descriptor() again. We only enter the if branch (line
214) to usbd_set_config_no() when there was a problem with the first
usbd_get_config_descriptor() call:
cdesc = usbd_get_config_descriptor(dev);
if (cdesc == NULL || cdesc->bConfigurationValue != configno) {
If there was no problem we skip that, and cdesc is still valid.
Therefore I think usbd_get_config_descriptor() should be only called
right after usbd_set_config_no().
> Index: ugen.c
> ===================================================================
> RCS file: /var/cvs/src/sys/dev/usb/ugen.c,v
> retrieving revision 1.113
> diff -u -p -r1.113 ugen.c
> --- ugen.c 28 Jan 2021 12:50:28 -0000 1.113
> +++ ugen.c 31 Jan 2021 15:37:05 -0000
> @@ -223,6 +223,7 @@ ugen_set_config(struct ugen_softc *sc, i
> }
> }
>
> + cdesc = usbd_get_config_descriptor(dev);
> memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints);
> for (ifaceno = 0; ifaceno < cdesc->bNumInterfaces; ifaceno++) {
> DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
>
Index: dev/usb/ugen.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.114
diff -u -p -u -p -r1.114 ugen.c
--- dev/usb/ugen.c 1 Feb 2021 09:21:51 -0000 1.114
+++ dev/usb/ugen.c 3 Feb 2021 19:27:37 -0000
@@ -220,6 +220,10 @@ ugen_set_config(struct ugen_softc *sc, i
err = usbd_set_config_no(dev, configno, 1);
if (err)
return (err);
+ cdesc = usbd_get_config_descriptor(dev);
+ if (cdesc == NULL ||
+ cdesc->bConfigurationValue != configno)
+ return (USBD_INVAL);
}
}
