On Thu, Jul 09, 2009 at 11:04:05AM -0400, Alan Stern wrote: > Your "USB: allow malformed LANGID descriptors" patch (commit > b7af0bb26899bb47ae16fb41d2296111b0784a56) is causing a regression: > > http://bugzilla.kernel.org/show_bug.cgi?id=13700 > > The usbmon log attached to comment #11 shows how the 5-2 device > doesn't work with this patch. It rejects attempts to read string 0 > (the language ID list) with a STALL, and it crashes when the kernel > attempts to read a string using language 0. Ok, let me get this straight. The device STALLS EP0 when asked for string descriptor 0, so it tells it's not willing to hand out any string at all. Haven't checked with the USB specs whether this is correct behaviour, but this is what it looks like. When an endpoint communication stalls, the return value is -EPIPE, and in case of a malformed string descriptor we seem to be getting -ENODATA. Handling those two case seperately should make both sides happy. Could you test the patch below? It's only compile tested for now. Thanks, Daniel diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 2bed83c..5deb286 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -840,20 +840,29 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size) /* get langid for strings if it's not yet known */ if (!dev->have_langid) { err = usb_string_sub(dev, 0, 0, tbuf); - if (err < 0) { + + if (err == -ENODATA || err < 4) { + /* the string was reported, but is malformed. + * default to english in this case */ + dev->string_langid = 0x0409; + dev->have_langid = 1; + dev_err(&dev->dev, + "string descriptor 0 malformed (err = %d), " + "defaulting to 0x%04x\n", + err, dev->string_langid); + } else if (err < 0) { + /* in case of all other errors, don't set have_langid + * so we won't retrieve any strings from the device */ dev_err(&dev->dev, "string descriptor 0 read error: %d\n", err); - } else if (err < 4) { - dev_err(&dev->dev, "string descriptor 0 too short\n"); } else { dev->string_langid = tbuf[2] | (tbuf[3] << 8); + dev->have_langid = 1; /* always use the first langid listed */ dev_dbg(&dev->dev, "default language 0x%04x\n", dev->string_langid); } - - dev->have_langid = 1; } err = usb_string_sub(dev, dev->string_langid, index, tbuf); -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
