The following reply was made to PR kernel/6583; it has been noted by GNATS.
From: Yuichiro Goto <[email protected]> To: Jacob Meuser <[email protected]> Cc: [email protected] Subject: Re: kernel/6583: memory leak in usbd_set_config_index() Date: Tue, 29 Mar 2011 08:39:25 +0900 Hello, > doesn't this now miss 'free(cdp)'? and why 'free(dev->ifaces)'? that was > not allocated here. The below code snippet is taken from the original code. free(dev->cdesc, M_USB) is same effect as free(cdp, M_USB) because dev->cdesc is initialized as cdp at the line 780. dev->ifaces is allocated at the line 773 and it should be freed if usbd_fill_iface_data() fails. 771: /* Allocate and fill interface data. */ 772: nifc = cdp->bNumInterface; 773: dev->ifaces = malloc(nifc * sizeof(struct usbd_interface), 774: M_USB, M_NOWAIT); 775: if (dev->ifaces == NULL) { 776: err = USBD_NOMEM; 777: goto bad; 778: } 779: DPRINTFN(5,("usbd_set_config_index: dev=%p cdesc=%p\n", dev, cdp)); 780: dev->cdesc = cdp; 781: dev->config = cdp->bConfigurationValue; 782: for (ifcidx = 0; ifcidx < nifc; ifcidx++) { 783: err = usbd_fill_iface_data(dev, ifcidx, 0); 784: if (err) { 785: while (--ifcidx >= 0) 786: usbd_free_iface_data(dev, ifcidx); 787: goto bad; 788: } 789: } Regards, Yuichiro Goto On Tue, Mar 29, 2011 at 2:44 AM, Jacob Meuser <[email protected]> wrote: > On Mon, Mar 28, 2011 at 11:12:14PM +0900, [email protected] wrote: >> >Number: 6583 >> >Category: kernel >> >Synopsis: memory leak in usbd_set_config_index() >> >Confidential: yes >> >Severity: serious >> >Priority: medium >> >Responsible: bugs >> >State: open >> >Quarter: >> >Keywords: >> >Date-Required: >> >Class: sw-bug >> >Submitter-Id: unknown >> >Arrival-Date: Mon Mar 28 15:10:01 GMT 2011 >> >Closed-Date: >> >Last-Modified: >> >Originator: >> >Release: >> >Organization: >> >Environment: >> System : OpenBSD 4.8 >> Details : OpenBSD 4.8 (GENERIC) #136: Mon Aug 16 09:06:23 MDT 2010 >> [email protected]:/usr/src/sys/arch/i386/compile/GENERIC >> >> Architecture: OpenBSD.i386 >> Machine : i386 >> >Description: >> dev->ifaces is not freed after failure of usbd_fill_iface_data() in usbd_set_config_index(). >> >How-To-Repeat: >> >> >Fix: >> See the following diff: >> >> Index: dev/usb/usb_subr.c >> =================================================================== >> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v >> retrieving revision 1.73 >> diff -u -r1.73 usb_subr.c >> --- dev/usb/usb_subr.c 14 Jan 2009 21:02:57 -0000 1.73 >> +++ dev/usb/usb_subr.c 22 Mar 2011 13:42:36 -0000 >> @@ -731,7 +731,7 @@ >> } >> DPRINTF(("usbd_set_config_index: (addr %d) cno=%d attr=0x%02x, " >> "selfpowered=%d, power=%d\n", >> - cdp->bConfigurationValue, dev->address, cdp->bmAttributes, >> + dev->address, cdp->bConfigurationValue, cdp->bmAttributes, >> selfpowered, cdp->bMaxPower * 2)); >> >> /* Check if we have enough power. */ >> @@ -784,7 +784,12 @@ >> if (err) { >> while (--ifcidx >= 0) >> usbd_free_iface_data(dev, ifcidx); >> - goto bad; >> + free(dev->ifaces, M_USB); >> + free(dev->cdesc, M_USB); >> + dev->ifaces = NULL; >> + dev->cdesc = NULL; >> + dev->config = USB_UNCONFIG_NO; >> + return (err); >> } >> } >> > > > doesn't this now miss 'free(cdp)'? and why 'free(dev->ifaces)'? that was > not allocated here. > > -- > [email protected] > SDF Public Access UNIX System - http://sdf.lonestar.org
