>
> > So what you're looking for is a path that sets in_addr but
> > not hostFamily...
> > and it's probably in netFlow since you're the only one with
> > problems.

I found the problem to originate in hashHost. Take a breath, here comes the
full story ;)

The problem begins (and I'm not sure whose responsibility it is), when
hashHost is called. When this happens,
myGlobals.otherHostEntry->hostIpAddress.hostFamily is *NEVER* set. My added
debug statement in the beginning of the function always prints
"XXXXX: hashHost init: myGlobals.otherHostEntry->hostIpAddress.hostFamily
not set!"

u_int hashHost(HostAddr *hostIpAddress,  u_char *ether_addr,
               short* useIPAddressForSearching, HostTraffic **el,
               int actualDeviceId) {
  u_int idx = 0;
  *el = NULL;

  if ( (myGlobals.otherHostEntry->hostIpAddress.hostFamily != AF_INET) &&
(myGlobals.otherHostEntry->hostIpAddress.hostFamily != AF_INET6) )
    traceEvent(CONST_TRACE_WARNING, "XXXXX: hashHost init:
myGlobals.otherHostEntry->hostIpAddress.hostFamily not set!");
  else
    traceEvent(CONST_TRACE_WARNING, "XXXXX: hashHost init:
myGlobals.otherHostEntry->hostIpAddress.hostFamily ok");

  if(myGlobals.dontTrustMACaddr)  /* MAC addresses don't make sense here */
    (*useIPAddressForSearching) = 1;

  if((*useIPAddressForSearching) && (hostIpAddress == NULL)) {
#if 0
    traceEvent(CONST_TRACE_WARNING, "Index calculation problem
(hostIpAddress=%x, ether_addr=%x)",
               hostIpAddress, ether_addr);
#endif
    traceEvent(CONST_TRACE_WARNING, "XXXXX: hashHost: returning
FLAG_NO_PEER");
    return(FLAG_NO_PEER);
  }

  if(((*useIPAddressForSearching) == 1)
     || ((ether_addr == NULL) && (hostIpAddress != NULL))) {
    if(myGlobals.trackOnlyLocalHosts
       && (!isLocalAddress(hostIpAddress, actualDeviceId))
       && (!_pseudoLocalAddress(hostIpAddress))) {
      *el = myGlobals.otherHostEntry;
      traceEvent(CONST_TRACE_WARNING, "XXXXX: hashHost: returning
OTHER_HOSTS_ENTRY");
      return(OTHER_HOSTS_ENTRY);
    } else {
      /* idx = hostIpAddress->s_addr; */
      if(hostIpAddress->hostFamily == AF_INET)
        idx = (hostIpAddress->Ip4Address.s_addr & 0xffff) ^
((hostIpAddress->Ip4Address.s_addr >> 15) & 0xffff);
#ifdef INET6
      else if(hostIpAddress->hostFamily == AF_INET6)
        idx = in6_hash(&hostIpAddress->Ip6Address);
#endif
    }

This is bad, because in some cases (like with return(OTHER_HOSTS_ENTRY)),
the hashHost function *will* return with *el = myGlobals.otherHostEntry.

In the lookupHost function:

  idx = hashHost(hostIpAddress, ether_addr,
                 &useIPAddressForSearching,
                 &el, actualDeviceId);

  if(el != NULL)
  {
    return(el); /* Found */
  }

So, with *el != NULL, lookupHost will return immediately to netflowPlugin.c
by this line:

srcHost = lookupHost(&addr2, NULL, 0, 1, myGlobals.netFlowDeviceId);

So, now we have a srcHost which has an hostIpAddress without hostFamily set.
This bombs a few lines later in netflowPlugin.c:

  updatePacketCount(srcHost, &srcHost->hostIpAddress,
                    dstHost, &dstHost->hostIpAddress,
                    ctr, numPkts, actualDeviceId);

updatePacketCount calls addContactedPeers, and addContactedPeers makes since
about a month use of addrcpy to feed an address to IncrementUsageCounter.
addrcpy fails here because of no hostFamily is set, and ergo, counters are
fucked (= what I'm seeing).

So, basically, since the hostFamily isn't in set this path it fucks up the
code later on, which expects this to be set always. And the changes in
pbuf.c addContactedPeers bring this to light.

So, where should the code be fixed?
Should myGlobals.otherHostEntry->hostIpAddress.hostFamily always be set?
Should hashHost fix this up? Or should addContactedPeers be able to expect
an empty hostFamily (which it doesnt now) by checking return value of
addrcpy?

-- Robbert


_______________________________________________
Ntop-dev mailing list
[EMAIL PROTECTED]
http://listgateway.unipi.it/mailman/listinfo/ntop-dev

Reply via email to