Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\))
Subject: Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <530ec927-e9c6-490c-1382-8e5be1aea203@RedHat.com>
Date:   Mon, 18 Feb 2019 13:23:23 -0500
Cc:     Peter Wagner <tripolar@gmx.at>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: 7bit
Message-Id: <0B1E18B0-0034-4612-B63B-41155D2E0D64@oracle.com>
References: <20190217173901.33296254@onion.lan>
 <1081BBCD-8CA0-49CC-A02C-16F46FF613BF@oracle.com>
 <20190218001153.3d21b28c@onion.lan>
 <34D9E70A-7F29-4C6B-8A10-18F7A7EFFE2E@oracle.com>
 <137b8db8-a6d6-33a6-408b-73f8915eee08@RedHat.com>
 <EE3790BC-A5B4-4C34-9F21-7009579B3A25@oracle.com>
 <530ec927-e9c6-490c-1382-8e5be1aea203@RedHat.com>
To:     Steve Dickson <SteveD@RedHat.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk


> On Feb 18, 2019, at 12:59 PM, Steve Dickson <SteveD@RedHat.com> wrote:
> 
> 
> On 2/18/19 12:00 PM, Chuck Lever wrote:
>>> I also took a look at how freeaddrinfo(3) in glibc works:
>>> 
>>> void
>>> freeaddrinfo (struct addrinfo *ai)
>>> {
>>> struct addrinfo *p;
>>> 
>>> while (ai != NULL)
>>>   {
>>>     p = ai;
>>>     ai = ai->ai_next;
>>>     free (p->ai_canonname);
>>>     free (p);
>>>   }
>>> }
>>> It makes the assumption that ai_canonname is
>>> free-able memory....
>> Right, I seem to recall looking at this implementation when
>> I wrote the IPv6 support for nfs-utils. That seems sensible
>> to me.
>> 
>> However Peter's point is not all C libraries have to work
>> that way, and I can see that argument too.
> I agreed.... 
>> 
>> I have no objection to taking his fix as long as the hunk
>> in host_numeric_addrinfo() is dropped while we figure out
>> a more complete fix for that issue. 
> Taking it out I can see us getting flagged for possible memory leaking....

Exactly, with glibc C, that memory will leak if it's not freed
there.

We need a different solution here that works with all C libraries.
If Peter agrees, I'll volunteer to have a look at this.


> Taking a look to why those frees were add... see commit 94ce1eb9
> They should look very familiar to you ;-)  
> 
>> I would also like to
>> see his fix use a helper like nfs_freeaddrinfo(), but that
>> is only a suggestion.
> That is a good suggestion... It definitely cleans up the code.
> 
> steved. 
>> 
>> 

--
Chuck Lever