Ticket #27 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

Libusb 1.0.6 Big Endian issue

Reported by: mig_143 Owned by:
Milestone: Component: libusb-1.0
Keywords: Big Endian Cc:
Blocked By: Blocks:

Description

Hi,
I am running Libusb 1.0.6 on x86 (little endian) board. It works fine.

But when I run the same on Mips (Big Endian) it doesn't works.

I investigated and found in Descriptor.c file functions like usbi_parse_descriptor, parse_endpoint, parse_interface ... etc. seems to have endian flag (int host_endian) in function arguments, but calling functions are not taking care about the endian issue.

I searched the internet and got one suggestion
I added the following snippet, in above said functions

#ifdef BIG_ENDIAN
host_endian = 0;
#else
host_endian = 1;
#endif

and after that libusb is working fine.

Libusb should handle this also.

Thanks

Mohit

code snippet ---->

int usbi_parse_descriptor(unsigned char *source, char *descriptor, void *dest,

int host_endian)

{

unsigned char *sp = source, *dp = dest;
uint16_t w;
char *cp;

#ifdef BIG_ENDIAN
host_endian = 0;
#else
host_endian = 1;
#endif

for (cp = descriptor; *cp; cp++) {

switch (*cp) {

case 'b': /* 8-bit byte */

*dp++ = *sp++;
break;

case 'w': /* 16-bit word, convert from little endian to CPU */

dp += ((unsigned long)dp & 1); /* Align to word boundary */

if (host_endian) {

memcpy(dp, sp, 2);

} else {

w = (sp[1] << 8) | sp[0];
*((uint16_t *)dp) = w;

}
sp += 2;
dp += 2;
break;

}

}

return sp - source;

}

Change History

comment:1 Changed 3 years ago by mig_143

  • Summary changed from Libusb 1.0.6 don't caring about big endian issue to Libusb 1.0.6 Big Endian issue

comment:2 follow-up: Changed 3 years ago by dsd

What do you mean by "it doesn't work" ?

comment:3 in reply to: ↑ 2 Changed 3 years ago by mig_143

Replying to dsd:

What do you mean by "it doesn't work" ?

It does not enumerate the device and returns error.
That happens because it is not handling big-endian issue properly.
Like in our case device descriptor lenth is 0x19 (25) bytes, but because while parsing the device descriptor it issues a request for 0x1900 (6400) bytes.
Device returns 25 bytes but it returns error saying --->

libusb:error [get_config_descriptor] short output read 25/6400.

You can also verify the same, running Libusb on some big endian machine.

Also access this link-->
http://old.nabble.com/build-libusb-for-big-endian-target---how-to-and-a-recommended-patch-td22956999.html

Further see my previous comments for solution I have for this.

Thanks
Mohit

comment:4 Changed 3 years ago by mig_143

Today I find a better way to put this patch... At run time it will check the plaform for big-endian and little endian.

If I am adding only this patch, in usbi_parse_descriptor function in descriptor.c file, all the problems are solved.

I found in the code, many functions are calling usbi_parse_descriptor with a hardcoded valude for host_endian (0 or 1).

Please check the following code snippet-->

int usbi_parse_descriptor(unsigned char *source, char *descriptor, void *dest,

int host_endian)

{

unsigned char *sp = source, *dp = dest;
uint16_t w;
char *cp;

#ifdef GET_ENDIAN

w = 1;
host_endian = *(unsigned char *)(& w);

#endif

for (cp = descriptor; *cp; cp++) {

switch (*cp) {

case 'b': /* 8-bit byte */

*dp++ = *sp++;
break;

Thanks

Mohit

comment:5 Changed 3 years ago by dsd

The host_endian flag means that we expect the data to be in the endianness of the host, so conversion is never necessary. If this is not correct for a particular case then the caller should be fixed, we should not break the concept of host endianness for every single caller.

If you look at the comments in the file you can see it is a bit of a delicate area and there are multiple codepaths for reading the descriptor info, so this needs looking at in more detail. It's unlikely that the correct fix would touch the usbi_parse_descriptor() function.

comment:6 Changed 3 years ago by mig_143

ya you are right. callers should be fixed for that. As you told multiple codepaths are there for reading the descriptor info, they should pass the correct value for host_endian parameter while calling the "usbi_parse_descriptor" function.

But they are not doing that. And all those callers are part of libusb code. In most of the places they are passing hard coded value (0 or 1). So lot of changes will have to made. So for fixing it with the smallest code possible I tried putting in the usbi_parse_descriptor function.

I hope in next release of libusb this issue will be taken care

comment:7 Changed 3 years ago by dsd

It seems like you don't understand what is meant by host endian - it means that the data is expected to be already in the endianness of the host, therefore no conversion is necessary *on any system* regardless of its endianness. Because host endianness is not something that varies by platform, it is intentional that those 0s and 1s are hardcoded. What you are probably seeing is a small detail where we have overlooked a small detail and there is a 1 instead of a 0 or something like that.

comment:8 Changed 3 years ago by mig_143

So according to you no conversion is required.
Can you tell me, if I have to run libusb 1.0.6 on a Big-Endian machine do I need to configure any flag or option before compiling the code ?

comment:9 Changed 3 years ago by dsd

I'm saying that there is somewhere in the code that is doing a conversion where none is required, or not doing a conversion where one is required. This will likely just be a 0 or 1 flag set incorrectly. Figuring out which callsite is the source of this is the first step towards the solutoin.

comment:10 Changed 3 years ago by mig_143

I have removed the patch from usbi_parse_descriptor function (descriptor.c file), as I suggested earlier.

I have modified in Linux_usbfs.c file. In function -->

static int cache_active_config(struct libusb_device *dev, int fd, int active_config)

usbi_parse_descriptor is called with host_endian value as 1 -->
usbi_parse_descriptor(tmp, "bbw", &config, 1);

I changed it to 0--> usbi_parse_descriptor(tmp, "bbw", &config, 0);

and it works on both the Hardware platforms i.e. Little Endian as well Big-Endian.

comment:11 Changed 3 years ago by dsd

yep, thats half of it. Should be fully fixed in commit 02df59a309e813c50b8230de99e69fb4e1814279 for the next release

comment:12 Changed 3 years ago by dsd

  • Resolution set to fixed
  • Status changed from new to closed

fixed in v1.0.7

Note: See TracTickets for help on using tickets.