Date: Fri, 6 Nov 2009 00:26:14 GMT From: Robert Jenssen <robertjenssen@hotmail.com> To: freebsd-gnats-submit@FreeBSD.org Subject: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 Message-ID: <200911060026.nA60QEoB073159@www.freebsd.org> Resent-Message-ID: <200911060030.nA60U8mc078359@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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@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@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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911060026.nA60QEoB073159>