From owner-freebsd-usb@FreeBSD.ORG Sat Nov 7 01:04:24 2009 Return-Path: Delivered-To: freebsd-usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BAAC4106566B; Sat, 7 Nov 2009 01:04:24 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe11.tele2.se [212.247.155.65]) by mx1.freebsd.org (Postfix) with ESMTP id DAB5F8FC12; Sat, 7 Nov 2009 01:04:23 +0000 (UTC) X-Cloudmark-Score: 0.000000 [] X-Cloudmark-Analysis: v=1.0 c=1 a=JhkvhRibbtIA:10 a=RERtC8nhXGhYvIZhK0yWrQ==:17 a=6I5d2MoRAAAA:8 a=uKoqv_oPnLvkEHPJRaQA:9 a=ccsqx_3khbR0apkUrCIA:7 a=3JY17-B5VkS6UhrXE7GFpebtYCIA:4 a=SV7veod9ZcQA:10 a=Lsvu8ZUqSK3jz0Eo:21 a=5SAJ2YTNqifRsb1o:21 Received: from [90.149.203.35] (account mc467741@c2i.net HELO laptop.adsl.tele2.no) by mailfe11.swip.net (CommuniGate Pro SMTP 5.2.16) with ESMTPA id 1160189320; Sat, 07 Nov 2009 02:04:22 +0100 From: Hans Petter Selasky To: freebsd-usb@freebsd.org Date: Sat, 7 Nov 2009 02:05:43 +0100 User-Agent: KMail/1.11.4 (FreeBSD/9.0-CURRENT; KDE/4.2.4; i386; ; ) References: <200911060026.nA60QEoB073159@www.freebsd.org> <200911060951.34794.hselasky@c2i.net> In-Reply-To: <200911060951.34794.hselasky@c2i.net> X-Face: (%:6u[ldzJ`0qjD7sCkfdMmD*RxpOwEEQ+KWt[{J#x6ow~JO:,zwp.(t; @Aq :4:&nFCgDb8[3oIeTb^'",;u{5{}C9>"PuY\)!=#\u9SSM-nz8+SR~B\!qBv MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200911070205.44380.hselasky@c2i.net> Cc: freebsd-gnats-submit@freebsd.org, Robert Jenssen Subject: Re: 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: Sat, 07 Nov 2009 01:04:24 -0000 On Friday 06 November 2009 09:51:34 Hans Petter Selasky wrote: > 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@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: > > I'm working on these issues. > > --HPS First set of patches has been commited to USB P4 with some modifications. Please verify: http://p4web.freebsd.org/chv.cgi?CH=170304 --HPS