On Wed, 25 May 2005 11:54:44 +0100, Ian Abbott <[EMAIL PROTECTED]> wrote:
> > Bug inside bugfix.
>
> Please see attached patch for 2.6. It might also apply to
> 2.4.31-pre2-bk4 with some fuzz, but I haven't tried it.
> - if (unicode_size <= 0)
> - return;
> + if (unicode_size < 0)
> + unicode_size = 0;
>
> for (i = 0; i < unicode_size; ++i)
> string[i] = (char)(le16_to_cpu(unicode[i]));
I do not like this either. What does happen when the unicode_size is
larger than 30? Someone was in a big hurry pushing "security" fix out.
But let's fix it properly, shall we?
Please see how the math is made more elegant. Plus, all sorts of little
bugs were removed. Notice how the last character of the manufacturer's
name was always overwritten with space. Now, it's not (unless zero).
diff -urp -X dontdiff linux-2.6.12-rc4/drivers/usb/serial/io_edgeport.c
linux-2.6.12-rc4-lem/drivers/usb/serial/io_edgeport.c
--- linux-2.6.12-rc4/drivers/usb/serial/io_edgeport.c 2005-05-14
00:02:46.000000000 -0700
+++ linux-2.6.12-rc4-lem/drivers/usb/serial/io_edgeport.c 2005-05-25
09:48:28.000000000 -0700
@@ -361,7 +361,7 @@ struct edgeport_port {
/* This structure holds all of the individual device information */
struct edgeport_serial {
- char name[MAX_NAME_LEN+1]; /* string name
of this device */
+ char name[MAX_NAME_LEN+2]; /* string name
of this device */
struct edge_manuf_descriptor manuf_descriptor; /* the
manufacturer descriptor */
struct edge_boot_descriptor boot_descriptor; /* the boot
firmware descriptor */
@@ -489,7 +489,7 @@ static void get_manufacturing_desc (stru
static void get_boot_desc (struct edgeport_serial *edge_serial);
static void load_application_firmware (struct edgeport_serial *edge_serial);
-static void unicode_to_ascii (char *string, __le16 *unicode, int
unicode_size);
+static void unicode_to_ascii(char *string, int buflen, __le16 *unicode, int
unicode_size);
// ************************************************************************
@@ -592,7 +592,7 @@ static void update_edgeport_E2PROM (stru
* Get string descriptor from device *
* *
************************************************************************/
-static int get_string (struct usb_device *dev, int Id, char *string)
+static int get_string (struct usb_device *dev, int Id, char *string, int
buflen)
{
struct usb_string_descriptor StringDesc;
struct usb_string_descriptor *pStringDesc;
@@ -614,7 +614,7 @@ static int get_string (struct usb_device
return 0;
}
- unicode_to_ascii(string, pStringDesc->wData,
pStringDesc->bLength/2-1);
+ unicode_to_ascii(string, buflen, pStringDesc->wData,
pStringDesc->bLength/2);
kfree(pStringDesc);
return strlen(string);
@@ -2789,16 +2789,20 @@ static void change_port_settings (struct
* ASCII range, but it's only for debugging...
* NOTE: expects the unicode in LE format
****************************************************************************/
-static void unicode_to_ascii (char *string, __le16 *unicode, int unicode_size)
+static void unicode_to_ascii(char *string, int buflen, __le16 *unicode, int
unicode_size)
{
int i;
- if (unicode_size <= 0)
+ if (buflen <= 0) /* never happens, but... */
return;
+ --buflen; /* space for nul */
- for (i = 0; i < unicode_size; ++i)
+ for (i = 0; i < unicode_size; i++) {
+ if (i >= buflen)
+ break;
string[i] = (char)(le16_to_cpu(unicode[i]));
- string[unicode_size] = 0x00;
+ }
+ string[i] = 0x00;
}
@@ -2828,11 +2832,17 @@ static void get_manufacturing_desc (stru
dbg(" BoardRev: %d",
edge_serial->manuf_descriptor.BoardRev);
dbg(" NumPorts: %d",
edge_serial->manuf_descriptor.NumPorts);
dbg(" DescDate: %d/%d/%d",
edge_serial->manuf_descriptor.DescDate[0],
edge_serial->manuf_descriptor.DescDate[1],
edge_serial->manuf_descriptor.DescDate[2]+1900);
- unicode_to_ascii (string,
edge_serial->manuf_descriptor.SerialNumber,
edge_serial->manuf_descriptor.SerNumLength/2-1);
+ unicode_to_ascii(string, 30,
+ edge_serial->manuf_descriptor.SerialNumber,
+ edge_serial->manuf_descriptor.SerNumLength/2);
dbg(" SerialNumber: %s", string);
- unicode_to_ascii (string,
edge_serial->manuf_descriptor.AssemblyNumber,
edge_serial->manuf_descriptor.AssemblyNumLength/2-1);
+ unicode_to_ascii(string, 30,
+ edge_serial->manuf_descriptor.AssemblyNumber,
+ edge_serial->manuf_descriptor.AssemblyNumLength/2);
dbg(" AssemblyNumber: %s", string);
- unicode_to_ascii (string,
edge_serial->manuf_descriptor.OemAssyNumber,
edge_serial->manuf_descriptor.OemAssyNumLength/2-1);
+ unicode_to_ascii(string, 30,
+ edge_serial->manuf_descriptor.OemAssyNumber,
+ edge_serial->manuf_descriptor.OemAssyNumLength/2);
dbg(" OemAssyNumber: %s", string);
dbg(" UartType: %d",
edge_serial->manuf_descriptor.UartType);
dbg(" IonPid: %d",
edge_serial->manuf_descriptor.IonPid);
@@ -2961,11 +2971,11 @@ static int edge_startup (struct usb_seri
usb_set_serial_data(serial, edge_serial);
/* get the name for the device from the device */
- if ( (i = get_string(dev, dev->descriptor.iManufacturer,
&edge_serial->name[0])) != 0) {
- edge_serial->name[i-1] = ' ';
- }
-
- get_string(dev, dev->descriptor.iProduct, &edge_serial->name[i]);
+ i = get_string(dev, dev->descriptor.iManufacturer,
+ &edge_serial->name[0], MAX_NAME_LEN+1);
+ edge_serial->name[i++] = ' ';
+ get_string(dev, dev->descriptor.iProduct,
+ &edge_serial->name[i], MAX_NAME_LEN+2 - i);
dev_info(&serial->dev->dev, "%s detected\n", edge_serial->name);
All this only goes to show that someone should have used usb_string()
instead of having own get_string().
I think I should save this little problem for interviews.
It's all about the good taste and good traditions, down to "++i".
But most importantly, Ian, I challenge you to find a corner case which
the patch above does not process right. Apply to your tree only if
you're unable! Otherwise, send me a scenario where it fails.
Cheers,
-- Pete
-------------------------------------------------------
SF.Net email is sponsored by: GoToMeeting - the easiest way to collaborate
online with coworkers and clients while avoiding the high cost of travel and
communications. There is no equipment to buy and you can meet as often as
you want. Try it free.http://ads.osdn.com/?ad_id=7402&alloc_id=16135&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
