|

»

usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20

View: New views

7 Messages — Rating Filter: Alert me

	usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 by Robert Jenssen-2 :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message >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@.... 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: _______________________________________________ freebsd-usb@... mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."
	Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 by Robert Jenssen :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message The following reply was made to PR usb/140325; it has been noted by GNATS. From: Robert Jenssen <robertjenssen@...> To: bug-followup@..., robertjenssen@... Cc: Subject: Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 Date: Fri, 6 Nov 2009 14:42:13 +1100 Hi, Regarding my bug report usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20. I revised my simple test to: #include <stdio.h> #include <libusb.h> int main(void) { libusb_context context; struct libusb_device devs; libusb_device_handle handle; struct libusb_config_descriptor config; struct libusb_device_descriptor device_desc; int bytes; #define STRLEN 128 unsigned char str[STRLEN]; int transferred; libusb_init(&context); libusb_get_device_list(context, &devs); libusb_get_active_config_descriptor(devs[0], &config); libusb_free_config_descriptor(config); libusb_get_device_descriptor(devs[0], &device_desc); libusb_open(devs[0], &handle); libusb_get_string_descriptor_ascii(handle,device_desc.iProduct,str,STRLEN); libusb_claim_interface(handle, 1); libusb_bulk_transfer(handle, 1, str, STRLEN, &transferred, 0); libusb_release_interface(handle, 1); libusb_close(handle); libusb_free_device_list(devs, 1); libusb_exit(context); return 0; } and found two additional problems: 4. A jump on uninitialised occurs at libusb20.c:658 in libusb20_dev_req_string_sync(): req.wLength = (uint8_t )ptr; / bytes */ if (req.wLength > len) { To fix, zero the upper byte with: memset(ptr, 0, len); 5. A memory leak occurs for devs[0] in the above test. devs[0]->refcnt is incremented to 3 during libusb_bulk_transfer() but not decremented on exit from that function. Consequently, devs[0] is not freed in libusb_free_device_list(). I couldn't see a quick fix for this and it's a minor memory leak (44 bytes) so I will leave it for an expert. Regards, Rob -- Robert Jenssen <robertjenssen@...> _______________________________________________ freebsd-usb@... mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."
	Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 by Robert Jenssen-2 :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message The following reply was made to PR usb/140325; it has been noted by GNATS. From: Robert Jenssen <robertjenssen@...> To: <bug-followup@...> Cc: Subject: Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 Date: Fri, 6 Nov 2009 14:57:00 +1100 --_813ea73d-cf99-4614-b03d-4bb9d22d7898_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi=2C Sorry for the noise. In my last email I missed out a memory leak: 6. In libusb10.c=2C libusb_close()=2C pdev isn't freed. Here is a diff: * libusb10.c 2009-11-06 13:30:51.000000000 +1100 --- libusb10.c.orig 2009-08-03 18:13:06.000000000 +1000 *********** * 416=2C422 **** libusb10_remove_pollfd(ctx=2C &dev->dev_poll)=3B =20 libusb20_dev_close(pdev)=3B - free(pdev)=3B libusb_unref_device(dev)=3B =20 /* make sure our event loop detects the closed device / --- 416=2C421 ---- --=20 Robert Jenssen <robertjenssen@...> =20 _________________________________________________________________ Looking to move this spring? With all the lastest places=2C searching has n= ever been easier. Look now! http://clk.atdmt.com/NMN/go/157631292/direct/01/= --_813ea73d-cf99-4614-b03d-4bb9d22d7898_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable <html> <head> <style><!-- .hmmessage P { margin:0px=3B padding:0px } body.hmmessage { font-size: 10pt=3B font-family:Verdana } --></style> </head> <body class=3D'hmmessage'> Hi=2C<br><br>Sorry for the noise. In my last email I missed out a memory le= ak:<br><br>6. In libusb10.c=2C libusb_close()=2C pdev isn't freed. Here is = a diff:<br> libusb10.c =3B =3B =3B2009-11-06 13:30:51.00000= 0000 +1100<br>--- libusb10.c.orig =3B =3B =3B2009-08-03 18:13:= 06.000000000 +1000<br>***********<br>* 416=2C422 ***<br> =3B &nb= sp=3B =3B =3Blibusb10_remove_pollfd(ctx=2C &=3Bdev->=3Bdev_po= ll)=3B<br> =3B <br> =3B =3B =3B =3Blibusb20_dev_close= (pdev)=3B<br>- =3B =3B free(pdev)=3B<br> =3B =3B =3B &= nbsp=3Blibusb_unref_device(dev)=3B<br> =3B <br> =3B =3B = =3B =3B/ make sure our event loop detects the closed device */<br>---= 416=2C421 ----<br><br>-- <br>Robert Jenssen &lt=3Brobertjenssen@...= m>=3B<br><br> <br /><hr />With all the lastest places=2C searc= hing has never been easier. Look now! <a href=3D'http://clk.atdmt.com/NMN/g= o/157631292/direct/01/' target=3D'_new'>Looking to move this spring?</a></b= ody> </html>= --_813ea73d-cf99-4614-b03d-4bb9d22d7898_-- _______________________________________________ freebsd-usb@... mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."
	Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 by Hans Petter Selasky :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message 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@.... 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 _______________________________________________ freebsd-usb@... mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."
	Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 by Hans Petter Selasky :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message The following reply was made to PR usb/140325; it has been noted by GNATS. From: Hans Petter Selasky <hselasky@...> To: freebsd-usb@... Cc: Robert Jenssen <robertjenssen@...>, freebsd-gnats-submit@... 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@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@.... 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 _______________________________________________ freebsd-usb@... mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."
	Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 by Hans Petter Selasky :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message 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@.... 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 First set of patches has been commited to USB P4 with some modifications. Please verify: http://p4web.freebsd.org/chv.cgi?CH=170304 --HPS _______________________________________________ freebsd-usb@... mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."
	Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 by Hans Petter Selasky :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message The following reply was made to PR usb/140325; it has been noted by GNATS. From: Hans Petter Selasky <hselasky@...> To: freebsd-usb@... Cc: Robert Jenssen <robertjenssen@...>, freebsd-gnats-submit@... Subject: Re: usb/140325: Missing/incorrect initialisation and memory leak in libusb10/libusb20 Date: Sat, 7 Nov 2009 02:05:43 +0100 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@.... 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 First set of patches has been commited to USB P4 with some modifications. Please verify: http://p4web.freebsd.org/chv.cgi?CH=170304 --HPS _______________________________________________ freebsd-usb@... mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscribe@..."