Hi, On 05/30/2013 04:51 PM, Alan Stern wrote:
On Thu, 30 May 2013, Hans de Goede wrote:While reading the config parsing code I noticed this check is missing, without this check config->desc.wTotalLength can end up with a value larger then the dev->rawdescriptors length for the config, and when userspace then tries to get the rawdescriptors bad things may happen. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- drivers/usb/core/config.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index 7199adc..a6b2cab 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -424,7 +424,8 @@ static int usb_parse_configuration(struct usb_device *dev, int cfgidx, memcpy(&config->desc, buffer, USB_DT_CONFIG_SIZE); if (config->desc.bDescriptorType != USB_DT_CONFIG || - config->desc.bLength < USB_DT_CONFIG_SIZE) { + config->desc.bLength < USB_DT_CONFIG_SIZE || + config->desc.bLength > size) { dev_err(ddev, "invalid descriptor for config index %d: " "type = 0x%X, length = %d\n", cfgidx, config->desc.bDescriptorType, config->desc.bLength);Another thing that should be changed along with this is in line 710: } else if (result < 4) { really should be } else if (result != USB_DT_CONFIG_SIZE) { Otherwise we may read a garbage value for desc->wTotalLength.
Erm, no wTotalLength is byte 3 and 4, so the original check covers it. I actually had a patch such as you suggested, but then thought it would be better to leave things as is, so as to avoid regressions, in case some crazy device has result >= 4 && result < USB_DT_CONFIG_SIZE on the first (header only) usb_get_descriptor call. For the second (full) usb_get_descriptor call, my patch already ensures that result >= USB_DT_CONFIG_SIZE. Regards, Hans -- 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
