usb/140325: Missing/incorrect initialisation and memory leak in
libusb10/libusb20
Hans Petter Selasky
hselasky at c2i.net
Fri Nov 6 09:00:10 UTC 2009
The following reply was made to PR usb/140325; it has been noted by GNATS.
From: Hans Petter Selasky <hselasky at c2i.net>
To: freebsd-usb at freebsd.org
Cc: Robert Jenssen <robertjenssen at hotmail.com>,
freebsd-gnats-submit at freebsd.org
Subject: Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20
Date: Fri, 6 Nov 2009 09:51:34 +0100
On Friday 06 November 2009 01:26:14 Robert Jenssen wrote:
> >Number: 140325
> >Category: usb
> >Synopsis: Missing/incorrect initialisation and memory leak in
> > libusb10/libusb20 Confidential: no
> >Severity: serious
> >Priority: medium
> >Responsible: freebsd-usb
> >State: open
> >Quarter:
> >Keywords:
> >Date-Required:
> >Class: sw-bug
> >Submitter-Id: current-users
> >Arrival-Date: Fri Nov 06 00:30:07 UTC 2009
> >Closed-Date:
> >Last-Modified:
> >Originator: Robert Jenssen
> >Release: 8.0RC2
> >Organization:
> >Environment:
>
> FreeBSD kraken 8.0-RC2 FreeBSD 8.0-RC2 #0: Fri Nov 6 02:43:24 EST 2009
> root at kraken:/usr/obj/usr/src/sys/KRAKEN i386
>
> >Description:
>
> I was getting some weird values for usb configuration descriptor extra
> length. Valgrind is a wonderful tool recently ported to FreeBSD by
> stas at FreeBSD.org. Using valgrind I found the following problems (fixed in
> the attached patch):
>
> 1. In libusb10_desc.c, libusb_get_config_descriptor(), at line 162:
> pconfd->interface = (libusb_interface *) (pconfd +
> sizeof(libusb_config_descriptor));
> should be:
> pconfd->interface = (libusb_interface *) (pconfd + 1);
> This problem causes illegal writes past the end of pconfd.
>
> 2. In libusb20_ugen20.c , ugen20_get_config_desc_full(), cdesc and ptr are
> not initialised. This problem causes branches on uninitialised values.
>
> 3. In libusb20.c, libusb20_be_free(), pbe is not free'd. This problem
> causes a minor memory leak.
>
> >How-To-Repeat:
>
> Compile the following test, link with a debug version of libusb.a and run
> valgrind.
>
> #include <libusb.h>
> int main(void) {
> libusb_context *context;
> struct libusb_device **devs;
> struct libusb_config_descriptor *config;
>
> libusb_init(&context);
> libusb_get_device_list(context, &devs);
> libusb_get_active_config_descriptor(devs[0], &config);
> libusb_free_config_descriptor(config);
> libusb_free_device_list(devs, 1);
> libusb_exit(context);
> return 0;
> }
>
> >Fix:
>
> Apply the attached patch in /usr/src/lib/libusb
>
>
> Patch attached with submission follows:
>
> *** libusb10_desc.c 2009-11-06 10:35:00.000000000 +1100
> --- libusb10_desc.c.orig 2009-08-03 18:13:06.000000000 +1000
> ***************
> *** 116,133 ****
> nep = 0;
> nextra = pconf->extra.len;
>
> - #define NEXTRA_ALIGN_TO(n) (nextra=((nextra+n)/n)*n)
> for (i = 0; i < nif; i++) {
>
> pinf = pconf->interface + i;
> nextra += pinf->extra.len;
> - NEXTRA_ALIGN_TO(16);
> nep += pinf->num_endpoints;
> k = pinf->num_endpoints;
> pend = pinf->endpoints;
> while (k--) {
> nextra += pend->extra.len;
> - NEXTRA_ALIGN_TO(16);
> pend++;
> }
>
> --- 116,130 ----
> ***************
> *** 136,148 ****
> pinf = pinf->altsetting;
> while (j--) {
> nextra += pinf->extra.len;
> - NEXTRA_ALIGN_TO(16);
> nep += pinf->num_endpoints;
> k = pinf->num_endpoints;
> pend = pinf->endpoints;
> while (k--) {
> nextra += pend->extra.len;
> - NEXTRA_ALIGN_TO(16);
> pend++;
> }
> pinf++;
> --- 133,143 ----
> ***************
> *** 155,163 ****
> (nalt * sizeof(libusb_interface_descriptor)) +
> (nep * sizeof(libusb_endpoint_descriptor));
>
> - /* Align nextra */
> - NEXTRA_ALIGN_TO(16);
> -
> pconfd = malloc(nextra);
>
> if (pconfd == NULL) {
> --- 150,155 ----
> ***************
> *** 167,173 ****
> /* make sure memory is clean */
> memset(pconfd, 0, nextra);
>
> ! pconfd->interface = (libusb_interface *) (pconfd + 1);
>
> ifd = (libusb_interface_descriptor *) (pconfd->interface + nif);
> endd = (libusb_endpoint_descriptor *) (ifd + nalt);
> --- 159,166 ----
> /* make sure memory is clean */
> memset(pconfd, 0, nextra);
>
> ! pconfd->interface = (libusb_interface *) (pconfd +
> ! sizeof(libusb_config_descriptor));
>
> ifd = (libusb_interface_descriptor *) (pconfd->interface + nif);
> endd = (libusb_endpoint_descriptor *) (ifd + nalt);
> ***************
> *** 194,200 ****
>
> for (i = 0; i < nif; i++) {
>
> - pconfd->interface[i].altsetting = 0;
> pconfd->interface[i].altsetting = ifd;
> ifd->endpoint = endd;
> endd += pconf->interface[i].num_endpoints;
> --- 187,192 ----
> *** libusb20.c 2009-11-06 10:35:00.000000000 +1100
> --- libusb20.c.orig 2009-08-03 18:13:06.000000000 +1000
> ***************
> *** 1093,1100 ****
> if (pbe->methods->exit_backend) {
> pbe->methods->exit_backend(pbe);
> }
> - /* free backend */
> - free(pbe);
> return;
> }
>
> --- 1093,1098 ----
> *** libusb20_desc.c 2009-11-06 10:35:00.000000000 +1100
> --- libusb20_desc.c.orig 2009-08-03 18:13:06.000000000 +1000
> ***************
> *** 118,124 ****
> if (lub_config == NULL) {
> return (NULL); /* out of memory */
> }
> - memset(lub_config, 0, size);
> lub_interface = (void *)(lub_config + 1);
> lub_alt_interface = (void *)(lub_interface + niface_no_alt);
> lub_endpoint = (void *)(lub_interface + niface);
> --- 118,123 ----
> *** libusb20_ugen20.c 2009-11-06 10:35:00.000000000 +1100
> --- libusb20_ugen20.c.orig 2009-10-23 23:02:01.000000000 +1100
> ***************
> *** 449,455 ****
> uint16_t len;
> int error;
>
> - memset(&cdesc, 0, sizeof(cdesc));
> memset(&gen_desc, 0, sizeof(gen_desc));
>
> gen_desc.ugd_data = &cdesc;
> --- 449,454 ----
> ***************
> *** 469,475 ****
> if (!ptr) {
> return (LIBUSB20_ERROR_NO_MEM);
> }
> - memset(ptr, 0, len);
> gen_desc.ugd_data = ptr;
> gen_desc.ugd_maxlen = len;
>
> --- 468,473 ----
>
> >Release-Note:
> >Audit-Trail:
> >Unformatted:
>
I'm working on these issues.
--HPS
More information about the freebsd-usb
mailing list