Fix descriptor handling memory leaks

4 files changed, 104 insertions, 39 deletions

diff --git a/libusb/core.c b/libusb/core.c
index 450c5fb..6348c93 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -383,8 +383,10 @@ API_EXPORTED void libusb_device_unref(struct libusb_device *dev)
 		list_del(&dev->list);
 		pthread_mutex_unlock(&usb_devs_lock);
 
-		if (dev->config)
+		if (dev->config) {
+			usbi_clear_configurations(dev);
 			free(dev->config);
+		}
 		free(dev);
 	}
 }
diff --git a/libusb/descriptor.c b/libusb/descriptor.c
index da300e4..dbc2252 100644
--- a/libusb/descriptor.c
+++ b/libusb/descriptor.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include <errno.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -71,6 +72,12 @@ int usbi_parse_descriptor(unsigned char *source, char *descriptor, void *dest)
 	return sp - source;
 }
 
+static void clear_endpoint(struct libusb_endpoint_descriptor *endpoint)
+{
+	if (endpoint->extra)
+		free((unsigned char *) endpoint->extra);
+}
+
 static int parse_endpoint(struct libusb_endpoint_descriptor *endpoint,
 	unsigned char *buffer, int size)
 {
@@ -141,7 +148,7 @@ static int parse_endpoint(struct libusb_endpoint_descriptor *endpoint,
 	endpoint->extra = extra;
 	if (!extra) {
 		endpoint->extralen = 0;
-		return parsed;
+		return -ENOMEM;
 	}
 
 	memcpy(extra, begin, len);
@@ -150,6 +157,30 @@ static int parse_endpoint(struct libusb_endpoint_descriptor *endpoint,
 	return parsed;
 }
 
+static void clear_interface(struct libusb_interface *interface)
+{
+	int i;
+	int j;
+
+	if (interface->altsetting) {
+		for (i = 0; i < interface->num_altsetting; i++) {
+			struct libusb_interface_descriptor *ifp =
+				(struct libusb_interface_descriptor *)
+				interface->altsetting + i;
+			if (ifp->extra)
+				free((void *) ifp->extra);
+			if (ifp->endpoint) {
+				for (j = 0; j < ifp->bNumEndpoints; j++)
+					clear_endpoint((struct libusb_endpoint_descriptor *)
+						ifp->endpoint + j);
+				free((void *) ifp->endpoint);
+			}
+		}
+		free((void *) interface->altsetting);
+	}
+	
+}
+
 static int parse_interface(struct libusb_interface *interface,
 	unsigned char *buffer, int size)
 {
@@ -170,13 +201,18 @@ static int parse_interface(struct libusb_interface *interface,
 		altsetting = realloc(altsetting,
 			sizeof(struct libusb_interface_descriptor) *
 			(interface->num_altsetting + 1));
-		if (!altsetting)
-			return -1;
+		if (!altsetting) {
+			r = -ENOMEM;
+			goto err;
+		}
 		interface->altsetting = altsetting;
 
 		ifp = altsetting + interface->num_altsetting;
 		interface->num_altsetting++;
 		usbi_parse_descriptor(buffer, "bbbbbbbbb", ifp);
+		ifp->extra = NULL;
+		ifp->extralen = 0;
+		ifp->endpoint = NULL;
 
 		/* Skip over the interface */
 		buffer += ifp->bLength;
@@ -190,7 +226,8 @@ static int parse_interface(struct libusb_interface *interface,
 			usbi_parse_descriptor(buffer, "bb", &header);
 			if (header.bLength < 2) {
 				usbi_err("invalid descriptor of length %d", header.bLength);
-				return -1;
+				r = -EINVAL;
+				goto err;
 			}
 
 			/* If we find another "proper" descriptor then we're done */
@@ -208,18 +245,13 @@ static int parse_interface(struct libusb_interface *interface,
 		/* Copy any unknown descriptors into a storage area for */
 		/*  drivers to later parse */
 		len = (int)(buffer - begin);
-		if (!len) {
-			ifp->extra = NULL;
-			ifp->extralen = 0;
-		} else {
-			unsigned char *extra = malloc(len);
-			ifp->extra = extra;
-			if (!extra) {
-				ifp->extralen = 0;
-				/* FIXME will leak memory */
-				return -1;
+		if (len) {
+			ifp->extra = malloc(len);
+			if (!ifp->extra) {
+				r = -ENOMEM;
+				goto err;
 			}
-			memcpy(extra, begin, len);
+			memcpy((unsigned char *) ifp->extra, begin, len);
 			ifp->extralen = len;
 		}
 
@@ -232,8 +264,8 @@ static int parse_interface(struct libusb_interface *interface,
 
 		if (ifp->bNumEndpoints > USB_MAXENDPOINTS) {
 			usbi_err("too many endpoints (%d)", ifp->bNumEndpoints);
-			/* FIXME will leak memory */
-			return -1;
+			r = -EOVERFLOW;
+			goto err;
 		}
 
 		if (ifp->bNumEndpoints > 0) {
@@ -241,9 +273,10 @@ static int parse_interface(struct libusb_interface *interface,
 			tmp = ifp->bNumEndpoints * sizeof(struct libusb_endpoint_descriptor);
 			endpoint = malloc(tmp);
 			ifp->endpoint = endpoint;
-			if (!endpoint)
-				/* FIXME will leak memory? */
-				return -1;      
+			if (!endpoint) {
+				r = -ENOMEM;
+				goto err;
+			}
 
 			memset(endpoint, 0, tmp);
 			for (i = 0; i < ifp->bNumEndpoints; i++) {
@@ -251,21 +284,19 @@ static int parse_interface(struct libusb_interface *interface,
 
 				if (header.bLength > size) {
 					usbi_err("ran out of descriptors parsing");
-					/* FIXME will leak memory */
-					return -1;
+					r = -ENOSPC;
+					goto err;
 				}
 
 				r = parse_endpoint(endpoint + i, buffer, size);
 				if (r < 0)
-					/* FIXME will leak memory */
-					return r;
+					goto err;
 
 				buffer += r;
 				parsed += r;
 				size -= r;
 			}
-		} else
-			ifp->endpoint = NULL;
+		}
 
 		/* We check to see if it's an alternate to this one */
 		ifp = (struct libusb_interface_descriptor *) buffer;
@@ -276,6 +307,29 @@ static int parse_interface(struct libusb_interface *interface,
 	}
 
 	return parsed;
+err:
+	clear_interface(interface);
+	return r;
+}
+
+static void clear_configuration(struct libusb_config_descriptor *config)
+{
+	if (config->interface) {
+		int i;
+		for (i = 0; i < config->bNumInterfaces; i++)
+			clear_interface((struct libusb_interface *)
+				config->interface + i);
+		free((void *) config->interface);
+	}
+	if (config->extra)
+		free((void *) config->extra);
+}
+
+void usbi_clear_configurations(struct libusb_device *dev)
+{
+	int i;
+	for (i = 0; i < dev->desc.bNumConfigurations; i++)
+		clear_configuration(dev->config + i);
 }
 
 int usbi_parse_configuration(struct libusb_config_descriptor *config,
@@ -293,14 +347,14 @@ int usbi_parse_configuration(struct libusb_config_descriptor *config,
 
 	if (config->bNumInterfaces > USB_MAXINTERFACES) {
 		usbi_err("too many interfaces (%d)", config->bNumInterfaces);
-		return -1;
+		return -EOVERFLOW;
 	}
 
 	tmp = config->bNumInterfaces * sizeof(struct libusb_interface);
 	interface = malloc(tmp);
 	config->interface = interface;
 	if (!config->interface)
-		return -1;      
+		return -ENOMEM;
 
 	memset(interface, 0, tmp);
 	buffer += config->bLength;
@@ -322,7 +376,8 @@ int usbi_parse_configuration(struct libusb_config_descriptor *config,
 			if ((header.bLength > size) ||
 					(header.bLength < DESC_HEADER_LENGTH)) {
 				usbi_err("invalid descriptor length of %d", header.bLength);
-				return -1;
+				r = -EIO;
+				goto err;
 			}
 
 			/* If we find another "proper" descriptor then we're done */
@@ -343,28 +398,30 @@ int usbi_parse_configuration(struct libusb_config_descriptor *config,
 		if (len) {
 			/* FIXME: We should realloc and append here */
 			if (!config->extralen) {
-				unsigned char *extra = malloc(len);
-				config->extra = extra;
-				if (!extra) {
-					config->extralen = 0;
-					/* FIXME will leak memory */
-					return -1;
+				config->extra = malloc(len);
+				if (!config->extra) {
+					r = -ENOMEM;
+					goto err;
 				}
 
-				memcpy(extra, begin, len);
+				memcpy((unsigned char *) config->extra, begin, len);
 				config->extralen = len;
 			}
 		}
 
 		r = parse_interface(interface + i, buffer, size);
 		if (r < 0)
-			return r;
+			goto err;
 
 		buffer += r;
 		size -= r;
 	}
 
 	return size;
+
+err:
+	clear_configuration(config);
+	return r;
 }
 
 /** \ingroup desc
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 737d20c..0e5f617 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -224,6 +224,7 @@ void usbi_handle_transfer_cancellation(struct usbi_transfer *transfer);
 int usbi_parse_descriptor(unsigned char *source, char *descriptor, void *dest);
 int usbi_parse_configuration(struct libusb_config_descriptor *config,
 		unsigned char *buffer);
+void usbi_clear_configurations(struct libusb_device *dev);
 
 /* polling */
 
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index 8226765..6379da9 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -222,9 +222,13 @@ static int initialize_device(struct libusb_device *dev, uint8_t busnum,
 		}
 
 		r = usbi_parse_configuration(&dev->config[i], bigbuffer);
+		free(bigbuffer);
+		if (r < 0) {
+			usbi_err("parse_configuration failed with code %d", r);
+			goto err;
+		}
 		if (r > 0)
 			usbi_warn("descriptor data still left\n");
-		free(bigbuffer);
 	}
 
 	priv->nodepath = strdup(path);
@@ -240,6 +244,7 @@ err:
 	if (fd)
 		close(fd);
 	if (dev->config) {
+		usbi_clear_configurations(dev);
 		free(dev->config);
 		dev->config = NULL;
 	}