> 1) It gets a SIGBUS on my Solaris box as it tries to do an unaligned
> access referencing a long value in the packet
Even on processors that *can* do unaligned access to 2-byte or 4-byte
integral quantities, doing so works only if the protocol in question is
specified to store those quantities in the byte order of the machine on
which you're going to read network traces - which doesn't work very well
if you're going to, say, analyze the trace on both a PC and a Sun. :-)
The code that I see that does that sort of casting (I searched for "*("
in "packet-ntp.c") deals with the reference clock ID:
/* Now, there is a problem with secondary servers. Standards asks
* from stratum-2 - stratum-15 servers to set this to the low order
* 32 bits of the latest transmit timestamp of the reference source.
* But, all V3 and V4 servers set this to IP adress of their higher
* level server. My decision was to resolve this address.
*/
if (*pkt->stratum <= 1) {
strcpy (buff, "unindentified reference source");
for (i = 0; primary_sources[i].id; i++)
if (*((guint32 *) pkt->refid) == *((guint32 *) primary_sources[i].id))
strcpy (buff, primary_sources[i].data);
} else strcpy (buff, get_hostname (*((u_int *) pkt->refid)));
The problems with this code are:
1) "primary_sources[]" is an array of structures whose two
members are "char *"s - comparing the reference clock ID
with the lower 32 bits of a random address in Ethereal's
address space doesn't make sense;
2) IP addresses are presumably in network byte order - if so,
the "get_hostname()" should do
get_hostname (pntohl(pkt->refid))
so that it works correctly regardless of the byte order of
the machine on which you're running Ethereal (that also
eliminates the alignment problem, as "pntohl()" doesn't
require that its argument point to an address aligned on any
particular boundary).
> I haven't finished looking at it, but most of the payload portion of
> the packet should probably be copied out to avoid this type of problem.
Unless there's such a casting that I've missed (and probably even if
there *is* such a casting), that's not necessary - adding the "pntohl()"
is not only necessary if the reference clock ID truly is an IP address
in network byte order, it eliminates the need for said copy.
Similarly, comparing it with the lower 32 bits of a random pointer in
the address space of Ethereal is bogus, so presumably that code would be
rewritten not to do that - in which case it should be rewritten to
extract that field in some fashion that
1) doesn't depend on it being aligned
and
2) puts it in the right byte order if it's in a standard byte
order.
However:
> 2) There is a comment in the code that all v3 and v4 servers (if not
> stratum 1) set the reference identifier field to the IP address of their
> higher-level server. Before attempting a hostname conversion, there is
> no check that this packet is FROM a server (in my case, it's from a
> client request packet, and I have NO idea what's in this field).
> Secondly, I'm not comfortable with the claim that ALL non-stratum-1
> servers set this field to an IP address.
if, in fact, that field is specified only to be an opaque 4-byte field,
perhaps it should simply be dissected as a byte array.
