Hi Dave, please apply the appended patch.
I somehow thought I had fixed this quite some time ago. Probably I lost
it with some merge :(
Thanks,
--
- Harald Welte <[email protected]> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
On Sat, Sep 03, 2005 at 10:43:15AM +0200, Harald Welte wrote:
> htonll() is nothing else than cpu_to_be64(), so we'd rather call the
> latter.
Actually, the htonll() implementation does not seem to be doing what
cpu_to_be64() is doing.. However, I would assume this is a bug in
htonll() and this change to use cpu_to_be64() is fixing that. Can this
bug cause any major problems in the current implementation?
> -u_int64_t htonll(u_int64_t in)
> -{
> - u_int64_t out;
> - int i;
> -
> - for (i = 0; i < sizeof(u_int64_t); i++)
> - ((u_int8_t *)&out)[sizeof(u_int64_t)-1] = ((u_int8_t *)&in)[i];
I would assume that the first index should have had '-i' added to it, if
the purpose is to swap byte order.. The code here is leaving some
arbitrary data in 7 bytes of the 64-bit variable and setting
(u8*)&out[7] = (u8*)&in[7] in somewhat inefficient way ;-). In addition,
this looks more like swap-8-bytes-unconditionally than doing this based
on host byte order..
--
Jouni Malinen PGP id EFC895FA
On Sat, Sep 03, 2005 at 09:54:25AM -0700, Jouni Malinen wrote:
> On Sat, Sep 03, 2005 at 10:43:15AM +0200, Harald Welte wrote:
>
> > htonll() is nothing else than cpu_to_be64(), so we'd rather call the
> > latter.
>
> Actually, the htonll() implementation does not seem to be doing what
> cpu_to_be64() is doing.. However, I would assume this is a bug in
> htonll() and this change to use cpu_to_be64() is fixing that.
ACK.
> Can this bug cause any major problems in the current implementation?
the "current implementation" was only merged after 2.6.13 is released,
so I doubt anyone but the netfilter developers is using it yet.
> I would assume that the first index should have had '-i' added to it, if
> the purpose is to swap byte order.. The code here is leaving some
> arbitrary data in 7 bytes of the 64-bit variable and setting
> (u8*)&out[7] = (u8*)&in[7] in somewhat inefficient way ;-). In addition,
> this looks more like swap-8-bytes-unconditionally than doing this based
> on host byte order..
yes, yes, yes. Somehow this ancient buggy implementation slipped into
mainline. I know I had fixed this before.
So please let's all forget about this embarrassing htonll() and move on.
--
- Harald Welte <[email protected]> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie