[U-Boot] [PATCH v2] usb: Use well-known descriptor sizes when parsing configuration

[Julius Werner]

> From a security / robustness standpoint,
>
> - if the descriptor length field is found to be abnormal, then the code
>   should not process the packet at all. Here it seems it only warns
>   then goes on to use the descriptor.

Weren't you the guy who was so worried about poor Chinese devices who
don't follow the spec just two mails ago? This code just tries to make
the most of a broken response while keeping the host safe. Since the
descriptors are just put into a structure and left there for the
driver to decide which ones it's interested in anyway, more is usually
better than less.

> - if for some reason (although I don't see any) the descriptor must be
>   processed despite its wrong length, then only the bytes at positions
>   0..length-1 should be considered; bytes beyond the descriptor's
>   announced length should be deemed undefined (and, I might add,
>   should not be even be read from the device).

This is not reading from the device, it's memcpy()'ing between an
already read buffer and another data structure. memcpy()'ing one byte
less into an uninitialized struct doesn't make the last byte any less
undefined than copying garbage.

> Does this change fall into either "using the well-defined length"
> or "adding safety to out-of-order descriptors" category? If none, then
> there should be a third category of change documented in the commit
> message.

It's another thing about the length... it prevents the code from
running into an endless loop when it encounters a descriptor with
length 0.

>> -     if (usb_get_hub_descriptor(dev, buffer, descriptor->bLength) < 0) {
>> +     if (usb_get_hub_descriptor(dev, buffer, length) < 0) {
>>               debug("usb_hub_configure: failed to get hub " \
>>                     "descriptor 2nd giving up %lX\n", dev->status);
>
> This message seems less clear than the one it replaces; I suspect
> "failed to get hub descriptor 2nd giving up" is not a valid sentence.

I'm only touching the condition... if you don't like the message, I'd
be happy to review your patch for it.