Beattie, David
2004-10-15 09:21:16 UTC
Hi all,
In libnet-1.1.2, one of the changelog items says:
Fixed a bug on big endian boxes that had TCP and UDP checksums with
odd
payloads come out incorrect
I just upgraded to libnet-1.1.2, and I started immediately having
trouble with checksums on my (little-endian, but that's irrelevant as
you will see) box, with odd-sized tcp payloads. So, I decided to look
at the code, and found it to be clearly wrong-it can overrun the end of
an odd-sized buffer and grab a random value from underlying memory and
add it into the checksum. Here's a possible fix with minimal changes
from the current released code (this is what I'm using now):
libnet_in_cksum(u_int16_t *addr, int len)
{
int sum;
union {
u_int16_t s;
u_int8_t b[2];
} pad;
sum = 0;
while (len > 1)
{
sum += *addr++;
len -= 2;
}
if (len == 1)
{
pad.b[0] = *(u_int8_t *)addr;
pad.b[1] = 0;
sum += pad.s;
}
return (sum);
}
I haven't tested on any big-endian systems, but I imagine that it would
work. It definitely solves my problem on my little-endian system, and
seems to be fundamentally correct, if my assumption that the compiler
will always pack elements in a union at the start of the union's memory
is correct. Of course, if the compiler can't do the union in a register
(anybody know?), then my way involves a memory-to-memory copy, and
therefore is not efficient... so the really efficient way may be an
#ifdef block per endian style.
As a suggestion, in my opinion, the parameter to libnet_in_cksum would
more naturally be a char * or u_int8_t *, and the cast to a u_int16_t *
would be done inside libnet_in_cksum while
dereferencing/autoincrementing. This would save a bunch of casting
elsewhere in this file. It's not important though-just a question of
style and personal preference.
--David
In libnet-1.1.2, one of the changelog items says:
Fixed a bug on big endian boxes that had TCP and UDP checksums with
odd
payloads come out incorrect
I just upgraded to libnet-1.1.2, and I started immediately having
trouble with checksums on my (little-endian, but that's irrelevant as
you will see) box, with odd-sized tcp payloads. So, I decided to look
at the code, and found it to be clearly wrong-it can overrun the end of
an odd-sized buffer and grab a random value from underlying memory and
add it into the checksum. Here's a possible fix with minimal changes
from the current released code (this is what I'm using now):
libnet_in_cksum(u_int16_t *addr, int len)
{
int sum;
union {
u_int16_t s;
u_int8_t b[2];
} pad;
sum = 0;
while (len > 1)
{
sum += *addr++;
len -= 2;
}
if (len == 1)
{
pad.b[0] = *(u_int8_t *)addr;
pad.b[1] = 0;
sum += pad.s;
}
return (sum);
}
I haven't tested on any big-endian systems, but I imagine that it would
work. It definitely solves my problem on my little-endian system, and
seems to be fundamentally correct, if my assumption that the compiler
will always pack elements in a union at the start of the union's memory
is correct. Of course, if the compiler can't do the union in a register
(anybody know?), then my way involves a memory-to-memory copy, and
therefore is not efficient... so the really efficient way may be an
#ifdef block per endian style.
As a suggestion, in my opinion, the parameter to libnet_in_cksum would
more naturally be a char * or u_int8_t *, and the cast to a u_int16_t *
would be done inside libnet_in_cksum while
dereferencing/autoincrementing. This would save a bunch of casting
elsewhere in this file. It's not important though-just a question of
style and personal preference.
--David