Re: [2.4] Memleak in drivers/usb/hub.c::usb_reset_device

[Oleg Drokin]

Hello!

On Fri, Mar 14, 2003 at 11:37:19AM -0800, Greg KH wrote:
> >    There seems to be a memleak in drivers/usb/hub.c::usb_reset_device()
> >    on error exit path. See the patch.
> >    Found with help of smatch + enhanced unfree script.
> And yes, as David said, there is another kind of error in this area for
> 2.5.  Patches to clean that up would be appreciated.

Ok, I guess something like that should work:

Bye,
    Oleg
===== drivers/usb/core/hub.c 1.57 vs edited =====
--- 1.57/drivers/usb/core/hub.c	Wed Mar  5 18:24:34 2003
+++ edited/drivers/usb/core/hub.c	Fri Mar 14 22:59:45 2003
@@ -1175,7 +1175,7 @@
 int usb_reset_device(struct usb_device *dev)
 {
 	struct usb_device *parent = dev->parent;
-	struct usb_device_descriptor descriptor;
+	struct usb_device_descriptor *descriptor;
 	int i, ret, port = -1;
 
 	if (!parent) {
@@ -1224,17 +1224,24 @@
 	 * If nothing changed, we reprogram the configuration and then
 	 * the alternate settings.
 	 */
-	ret = usb_get_descriptor(dev, USB_DT_DEVICE, 0, &descriptor,
-			sizeof(descriptor));
-	if (ret < 0)
+	descriptor = kmalloc(sizeof *descriptor, GFP_NOIO);
+	if (!descriptor) {
+		return -ENOMEM;
+	}
+	ret = usb_get_descriptor(dev, USB_DT_DEVICE, 0, descriptor,
+			sizeof(*descriptor));
+	if (ret < 0) {
+		kfree(descriptor);
 		return ret;
+	}
 
-	le16_to_cpus(&descriptor.bcdUSB);
-	le16_to_cpus(&descriptor.idVendor);
-	le16_to_cpus(&descriptor.idProduct);
-	le16_to_cpus(&descriptor.bcdDevice);
+	le16_to_cpus(&descriptor->bcdUSB);
+	le16_to_cpus(&descriptor->idVendor);
+	le16_to_cpus(&descriptor->idProduct);
+	le16_to_cpus(&descriptor->bcdDevice);
 
-	if (memcmp(&dev->descriptor, &descriptor, sizeof(descriptor))) {
+	if (memcmp(&dev->descriptor, descriptor, sizeof(*descriptor))) {
+		kfree(descriptor);
 		usb_destroy_configuration(dev);
 
 		ret = usb_get_device_descriptor(dev);
@@ -1267,6 +1274,8 @@
 
 		return 1;
 	}
+
+	kfree(descriptor);
 
 	ret = usb_set_configuration(dev, dev->actconfig->desc.bConfigurationValue);
 	if (ret < 0) {