Re: tcptrace-bugs Static variable returns of HostName() are not consider by callers

From: Manikantan Ramadas (mramadas@masaka.cs.ohiou.edu)
Date: 01/03/06


Message-Id: <66AD6D22-8134-4FBE-8AF2-2D8727229735@irg.cs.ohiou.edu>
From: Manikantan Ramadas <mramadas@masaka.cs.ohiou.edu>
Subject: Re: tcptrace-bugs Static variable returns of HostName() are not consider by callers
Date: Tue, 3 Jan 2006 23:46:12 -0500


Hi Ilpo,

   You are certainly right in that many Network Socket Library
implementations of gethostbyaddr tend to use static storage
internally. They perhaps cycle thru' a bunch of static memory locations.

    But I think tcptrace is safe on this one. I looked at the
relevant code sections you point out. HostName() function first looks
up its own internal cache before making the expensive gethostbyaddr
call; but since the cainsert routine (used to insert an entry into
the cache upon a cache-miss) makes a copy of the hostname found from
gethostbyaddr, we are protected from cache poisoning.

    Now HostName function either copies an entry found from the cache
into its own internal static variable name_buf and returns it, or
returns the char* as returned from gethostbyaddr. But as far as I can
see, tcptrace code seems cognizant of this fact; Note that all the
per-connection data-structure's hostname entries such as
ptp->a_hostname, ptp->b_hostname (trace.c) make an strdup call on
what the HostName returns. Every where else where HostName() gets
called, it is merely for debug-help printf messages; As long the
HostName char* pointers don't get corrupted within the printf's, we
should be alright.

    We know of folks using tcptrace to process GB dumpfiles;
therefore making such complex memory optimization with statics seem
worth the code-complexity introduced.

- Mani.

On Dec 30, 2005, at 10:24 AM, Ilpo J鋜vinen wrote:

> Hi,
>
> HostName() returns occassionally a pointer to the static variable
> name_buf, and similarly, "gethostbyaddr() may
> return pointers to static data, which may be overwritten
> by later calls. Copying the struct hostent does not suf
> fice, since it contains pointers - a deep copy is
> required." [from manpage]
> Therefore it is invalid to use the return value more than once in a
> printf, like in trace.c (I'm showing just this instance):
>
> if (debug > 3)
> printf("SameAddr(%s(%d),%s(%d)) returns %d\n",
> HostName(*paddr1), ADDR_VERSION(paddr1),
> HostName(*paddr2), ADDR_VERSION(paddr2),
> ret);
>
> Relevant names.c part:
>
> HostName(
> ipaddr ipaddress)
> {
> tcelen len;
> static int cache = -1;
> struct hostent *phe;
> char *sb_host;
> static char name_buf[100];
>
> [...snip...]
> if (calookup(cache,
> (char *) &ipaddress, (tcelen) sizeof(ipaddress),
> (char *) name_buf, &len) == OK) {
> if (debug > 2)
> fprintf(stderr,"Found host %s='%s' in cache\n",
> adr, name_buf);
> return(name_buf);
> }
>
>
> if (ADDR_ISV6(&ipaddress))
> phe = gethostbyaddr ((char *)&ipaddress.un.ip6,
> sizeof(ipaddress.un.ip6), AF_INET6);
> else
> phe = gethostbyaddr((char *)&ipaddress.un.ip4,
> sizeof(ipaddress.un.ip4), AF_INET);
> if (phe != NULL) {
> sb_host = phe->h_name;
> } else {
> sb_host = adr;
> }
>
> [...snip...]
>
> return(sb_host);
> }
>
>
> Also other statics, when returned, have similar hazards...
>
>
> --
> i.

--
"The quieter you become, the more you can hear." - Baba Ram Dass.
____________________________________________________________________
* Manikantan Ramadas * IRG, OU * http://irg.cs.ohiou.edu/~mramadas *
____________________________________________________________________




This archive was generated by hypermail 2.1.7 : 01/04/06 EST