Bug 16001 - calls to getaddrinfo() leak memory.
Summary: calls to getaddrinfo() leak memory.
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: network (show other bugs)
Version: 2.18
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-04 20:14 UTC by Debabrata Banerjee
Modified: 2014-10-31 08:12 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Leak test case. (604 bytes, text/x-csrc)
2013-10-04 20:14 UTC, Debabrata Banerjee
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Debabrata Banerjee 2013-10-04 20:14:03 UTC
Created attachment 7223 [details]
Leak test case.

Calls to getaddrinfo() followed by freeaddrinfo() leak memory. Test case attached. Sending fix patch to libc-alpha.
Comment 1 Andreas Schwab 2014-06-05 14:24:02 UTC
This is not how the addrinfo storage returned by getaddrinfo is supposed to be deallocated.
Comment 2 Debabrata Banerjee 2014-06-05 16:04:29 UTC
(In reply to Andreas Schwab from comment #1)
> This is not how the addrinfo storage returned by getaddrinfo is supposed to
> be deallocated.

What? This most certainly is a valid bug. My patch was superseded by better ideas about how to fix it. Whether or not it was resolved in tree yet is a good question.
Comment 3 Alexandre Oliva 2014-09-29 08:17:08 UTC
I see the testcase duplicates freeaddrinfo behavior, but I don't see how it shows there is (or was) any memory leak.  I couldn't locate the patch submission nor the better ideas about how to fix it to tell whether this should be reopened or left closed.  Pointers anyone?
Comment 4 Debabrata Banerjee 2014-10-13 03:11:55 UTC
(In reply to Alexandre Oliva from comment #3)
> I see the testcase duplicates freeaddrinfo behavior, but I don't see how it
> shows there is (or was) any memory leak.  I couldn't locate the patch
> submission nor the better ideas about how to fix it to tell whether this
> should be reopened or left closed.  Pointers anyone?

Hi, If you create a large number of local interfaces, then you should be able to reproduce it with the test case attached. i.e. Create 16,000 eth0:xxxxx aliases and assign a new IPv4 address to each one.
Comment 5 Alexandre Oliva 2014-10-31 07:13:56 UTC
If I were to do that, how would the testcase report the memory leak?  The bug report doesn't say that, and I don't see anything in the testcase that covers that either.

Could this possibly have been fixed by the patch for bug 16002?
Comment 6 Alexandre Oliva 2014-10-31 07:27:28 UTC
It looks like there is a proposed patch for this bug at https://www.sourceware.org/ml/libc-alpha/2013-10/msg00190.html with a bit of additional info on how to reproduce it.  It would have been nice to have had them cross-referenced.
Comment 7 Alexandre Oliva 2014-10-31 08:12:26 UTC
The current refcounting code is significantly different now than it was when you reported the patch.  I see usecnt is initialized to 2 (1 use for the cache, 1 use to be freed by the caller), and it's incremented when the cached list is reused, and decremented when the cache is replaced or when the list is freed.  I don't see how this could possibly leak in6ai lists.  Is this what used to leak before?  If so, can you confirm that the problem is fixed?

One problem I do see is that getaddrinfo, the only caller of check_pf, sorts a potentially shared in6ai list without any guards to prevent multiple threads from trying to do that concurrently and messing the array up.