# This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.743 -> 1.744 # drivers/usb/serial/usbserial.c 1.34 -> 1.35 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/07/19 stuartm@connecttech.com 1.744 # [PATCH] USB: usbserial.c fixup # # create_serial, get_free_serial and usb_serial_probe all do pretty much # the same thing. I'd like to reorg this into create_serial does all the # alloc and most of the setup, and get_free_serial just fills in the # MAGIC. # # There's currently a memory leak: if create_serial is called at probe # time or calc_ports time, and then get_free_serial returns NULL because # the table has no entries left, that usb_serial struct is leaked. # # get_free_serial doesn't check properly for free slots. The middle loop # doesn't terminate when the end of the table is reached, although the # assignment loop later does. The effect is that stuff past the end of # the table is allowed to decide if there's free space or not, and # occasionally it'll say "yes" and then the assignment loop will only # allocate slots up to the end of the table, preventing memory # scribbling. # # I haven't fixed any of this just yet because I'm not sure what the # intended behaviour is. Should get_free_serial allocate as many slots # as possible, or just be all or nothing? Similarly, I don't see a # problem with calling create_serial early in usb_serial_probe, and # removing the alloc code from get_free_serial; this would fix the leak. # # Ah heck, here's a patch. This is what I think things should look like. # get_free_serial is all or none, the leak is fixed and create_serial # does all the allocation. # -------------------------------------------- # diff -Nru a/drivers/usb/serial/usbserial.c b/drivers/usb/serial/usbserial.c --- a/drivers/usb/serial/usbserial.c Fri Jul 19 11:04:04 2002 +++ b/drivers/usb/serial/usbserial.c Fri Jul 19 11:04:04 2002 @@ -447,24 +447,18 @@ good_spot = 1; for (j = 1; j <= num_ports-1; ++j) - if (serial_table[i+j]) + if ((serial_table[i+j]) || (i+j >= SERIAL_TTY_MINORS)) { good_spot = 0; + i += j; + break; + } if (good_spot == 0) continue; - if (!serial) { - serial = kmalloc(sizeof(*serial), GFP_KERNEL); - if (!serial) { - err(__FUNCTION__ " - Out of memory"); - return NULL; - } - memset(serial, 0, sizeof(*serial)); - } serial->magic = USB_SERIAL_MAGIC; - serial_table[i] = serial; *minor = i; dbg(__FUNCTION__ " - minor base = %d", *minor); - for (i = *minor+1; (i < (*minor + num_ports)) && (i < SERIAL_TTY_MINORS); ++i) + for (i = *minor; (i < (*minor + num_ports)) && (i < SERIAL_TTY_MINORS); ++i) serial_table[i] = serial; return serial; } @@ -1207,14 +1201,14 @@ return(NULL); } + serial = create_serial (dev, interface, type); + if (!serial) { + err ("%s - out of memory", __FUNCTION__); + return NULL; + } + /* if this device type has a probe function, call it */ if (type->probe) { - serial = create_serial (dev, interface, type); - if (!serial) { - err ("%s - out of memory", __FUNCTION__); - return NULL; - } - if (type->owner) __MOD_INC_USE_COUNT(type->owner); retval = type->probe (serial); @@ -1293,6 +1287,7 @@ num_ports = num_bulk_out; if (num_ports == 0) { err("Generic device with no bulk out, not allowed."); + kfree (serial); return NULL; } } @@ -1300,14 +1295,6 @@ if (!num_ports) { /* if this device type has a calc_num_ports function, call it */ if (type->calc_num_ports) { - if (!serial) { - serial = create_serial (dev, interface, type); - if (!serial) { - err ("%s - out of memory", __FUNCTION__); - return NULL; - } - } - if (type->owner) __MOD_INC_USE_COUNT(type->owner); num_ports = type->calc_num_ports (serial); @@ -1318,22 +1305,17 @@ num_ports = type->num_ports; } - serial = get_free_serial (serial, num_ports, &minor); - if (serial == NULL) { + if (get_free_serial (serial, num_ports, &minor) == NULL) { err("No more free serial devices"); + kfree (serial); return NULL; } - serial->dev = dev; - serial->type = type; - serial->interface = interface; serial->minor = minor; serial->num_ports = num_ports; serial->num_bulk_in = num_bulk_in; serial->num_bulk_out = num_bulk_out; serial->num_interrupt_in = num_interrupt_in; - serial->vendor = dev->descriptor.idVendor; - serial->product = dev->descriptor.idProduct; /* set up the endpoint information */ for (i = 0; i < num_bulk_in; ++i) {