Re: squid-3.HEAD IPAddress leak

Amos Jeffries wrote:
> Alex Rousskov wrote:
>> On Fri, 2008-02-08 at 15:52 +1300, Amos Jeffries wrote:
>>
>>> The second;
>>> 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.
>>
>> Do you have a reference for that? I do not want to bug you with more
>> questions but I am surprised to learn that some kind of a
>> sockaddr_storage wrapper cannot work well for Squid... We may have to
>> fix Polygraph that is using that approach, IIRC.
>
> I don't have any online references. Everything I have found online
> indicated that the _storage should be padded properly.
> But, I found in my hex-level debugging of the IPv6 code in squid that
> some things consistently went badly because the cast sa_family field was
> something much higher than any IANA protocol family. Its too long ago
> now to recall exact test cases. May have been a system-specific given
> this connect() bug.

Correction: I did have online source that were somewhat confusing but
explained the behaviour a little. This seems to be saying what I
remember, http://www.kame.net/newsletter/19980604/

Amos

>
>>
>>> The third;
>>> addrinfo* defines a whole new type. Wrapping that old sockaddr*
>>> mess and
>>> providing in a nice set fo bells and whistles for use. Most importantly
>>> that flag we need to pas to the system calls.
>>
>> I have to say that the "nice set of bells and whistles" in a basic
>> address structure used throughout a performance-sensitive program raises
>> red flags, but perhaps the actual performance implications are not as
>> bad.
>>
>>> The important bits for the squid comm code are:
>>>
>>> struct addrinfo
>>> {
>>> int ai_family;
>>> int ai_socktype;
>>> int ai_protocol;
>>> struct sockaddr *ai_addr; // pointer to sockaddr_storage/*_in/*_in6
>>> int ai_addrlen;
>>> char *ai_canonname; // we never new/free this ourselves in squid.
>>> struct addrinfo *ai_next; // Pointer to next in list.
>>> };
>>
>> Can you replace IPAddress data members with the above, except not use
>> any pointers and forget about ai_next and ai_canonname? I think doing so
>> will eliminate temporary allocations and other things that look rather
>> scary to both code quality and performance folks.
>
> In the cases where addrinfo* is going into read-only usage yes.
>
>>
>> When you do need a list of addresses or canonname, it is OK to use
>> addrinfo and convert from/to IPAddress as needed, of course. I am
>> guessing such uses will not affect performance or overall code quality.
>>
>
> Amos

-- 
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.