Skip Menu |
 

Download (untitled) / with headers
text/plain 4.5KiB
From krb5-bugs-incoming-bounces@PCH.mit.edu Tue Jul 21 19:53:29 2009
Return-Path: <krb5-bugs-incoming-bounces@PCH.mit.edu>
Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90])
by krbdev.mit.edu (Postfix) with ESMTP id 1888ACCA05;
Tue, 21 Jul 2009 19:53:29 +0000 (UTC)
Received: from pch.mit.edu (pch.mit.edu [127.0.0.1])
by pch.mit.edu (8.13.6/8.12.8) with ESMTP id n6LJrSw2011109;
Tue, 21 Jul 2009 15:53:29 -0400
Received: from fort-point-station.mit.edu (FORT-POINT-STATION.MIT.EDU
[18.7.7.76])
by pch.mit.edu (8.13.6/8.12.8) with ESMTP id n6LHcPVk006251
for <krb5-bugs-incoming@PCH.mit.edu>; Tue, 21 Jul 2009 13:38:25 -0400
Received: from mit.edu (W92-130-BARRACUDA-2.MIT.EDU [18.7.21.223])
by fort-point-station.mit.edu (8.13.6/8.9.2) with ESMTP id
n6LHcGVf008229
for <krb5-bugs@mit.edu>; Tue, 21 Jul 2009 13:38:16 -0400 (EDT)
Received: from mss-uk.mssgmbh.com (localhost [127.0.0.1])
by mit.edu (Spam Firewall) with ESMTP id A6BDE5F24B9
for <krb5-bugs@mit.edu>; Tue, 21 Jul 2009 13:38:11 -0400 (EDT)
Received: from mss-uk.mssgmbh.com (mss-uk.mssgmbh.com [217.174.251.109]) by
mit.edu with ESMTP id lQnq33eOzhW3cCby (version=TLSv1
cipher=AES256-SHA bits=256 verify=NO) for <krb5-bugs@mit.edu>;
Tue, 21 Jul 2009 13:38:11 -0400 (EDT)
Received: from fever.mssgmbh.com ([217.111.56.3]) (authenticated bits=0)
by mss-uk.mssgmbh.com (8.13.5.20060308/8.13.5/Debian-3ubuntu1) with
ESMTP id n6LHc8Cp016018
(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO)
for <krb5-bugs@mit.edu>; Tue, 21 Jul 2009 19:38:09 +0200
Received: from fever.mssgmbh.com (localhost [127.0.0.1])
by fever.mssgmbh.com (8.14.3/8.13.8/Debian-3) with ESMTP id
n6LHc3BS022956
for <krb5-bugs@mit.edu>; Tue, 21 Jul 2009 19:38:03 +0200
Received: (from rw@localhost)
by fever.mssgmbh.com (8.14.3/8.13.4/Submit) id n6LHc2df022953;
Tue, 21 Jul 2009 19:38:02 +0200
Date: Tue, 21 Jul 2009 19:38:02 +0200
Message-Id: <200907211738.n6LHc2df022953@fever.mssgmbh.com>
To: krb5-bugs@mit.edu
Subject:
From: rweikusat@mssgmbh.com
X-send-pr-version: 3.99
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0
(mss-uk.mssgmbh.com [217.174.251.109]);
Tue, 21 Jul 2009 19:38:10 +0200 (CEST)
X-Spam-Score: 4.137
X-Spam-Level: **** (4.137)
X-Spam-Flag: NO
X-Scanned-By: MIMEDefang 2.42
X-Mailman-Approved-At: Tue, 21 Jul 2009 15:53:26 -0400
X-BeenThere: krb5-bugs-incoming@mailman.mit.edu
X-Mailman-Version: 2.1.6
Precedence: list
Reply-To: rweikusat@mssgmbh.com
Sender: krb5-bugs-incoming-bounces@PCH.mit.edu
Errors-To: krb5-bugs-incoming-bounces@PCH.mit.edu


Show quoted text
>Submitter-Id: net
>Originator: Rainer Weikusat
>Organization:
MadPartners LTD
Show quoted text
>Confidential: no
>Synopsis: getaddrinfo in src/util/support/fake-addrinfo.c causes leak
>Severity: non-critical
>Priority: medium
>Category: krb5-libs
>Class: sw-bug
>Release: 1.7
>Environment:

System: Linux fever 2.6.30 #2 SMP Thu Jun 18 19:35:55 CEST 2009 i686 GNU/Linux


Show quoted text
>Description:
The function whose name was given in the synopsis contains a workaround
for a history problem with the glibc getaddrinfo:

Linux libc version 6 (libc-2.2.4.so on Debian) is broken.

[...]

The glibc 2.2.5 sources indicate that the canonical name is
*not* allocated separately, it's just some extra storage tacked
on the end of the addrinfo structure. So, let's try this
approach: If getaddrinfo sets ai_canonname, we'll replace the
*first* one with allocated storage,

This issue was fixed by Ulrich Drepper on 2004/08/17, cf

http://sourceware.org/git/?p=glibc.git;a=commit;f=ChangeLog;h=b93437642453ab93f7da79a15ca29cc66048e828

which implies that the ai_canonname-member has been allocated via strdup
at least since glibc release 2.3.4. The workaround mentionedc above overwrites
this allocated name with a newly allocated one, causing a memory leak.
Show quoted text
>How-To-Repeat:
use krb5_sname_to_princiapal
Show quoted text
>Fix:
A somewhat crude way to fix this issues is provided by the patch below
(the warning is there because I didn't find the patchlevel anywhere in
the glibc includes, but need this for 2.3.6)

diff -pru krb5-1.7.orig/src/util/support/fake-addrinfo.c krb5-1.7/src/util/support/fake-addrinfo.c
--- krb5-1.7.orig/src/util/support/fake-addrinfo.c 2008-12-01 18:09:59.000000000 +0100
+++ krb5-1.7/src/util/support/fake-addrinfo.c 2009-07-21 19:19:16.000000000 +0200
@@ -140,7 +140,13 @@ extern /*@dependent@*/ char *gai_strerro
#endif

#if defined (__linux__) && defined(HAVE_GETADDRINFO)
+#ifdef __GLIBC_MINOR__
+#if __GLIBC_MINOR__ < 3
# define COPY_FIRST_CANONNAME
+#elif __GLIBC_MINOR__ == 3
+#warning GLIBC 2.3 < 2.3.4 needs COPY_FIRST_CANONNAME
+#endif
+#endif
#endif

#ifdef _AIX
Thanks for the report and proposed fix.

I have an idea for fixing the problem more robustly. I don't have any
better ideas than yours on how to detect whether we need the workaround,
but when we do apply the workaround, we shouldn't need to make
assumptions about whether the system getaddrinfo() allocated
ai_canonname or not. What we should do instead is:

1. Store the system getaddrinfo result in a temporary.
2. Construct our own copy of the result, allocated with our own code,
containing the modified ai_canonname fields.
3. Free the system getaddrinfo result with the system freeaddrinfo.
4. When our freeaddrinfo wrapper is called, use our own code to free the
addrinfo structure, which we know was constructed with our own code.

I will likely apply your patch in addition so that we aren't doing
unnecessary extra work on modern Linux systems.
I started trying to implement my idea and decided it was too much work,
with too much potential for bugginess, for a piece of code which is only
going to work around old buggy glibc versions. So I'm just going to
take your patch and adjust the comments a bit. Thanks again.
To: rt-comment@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #6534] getaddrinfo in src/util/support/fake-addrinfo.c causes leak
From: Rainer Weikusat <rweikusat@mssgmbh.com>
Date: Fri, 28 Aug 2009 17:55:53 +0200
RT-Send-Cc:
"Greg Hudson via RT" <rt-comment@krbdev.mit.edu> writes:
Show quoted text
> I started trying to implement my idea and decided it was too much work,
> with too much potential for bugginess, for a piece of code which is only
> going to work around old buggy glibc versions. So I'm just going to
> take your patch and adjust the comments a bit. Thanks again.

I am not quite sure if replying to this is the polite or the impolite
action, but 'I am always happy if can do something more universally
useful than just adapting code written by others for proprietary
applications'.
From: ghudson@mit.edu
Subject: SVN Commit

Disable the COPY_FIRST_CANONNAME workaround on Linux glibc 2.4 and
later, since it leaks memory on fixed glibc versions. We will still
leak memory on glibc 2.3.4 through 2.3.6 (e.g. RHEL 4) but that's
harder to detect.


https://github.com/krb5/krb5/commit/85959def61b4117bf2bbd17f61173d10ef2e7811
Commit By: ghudson
Revision: 22643
Changed Files:
U trunk/src/util/support/fake-addrinfo.c
From: tlyu@mit.edu
Subject: SVN Commit

pull up r22643 from trunk

------------------------------------------------------------------------
r22643 | ghudson | 2009-08-28 12:00:54 -0400 (Fri, 28 Aug 2009) | 7 lines

ticket: 6534

Disable the COPY_FIRST_CANONNAME workaround on Linux glibc 2.4 and
later, since it leaks memory on fixed glibc versions. We will still
leak memory on glibc 2.3.4 through 2.3.6 (e.g. RHEL 4) but that's
harder to detect.

https://github.com/krb5/krb5/commit/98f0a65646833156bf86112ddf7ca02c0cd3447f
Commit By: tlyu
Revision: 23630
Changed Files:
U branches/krb5-1-7/src/util/support/fake-addrinfo.c
Download (untitled) / with headers
text/plain 1.3KiB
In hindsight, I believe this patch's description was incorrect and I
accepted it too uncritically. Accepting this patch did fix a memory
leak, but I believe also caused rdns=false to be broken on Linux from
1.7.1 to 1.10.2.

The bug report's description claims that COPY_FIRST_CANONNAME was
working around a historical glibc problem related to memory allocation,
fixed in 2004-08-17. Actually reading Ken's comment reveals that
COPY_FIRST_CANONNAME actually works around glibc's sometimes-use of PTR
lookups to set the canonname. The 2004-08-17 glibc change caused our
workaround to start leaking memory, but didn't fix the problem of using
PTR lookups.

We later worked around the same problem in a different way, by using an
invocation of getaddrinfo which doesn't result in PTR lookups. That
workaround was in #7124, after a failed attempt in #6922.

Part of my confusion at the time likely came from the fact that the bug
report mentioned in Ken's comment (http://bugs.debian.org/cgi-
bin/bugreport.cgi?bug=133668) was closed on 2004-12-19 because his test
case no longer revealed the problem. The test case called getaddrinfo
without AI_ADDRCONFIG or an address family, which I guess used PTR
records in the past but doesn't now. However, getaddrinfo with
AI_ADDRCONFIG or an address family still uses PTR records.
To: rt@krbdev.MIT.EDU
Subject: Re: [krbdev.mit.edu #6534] getaddrinfo in src/util/support/fake-addrinfo.c causes leak
From: Tom Yu <tlyu@MIT.EDU>
Date: Thu, 28 Feb 2013 12:26:23 -0500
RT-Send-Cc:
"Greg Hudson via RT" <rt-comment@krbdev.mit.edu> writes:

Show quoted text
> In hindsight, I believe this patch's description was incorrect and I
> accepted it too uncritically. Accepting this patch did fix a memory
> leak, but I believe also caused rdns=false to be broken on Linux from
> 1.7.1 to 1.10.2.

It seems that for some reason rdns=false works in Raring, but there
doesn't seem to be any krb5 code change that would account for it. I
haven't tested personally. See

https://bugs.launchpad.net/ubuntu/+source/krb5/+bug/571572