Re: squid-3.HEAD IPAddress leak

From: Adrian Chadd <adrian@dont-contact.us>
Date: Fri, 8 Feb 2008 12:14:51 +0900

On Fri, Feb 08, 2008, Amos Jeffries wrote:

> sockaddr_storage (as Husni uses, and Adrian mentioned) was created to
> provide a better way of using sockaddr* so the sockaddr_in and
> sockaddr_in6 bits could be read-written easily. But the big/litte endian
> problems between OS screwed up the sockaddr_in* sa_family field locations
> inside it so developers still can't portably use it for the v6/v6 flag
> they wanted.

How'd that screw things up? Hm, I'll have to read.

> ... (1) pointer to somewhere holding the sockaddr_storage/*_in/*_in6
> struct sockaddr *ai_addr;

Which you have to allocate.

> char *ai_canonname;

Which we're not using, and you have to allocate.

> ... and because some system lookups in IPv6 can return _multiple_ addresses
> ... it has a linked-list of branchs for each of the alternatives
> struct addrinfo *ai_next; /* Pointer to next in list. */
> };
>

Which is only useful in the context of providing lists of addresses for
a given host or IP address. What getaddrinfo() returns.

> > Why do we need AddrInfo now
>
> To kill dozens of castings, magic operations on object sizes, and cut down
> dozens of lines of 'compatibility code'.

And add in a minimum of one, and a maximum of two allocations per socket
operation.

> Together they make a pretty tree. But every used piece is eseentially
> another new, memset, free.

Ah, and here you will have problems.

The members of that struct should probably be malloc, free, and not
new/delete. You're using new/delete which -should- map to a default
new operator and head off to the malloc libraries, but -squids- idea
of the malloc interface could differ from the -library- idea of
the malloc interface.

So you should probably drop the new/delete'ing of the addrinfo stuff
and replace it with malloc/free.

You're also memset()'ing the addrinfo struct whether you allocated
it or not, which may be double-memset'ing the thing, and if someone
passed in an addrinfo it may have structure members which have now
been leaked.

Adrian
Received on Thu Feb 07 2008 - 20:03:03 MST

This archive was generated by hypermail pre-2.1.9 : Sat Mar 01 2008 - 12:00:09 MST