From owner-freebsd-usb@FreeBSD.ORG Fri Nov 6 00:30:08 2009 Return-Path: Delivered-To: freebsd-usb@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6FB191065693 for ; Fri, 6 Nov 2009 00:30:08 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 33AD88FC1C for ; Fri, 6 Nov 2009 00:30:08 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id nA60U8RI078364 for ; Fri, 6 Nov 2009 00:30:08 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id nA60U8mc078359; Fri, 6 Nov 2009 00:30:08 GMT (envelope-from gnats) Resent-Date: Fri, 6 Nov 2009 00:30:08 GMT Resent-Message-Id: <200911060030.nA60U8mc078359@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-usb@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Robert Jenssen Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9F18A106568B for ; Fri, 6 Nov 2009 00:26:14 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 8E1CE8FC22 for ; Fri, 6 Nov 2009 00:26:14 +0000 (UTC) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id nA60QEkR073219 for ; Fri, 6 Nov 2009 00:26:14 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id nA60QEoB073159; Fri, 6 Nov 2009 00:26:14 GMT (envelope-from nobody) Message-Id: <200911060026.nA60QEoB073159@www.freebsd.org> Date: Fri, 6 Nov 2009 00:26:14 GMT From: Robert Jenssen To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Nov 2009 00:30:08 -0000 >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 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: