48960 - glibc 2.1.1 (but not 2.1.3) leaks in gethostbyname2{,_r}

Status: CLOSED WONTFIX

(NSPR :: NSPR, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

According to the Boehm GC leak detector, PR_GetIPNodeByName leaks a bunch of 44
byte and 28 byte blocks on Linux (on my somewhat upgraded RH 6.0 installation).

The 44 byte blocks are allocated at the following point:

__create_ib_request[/lib/libnsl.so.1 +0x0000A3A1]
nis_list[/lib/libnsl.so.1 +0x0000A71A]
_nss_nisplus_gethostbyname2_r[/lib/libnss_nisplus.so.2 +0x0000340E]
gethostbyname2_r[/lib/libc.so.6 +0x000BFFF9]
gethostbyname2[/lib/libc.so.6 +0x000BFAFE]
PR_GetIPNodeByName[./libnspr4.so +0x00020675]

The first and third 32-bit words of these structures look like pointers,
and the second and fourth are always 1 and 3 respectively.

The 28 byte blocks are allocated here:

readColdStartFile[/lib/libnsl.so.1 +0x00006D3E]
__nisfind_server[/lib/libnsl.so.1 +0x00009D5C]
nis_list[/lib/libnsl.so.1 +0x0000A7DB]
_nss_nisplus_gethostbyname2_r[/lib/libnss_nisplus.so.2 +0x0000340E]
gethostbyname2_r[/lib/libc.so.6 +0x000BFFF9]
gethostbyname2[/lib/libc.so.6 +0x000BFAFE]
PR_GetIPNodeByName[./libnspr4.so +0x00020675]

These 28 byte blocks are all null.


I believe this leak is caused by NSPR's failure to propertly free the hostent
struct returned from gethostbyname2 here (and not a leak in libc):

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/src/misc/prnetdb.c&rev=NSPRPUB_CLIENT_BRANCH&mark=452#442

I will attach the preproccessed source to the function, which shows that neither
the freehostent call nor the _pr_freehostent_t is included by the preprocessor.

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: preprocessed source to PR_GetIPNodeByName

Comment 2

[Wan-Teh Chang]

The gethostbyname2() function is specified in the
obsolete RFC 2133 (http://www.ietf.org/rfc/rfc2133.txt).
gethostbyname2() returns its result in a static buffer,
just like gethostbyname().  RFC 2133 says the following
about this:
   Note that gethostbyname() and gethostbyname2() are not thread-safe,
   since both return a pointer to a static hostent structure.  But
   several vendors have defined a thread-safe gethostbyname_r() function
   that requires four additional arguments.  We expect these vendors to
   also define a gethostbyname2_r() function.

The freehostent() function is specified in RFC 2553 (which
replaces RFC 2113).  freehostent() frees the hostent structure
returned by getipnodebyname() or getipnodebyaddr().  The following
is the relevant text from RFC 2553:
6.3 Freeing memory for getipnodebyname and getipnodebyaddr

   The hostent structure does not change from its existing definition.
   This structure, and the information pointed to by this structure, are
   dynamically allocated by getipnodebyname and getipnodebyaddr.  The
   following function frees this memory:

      #include <netdb.h>

      void freehostent(struct hostent *ptr);

Based on what RFC 2133 and RFC 2553 say, I believe
these blocks are leaked by the gethostbyname2()
function in libc on Linux.

I will resolve this bug as WONTFIX because it's better
to fix the memory leak at its source (libc) than in NSPR.
You might want to file a bug report against glibc.  If
you know of any workaround please let me know.  Thanks.

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

libc has a gethostbyname2_r.  (It's used by gethostbyname2, and that seems to be
the problem.)  How hard would it be to use that instead, as a workaround?

(It might also be useful to check if this is fixed in newer versions of libc. 
I've been planning to upgrade sometime...)

Comment 4

[Wan-Teh Chang]

NSPR 4.1 (the tip of mozilla/nsprpub) is using gethostbyname2_r().
You can give that a try.  You just need to modify mozilla/client.mk
and comment out the line that says
   NSPR_CO_TAG = NSPRPUB_CLIENT_BRANCH
You should remove mozilla/nsprpub before you do a check out;
otherwise you will continue to get NSPRPUB_CLIENT_BRANCH because
the cvs tag is sticky.

Do you know if this leak in gethostbyname2() is a one-time
leak (i.e., one leak per session) or a per-call leak (i.e.,
one leak per gethostbyname2() call)?  If it is a one-time
leak, it isn't serious because the leak won't increase as
time goes by. 

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

It looks like a per-call leak.  The number of leaks varies from 12 to 259 in the
per-session leak logs I have lying around.

Comment 6

[Wan-Teh Chang]

Hmm... that's not good.

In any case, please give NSPR 4.1 a try and see if
switching to gethostbyname2_r() helps.

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I'll try NSPR 4.1 sometime in the next week -- the next time I feel like doing a
full rebuild.

Comment 8

[Wan-Teh Chang]

Because NSPR 4.1 is backward compatible with NSPR 4.0.2
(what SeaMonkey is using), this does not require a full
rebuild.

I the following should work:
  cd to the parent directory of mozilla
  mv mozilla/nsprpub mozilla/nsprpub.save # save the original NSPR
  cvs co mozilla/nsprpub # check out NSPR, with no cvs tag
  # copy two *.mk files generated by Mozilla's configure script
  cp mozilla/nsprpub.save/config/my_*.mk mozilla/nsprpub/config
  cd mozilla/nsprpub
  cvs status Makefile.in # verify there is no "Sticky Tag"
  gmake # build it

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I still see the leaks with NSPR 4.1.  It is using gethostbyname2_r, and I'm
still seeing the leaks.  The preprocessed code is showing that localbuf and
tmpbuf are being used (i.e., _PR_HAVE_GETHOST_R is defined).

Comment 10

[Chris Waterson]

Wan-teh, do you have any suggestions as to how we can work around this bug?

Comment 11

[Wan-Teh Chang]

One possibility is to go to the dns-helper model and
fork helper processes to call dns lookup functions.
Then the memory leak in gethostbyname2_r() won't affect
the main mozilla process.

Another possibility, which we should do anyway, is to
report this bug to the glibc maintainer.  I don't know
how to do that though.  Maybe bugzilla.redhat.com?

Comment 12

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Upgrading from glibc 2.1.1 to 2.1.3 fixed the problem.

I'll go back to using the client branch of NSPR just to make sure...

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(2.1.1 is used in RedHat 6.0, 2.1.3 is used in RedHat 6.2)

Comment 14

[Wan-Teh Chang]

This is good news.  After you copy the original mozilla/nsprpub
back, remember to cd into it and do a gmake again so that the
original .so's are installed in mozilla/dist/{lib,bin}.  I'm
looking forward to your update.

Comment 15

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

This looks OK with NSPRPUB_CLIENT_BRANCH as well.

I'm marking this bug as CLOSED because it is not and never was a bug in any
product Bugzilla tracks.