open-iscsi / target-isns

Target-isns is an iSNS client for the Linux LIO iSCSI target
GNU General Public License v2.0
16 stars 19 forks source link

Handle big-endian architecture #27

Closed gonzoleeman closed 9 years ago

gonzoleeman commented 9 years ago

On a little Endian system, this will load addr with the IP address in "wrong" order, ie if the address was 1.2.3.4, then addr will contain 04030201

    ip[10] = ip[11] = 0xff;
    ip[15] = 0xff & (addr >> 24);
    ip[14] = 0xff & (addr >> 16);
    ip[13] = 0xff & (addr >> 8);
    ip[12] = 0xff & addr;

Using the example IP addr above, his will translate to

    /* addr == 0x04030201 */
    ip[15] = 4; /* 0xff & (addr >> 24); */
    ip[14] = 3; /* 0xff & (addr >> 16); */
    ip[13] = 2; /* 0xff & (addr >> 8); */
    ip[12] = 1; /* 0xff & addr; */

On a big Endian system, the contents of addr will not be byte swapped, and hence it will be stored in ip[] in the wrong order.

This picture doesn't change when using ntohl, because that is a NOP on s390. The only effect of using ntohl is that it will now be broken on x86_64 as well :-)

The real fix is to not bother with assigning to addr at all, but to do a memcpy straight from &l.s4)->sin_addr to ip + 12. That doesn't need any byte aerobics, as it just moves the data as-is.

Signed-of-by: Lee Duncan lduncan@suse.com

gonzoleeman commented 9 years ago

This pull request should be correct. The previous one erroniously removed the assignment of ip[10] and ip[11] to oxFF.

cvubrugier commented 9 years ago

Thank you Lee and Olaf! Really nice bug fix that even removes some code!