Re: squid-3.HEAD IPAddress leak

> On Fri, Feb 08, 2008, Amos Jeffries wrote:
>
>> Try it. You will be horrified.
>
> Why? Whats being trashed, AI, or AI->somemember?

AI->somemember.

>
> If AI is being trashed, then just create a temporary pointer copy
> of AI, use that in the socket call, and then throw it away.
> if its trashing the memory -at- the AI rather than the AI pointer
> itself then you should probably spend some time with valgrind.
>
>> The pointer-tree needs by the OS to work with screws that up. We would
>> need to allocate a new addrinfo for each master node and set its
>> pointers
>> to new memory, copy the original data into it ... and hey! thats what
>> IPAddress::GetAddrInfo() does!
>
> It doesn't do that in C! I don't get it.
>
>> Do not confuse IPAddress::GetAddrInfo() which allocates an initializes a
>> 'full' one-IP addrinfo* tree quickly which is about to be used in place
>> of
>> a blocking getaddrinfo() system call.
>>
>> The second-best alternative in many of these places is hard-coding
>> sockaddr_in* types and using #if USE_IPV6 to alternate the calls.
>
>> With IPAddress::InitAddrInfo() which allocates a single empty addrinfo*
>> ready for new data to be inserted and only sets the flags up.
>
> Are there any situations where you iterate over a list of AddrInfo's ?

Yes, in all those F->somewhere = *AI calls. Robert pointed out.
In all results from get*() system calls.
In all results from xgetaddrinfo() system calls.

>
> You're not passing in a struct addr_info which you've allocated yourself
> into
> a glibc call which then frobs it and possibly allocates another?

I suspect thats whats happening. Or, errors changing it to some sockaddr*
thing representing a bad-IP state. Same diff to squid.
When it should NOT be altering the pointers, but writing to the memory
provided.

Look at this:

int connect(int sockfd, const struct sockaddr *serv_addr, socklen_t addrlen);

in squid ...

  AI = { ...
        ai_addr (0xfff...) <my pointer somewhere to sockaddr>
        ai_addrlen = 16
        ... };

   connect(fd, AI->ai_addr, AI->ai_addrlen);

afterwards:

  AI = { ...
        ai_addr (0x80...) <definately NOT my pointer>
        ai_addrlen = 16
        ... };

the rest of AI is untouched.

So its altering a CONST parameter. And worse. It's taking the pointer
by-value as if it was accepting by-ref and alocating its own.

Amos
Received on Thu Feb 07 2008 - 16:10:22 MST