Greg Kurz <gr...@kaod.org> writes: ... >> >> I think there's an underlying problem with this code which we >> should deal with differently. The 'dataset' local in this >> file is (I think) pointing at on-the-wire information from >> the USB device, but we're treating it as an array of >> host-order uint16_t values. Is this really correct on a >> big-endian host ? > > I don't know much about usb-mtp and the MTP spec says: > > https://theta360blog.files.wordpress.com/2016/04/mtpforusb-ifv1-1.pdf > > 3.1.1 Multi-byte Data > > The standard format for multi-byte data in this specification is > big-endian. That is, the bits within a byte will be read such that > the most significant byte is read first. The actual multi-byte data > sent over the transport may not necessarily adhere to this same > format, and the actual multi-byte data used on the devices may also > use a different multi-byte format. The big-endian convention only > applies within this document, except where otherwise stated. > > So I'm not sure about what the code should really do here... :-\ >
If I remember correctly, with USB transport, multibyte values are little endian and it supersedes the MTP spec? (which is why the code works as expected on a little endian host). As Peter said, some byte swapping is probably needed for this to work on big endian hosts. >> Do we do the right thing if we are >> passed a malicious USB packet that ends halfway through a >> utf16_t character, or do we index off the end of the packet >> data ? >> > > Can you elaborate ? > >> I think that we should define the "filename" field in >> ObjectInfo to be a uint8_t array, make utf16_to_str() >> take a uint8_t* for its data array, and have it do the >> reading of data from the array with lduw_he_p(), which >> can handle accessing unaligned data. >> >> We should also check what the endianness of other fields in >> the ObjectInfo struct is (eg "format" and "size" and see >> whether we should be doing byte swapping here. >> > > I don't have any idea on that... the code just seems to assume > everything is host endian. > >> PS: it is a bit confusing that in this function the local >> variable "dataset" is a pointer to a struct of entirely >> different type to the one that s->dataset is. >> > > Maybe Gerd or Bandan can comment on that. > >> thanks >> -- PMM