[PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Peter Wagner]

Afer the update to musl 1.1.21 freeaddrinfo is broken in some places in
the nfs-utils code because glibc seems to ignore when freeaddrinfo is
called with a NULL pointer which seems to be not defined in the spec.

See: https://www.openwall.com/lists/musl/2019/02/03/4

The free in support/export/hostname.c is removed too

See: https://www.openwall.com/lists/musl/2019/02/17/2


From 43e27735553b4c1e75964f32b2f887e84398055f Mon Sep 17 00:00:00 2001
From: Peter Wagner <tripolar@gmx.at>
Date: Sun, 17 Feb 2019 17:32:08 +0100
Subject: [PATCH] fix addrinfo usage

Signed-off-by: Peter Wagner <tripolar@gmx.at>
---
 support/export/client.c   |  3 ++-
 support/export/hostname.c |  2 +-
 utils/exportfs/exportfs.c | 12 ++++++++----
 utils/mount/stropts.c     |  3 ++-
 utils/mountd/cache.c      |  6 ++++--
 utils/statd/hostname.c    |  6 ++++--
 6 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/support/export/client.c b/support/export/client.c
index baf59c8..750eb7d 100644
--- a/support/export/client.c
+++ b/support/export/client.c
@@ -309,7 +309,8 @@ client_lookup(char *hname, int canonical)
 		init_addrlist(clp, ai);
 
 out:
-	freeaddrinfo(ai);
+	if (ai)
+		freeaddrinfo(ai);
 	return clp;
 }
 
diff --git a/support/export/hostname.c b/support/export/hostname.c
index 5c4c824..710bf61 100644
--- a/support/export/hostname.c
+++ b/support/export/hostname.c
@@ -354,7 +354,7 @@ host_numeric_addrinfo(const struct sockaddr *sap)
 	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
 	 */
 	if (ai != NULL) {
-		free(ai->ai_canonname);		/* just in case */
+		//free(ai->ai_canonname);		/* just in case */
 		ai->ai_canonname = strdup(buf);
 		if (ai->ai_canonname == NULL) {
 			freeaddrinfo(ai);
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index cd3c979..2f8d59a 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -282,7 +282,8 @@ exportfs_parsed(char *hname, char *path, char *options, int verbose)
 	validate_export(exp);
 
 out:
-	freeaddrinfo(ai);
+	if (ai)
+		freeaddrinfo(ai);
 }
 
 static int exportfs_generic(char *arg, char *options, int verbose)
@@ -395,7 +396,8 @@ unexportfs_parsed(char *hname, char *path, int verbose)
 	if (!success)
 		xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname, path);
 
-	freeaddrinfo(ai);
+	if (ai)
+		freeaddrinfo(ai);
 }
 
 static int unexportfs_generic(char *arg, int verbose)
@@ -639,8 +641,10 @@ matchhostname(const char *hostname1, const char *hostname2)
 			}
 
 out:
-	freeaddrinfo(results1);
-	freeaddrinfo(results2);
+	if (results1)
+		freeaddrinfo(results1);
+	if (results2)
+		freeaddrinfo(results2);
 	return result;
 }
 
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 0a25b1f..8b7a0a8 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -1268,7 +1268,8 @@ int nfsmount_string(const char *spec, const char *node, char *type,
 	} else
 		nfs_error(_("%s: internal option parsing error"), progname);
 
-	freeaddrinfo(mi.address);
+	if (mi.address)
+		freeaddrinfo(mi.address);
 	free(mi.hostname);
 	return retval;
 }
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 7e8d403..8cee1c8 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -834,7 +834,8 @@ static void nfsd_fh(int f)
 out:
 	if (found_path)
 		free(found_path);
-	freeaddrinfo(ai);
+	if(ai)
+		freeaddrinfo(ai);
 	free(dom);
 	xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ? found->e_path : NULL);
 }
@@ -1355,7 +1356,8 @@ static void nfsd_export(int f)
 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
 	if (dom) free(dom);
 	if (path) free(path);
-	freeaddrinfo(ai);
+	if (ai)
+		freeaddrinfo(ai);
 }
 
 
diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
index 8cccdb8..6556ab1 100644
--- a/utils/statd/hostname.c
+++ b/utils/statd/hostname.c
@@ -308,8 +308,10 @@ statd_matchhostname(const char *hostname1, const char *hostname2)
 			}
 
 out:
-	freeaddrinfo(results2);
-	freeaddrinfo(results1);
+	if (results2)
+		freeaddrinfo(results2);
+	if (results1)
+		freeaddrinfo(results1);
 
 	xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
 			hostname1, hostname2,
-- 
2.20.1

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Chuck Lever]

Hi Peter-

> On Feb 17, 2019, at 11:39 AM, Peter Wagner <tripolar@gmx.at> wrote:
> 
> Afer the update to musl 1.1.21 freeaddrinfo is broken in some places in
> the nfs-utils code because glibc seems to ignore when freeaddrinfo is
> called with a NULL pointer which seems to be not defined in the spec.
> 
> See: https://www.openwall.com/lists/musl/2019/02/03/4

It might be cleaner to define a local version of freeaddrinfo in
nfs-utils to reduce duplication of code and provide a place
in the code itself to document this issue with a comment.


> The free in support/export/hostname.c is removed too
> 
> See: https://www.openwall.com/lists/musl/2019/02/17/2

Actually this seems like a separate issue because a distribution
that uses another C library might decide it is pertinent to apply
separately from the other changes here.

Please create a separate patch. It should remove the free(3)
call instead of commenting it out, and the patch description
should have its own copy of Rich’s email comments.

Thanks!


> From 43e27735553b4c1e75964f32b2f887e84398055f Mon Sep 17 00:00:00 2001
> From: Peter Wagner <tripolar@gmx.at>
> Date: Sun, 17 Feb 2019 17:32:08 +0100
> Subject: [PATCH] fix addrinfo usage
> 
> Signed-off-by: Peter Wagner <tripolar@gmx.at>
> ---
> support/export/client.c   |  3 ++-
> support/export/hostname.c |  2 +-
> utils/exportfs/exportfs.c | 12 ++++++++----
> utils/mount/stropts.c     |  3 ++-
> utils/mountd/cache.c      |  6 ++++--
> utils/statd/hostname.c    |  6 ++++--
> 6 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/support/export/client.c b/support/export/client.c
> index baf59c8..750eb7d 100644
> --- a/support/export/client.c
> +++ b/support/export/client.c
> @@ -309,7 +309,8 @@ client_lookup(char *hname, int canonical)
>       init_addrlist(clp, ai);
> 
> out:
> -    freeaddrinfo(ai);
> +    if (ai)
> +        freeaddrinfo(ai);
>   return clp;
> }
> 
> diff --git a/support/export/hostname.c b/support/export/hostname.c
> index 5c4c824..710bf61 100644
> --- a/support/export/hostname.c
> +++ b/support/export/hostname.c
> @@ -354,7 +354,7 @@ host_numeric_addrinfo(const struct sockaddr *sap)
>    * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
>    */
>   if (ai != NULL) {
> -        free(ai->ai_canonname);        /* just in case */
> +        //free(ai->ai_canonname);        /* just in case */
>       ai->ai_canonname = strdup(buf);
>       if (ai->ai_canonname == NULL) {
>           freeaddrinfo(ai);
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index cd3c979..2f8d59a 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -282,7 +282,8 @@ exportfs_parsed(char *hname, char *path, char *options, int verbose)
>   validate_export(exp);
> 
> out:
> -    freeaddrinfo(ai);
> +    if (ai)
> +        freeaddrinfo(ai);
> }
> 
> static int exportfs_generic(char *arg, char *options, int verbose)
> @@ -395,7 +396,8 @@ unexportfs_parsed(char *hname, char *path, int verbose)
>   if (!success)
>       xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname, path);
> 
> -    freeaddrinfo(ai);
> +    if (ai)
> +        freeaddrinfo(ai);
> }
> 
> static int unexportfs_generic(char *arg, int verbose)
> @@ -639,8 +641,10 @@ matchhostname(const char *hostname1, const char *hostname2)
>           }
> 
> out:
> -    freeaddrinfo(results1);
> -    freeaddrinfo(results2);
> +    if (results1)
> +        freeaddrinfo(results1);
> +    if (results2)
> +        freeaddrinfo(results2);
>   return result;
> }
> 
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 0a25b1f..8b7a0a8 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -1268,7 +1268,8 @@ int nfsmount_string(const char *spec, const char *node, char *type,
>   } else
>       nfs_error(_("%s: internal option parsing error"), progname);
> 
> -    freeaddrinfo(mi.address);
> +    if (mi.address)
> +        freeaddrinfo(mi.address);
>   free(mi.hostname);
>   return retval;
> }
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 7e8d403..8cee1c8 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -834,7 +834,8 @@ static void nfsd_fh(int f)
> out:
>   if (found_path)
>       free(found_path);
> -    freeaddrinfo(ai);
> +    if(ai)
> +        freeaddrinfo(ai);
>   free(dom);
>   xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ? found->e_path : NULL);
> }
> @@ -1355,7 +1356,8 @@ static void nfsd_export(int f)
>   xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
>   if (dom) free(dom);
>   if (path) free(path);
> -    freeaddrinfo(ai);
> +    if (ai)
> +        freeaddrinfo(ai);
> }
> 
> 
> diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
> index 8cccdb8..6556ab1 100644
> --- a/utils/statd/hostname.c
> +++ b/utils/statd/hostname.c
> @@ -308,8 +308,10 @@ statd_matchhostname(const char *hostname1, const char *hostname2)
>           }
> 
> out:
> -    freeaddrinfo(results2);
> -    freeaddrinfo(results1);
> +    if (results2)
> +        freeaddrinfo(results2);
> +    if (results1)
> +        freeaddrinfo(results1);
> 
>   xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
>           hostname1, hostname2,
> -- 
> 2.20.1
> 

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Peter Wagner]

Hi Chuck,

> It might be cleaner to define a local version of freeaddrinfo in
> nfs-utils to reduce duplication of code and provide a place
> in the code itself to document this issue with a comment.

But why use a local freeaddrinfo function? The code is checking at
multiple places if an addrinfo* == NULL and if it's NULL it just returns
without freeing it and at some other places it just frees it. For 
censistence reasions i would propose to check if it's != NULL before 
freeing it.

The spec also just defines the != NULL case and we shouldn't depend on
undefined behavior imho.

Regards,
Peter



On Sun, 17 Feb 2019 12:40:10 -0500
Chuck Lever <chuck.lever@oracle.com> wrote:

> Hi Peter-
> 
> > On Feb 17, 2019, at 11:39 AM, Peter Wagner <tripolar@gmx.at> wrote:
> > 
> > Afer the update to musl 1.1.21 freeaddrinfo is broken in some
> > places in the nfs-utils code because glibc seems to ignore when
> > freeaddrinfo is called with a NULL pointer which seems to be not
> > defined in the spec.
> > 
> > See: https://www.openwall.com/lists/musl/2019/02/03/4  
> 
> It might be cleaner to define a local version of freeaddrinfo in
> nfs-utils to reduce duplication of code and provide a place
> in the code itself to document this issue with a comment.
> 
> 
> > The free in support/export/hostname.c is removed too
> > 
> > See: https://www.openwall.com/lists/musl/2019/02/17/2  
> 
> Actually this seems like a separate issue because a distribution
> that uses another C library might decide it is pertinent to apply
> separately from the other changes here.
> 
> Please create a separate patch. It should remove the free(3)
> call instead of commenting it out, and the patch description
> should have its own copy of Rich’s email comments.
> 
> Thanks!
> 
> 
> > From 43e27735553b4c1e75964f32b2f887e84398055f Mon Sep 17 00:00:00
> > 2001 From: Peter Wagner <tripolar@gmx.at>
> > Date: Sun, 17 Feb 2019 17:32:08 +0100
> > Subject: [PATCH] fix addrinfo usage
> > 
> > Signed-off-by: Peter Wagner <tripolar@gmx.at>
> > ---
> > support/export/client.c   |  3 ++-
> > support/export/hostname.c |  2 +-
> > utils/exportfs/exportfs.c | 12 ++++++++----
> > utils/mount/stropts.c     |  3 ++-
> > utils/mountd/cache.c      |  6 ++++--
> > utils/statd/hostname.c    |  6 ++++--
> > 6 files changed, 21 insertions(+), 11 deletions(-)
> > 
> > diff --git a/support/export/client.c b/support/export/client.c
> > index baf59c8..750eb7d 100644
> > --- a/support/export/client.c
> > +++ b/support/export/client.c
> > @@ -309,7 +309,8 @@ client_lookup(char *hname, int canonical)
> >       init_addrlist(clp, ai);
> > 
> > out:
> > -    freeaddrinfo(ai);
> > +    if (ai)
> > +        freeaddrinfo(ai);
> >   return clp;
> > }
> > 
> > diff --git a/support/export/hostname.c b/support/export/hostname.c
> > index 5c4c824..710bf61 100644
> > --- a/support/export/hostname.c
> > +++ b/support/export/hostname.c
> > @@ -354,7 +354,7 @@ host_numeric_addrinfo(const struct sockaddr
> > *sap)
> >    * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
> >    */
> >   if (ai != NULL) {
> > -        free(ai->ai_canonname);        /* just in case */
> > +        //free(ai->ai_canonname);        /* just in case */
> >       ai->ai_canonname = strdup(buf);
> >       if (ai->ai_canonname == NULL) {
> >           freeaddrinfo(ai);
> > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> > index cd3c979..2f8d59a 100644
> > --- a/utils/exportfs/exportfs.c
> > +++ b/utils/exportfs/exportfs.c
> > @@ -282,7 +282,8 @@ exportfs_parsed(char *hname, char *path, char
> > *options, int verbose) validate_export(exp);
> > 
> > out:
> > -    freeaddrinfo(ai);
> > +    if (ai)
> > +        freeaddrinfo(ai);
> > }
> > 
> > static int exportfs_generic(char *arg, char *options, int verbose)
> > @@ -395,7 +396,8 @@ unexportfs_parsed(char *hname, char *path, int
> > verbose) if (!success)
> >       xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname,
> > path);
> > 
> > -    freeaddrinfo(ai);
> > +    if (ai)
> > +        freeaddrinfo(ai);
> > }
> > 
> > static int unexportfs_generic(char *arg, int verbose)
> > @@ -639,8 +641,10 @@ matchhostname(const char *hostname1, const
> > char *hostname2) }
> > 
> > out:
> > -    freeaddrinfo(results1);
> > -    freeaddrinfo(results2);
> > +    if (results1)
> > +        freeaddrinfo(results1);
> > +    if (results2)
> > +        freeaddrinfo(results2);
> >   return result;
> > }
> > 
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index 0a25b1f..8b7a0a8 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -1268,7 +1268,8 @@ int nfsmount_string(const char *spec, const
> > char *node, char *type, } else
> >       nfs_error(_("%s: internal option parsing error"), progname);
> > 
> > -    freeaddrinfo(mi.address);
> > +    if (mi.address)
> > +        freeaddrinfo(mi.address);
> >   free(mi.hostname);
> >   return retval;
> > }
> > diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> > index 7e8d403..8cee1c8 100644
> > --- a/utils/mountd/cache.c
> > +++ b/utils/mountd/cache.c
> > @@ -834,7 +834,8 @@ static void nfsd_fh(int f)
> > out:
> >   if (found_path)
> >       free(found_path);
> > -    freeaddrinfo(ai);
> > +    if(ai)
> > +        freeaddrinfo(ai);
> >   free(dom);
> >   xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ?
> > found->e_path : NULL); }
> > @@ -1355,7 +1356,8 @@ static void nfsd_export(int f)
> >   xlog(D_CALL, "nfsd_export: found %p path %s", found, path ?
> > path : NULL); if (dom) free(dom);
> >   if (path) free(path);
> > -    freeaddrinfo(ai);
> > +    if (ai)
> > +        freeaddrinfo(ai);
> > }
> > 
> > 
> > diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
> > index 8cccdb8..6556ab1 100644
> > --- a/utils/statd/hostname.c
> > +++ b/utils/statd/hostname.c
> > @@ -308,8 +308,10 @@ statd_matchhostname(const char *hostname1,
> > const char *hostname2) }
> > 
> > out:
> > -    freeaddrinfo(results2);
> > -    freeaddrinfo(results1);
> > +    if (results2)
> > +        freeaddrinfo(results2);
> > +    if (results1)
> > +        freeaddrinfo(results1);
> > 
> >   xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
> >           hostname1, hostname2,
> > -- 
> > 2.20.1
> >   
> 

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Chuck Lever]

> On Feb 17, 2019, at 6:11 PM, Peter Wagner <tripolar@gmx.at> wrote:
> 
> Hi Chuck,
> 
>> It might be cleaner to define a local version of freeaddrinfo in
>> nfs-utils to reduce duplication of code and provide a place
>> in the code itself to document this issue with a comment.
> 
> But why use a local freeaddrinfo function? The code is checking at
> multiple places if an addrinfo* == NULL and if it's NULL it just returns
> without freeing it and at some other places it just frees it. For 
> censistence reasions i would propose to check if it's != NULL before 
> freeing it.

I didn't think this would be a controversial suggestion.
Here is what I meant:

Find a common location, say support/include/exportfs.h, and
add this:

/*
 * Some versions of freeaddrinfo(3) do not tolerate being
 * passed a NULL pointer.
 */
static inline void nfs_freeaddrinfo(struct addrinfo *ai)
{
	if (ai)
		freeaddrinfo(ai);
}

Then replace the freeaddrinfo() call sites within nfs-utils
with calls to nfs_freeaddrinfo().



The logic in host_numeric_addrinfo() will need a closer
look. If what Rich says is true then both freeing the
ai_canonname string _and_ replacing it with strdup(3)
will be problematic.

I see that host_reliable_addrinfo() has the same issue.


> The spec also just defines the != NULL case and we shouldn't depend on
> undefined behavior imho.

Some counterpoints:

- It is good defensive coding that a library function should
perform basic sanity checks on its argument(s) to prevent
crashes.

- It's always possible for a spec to be wrong.

- If a spec says the behavior is "undefined" that generally
means the spec writer is giving the implementer permission
to choose a convenient and possibly useful behavior. Crashing
is not especially useful. A better approach would be to ignore
NULL, and add hooks to a tool like valgrind to report this
kind of issue.

- nfs-utils is not careless about this. The design of the code
clearly assumes that freeaddrinfo(3) will not crash in this
case. It's not an unreasonable assumption. The goal here is
to construct simple-to-read code.

- Strictly speaking, d1395c43 introduced a regression in
freeaddrinfo(3) that should be fixed, because existing
applications have depended on the old behavior for a long
while. IMO a better fix here would be to make musl's
freeaddrinfo(3) deal properly with a NULL argument instead
of changing all of its callers, but that's not my call.

- The equivalent to free(3) in the Linux kernel, kfree(),
explicitly allows callers to pass NULL because that eliminates
the need to add "if (arg != NULL)" at every call site.

I'm not arguing that we should do nothing, but rather that
there is a matter of subjectivity and taste here about what
is correct behavior. The OG spec is just one opinion.


> Regards,
> Peter
> 
> 
> 
> On Sun, 17 Feb 2019 12:40:10 -0500
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> Hi Peter-
>> 
>>> On Feb 17, 2019, at 11:39 AM, Peter Wagner <tripolar@gmx.at> wrote:
>>> 
>>> Afer the update to musl 1.1.21 freeaddrinfo is broken in some
>>> places in the nfs-utils code because glibc seems to ignore when
>>> freeaddrinfo is called with a NULL pointer which seems to be not
>>> defined in the spec.
>>> 
>>> See: https://www.openwall.com/lists/musl/2019/02/03/4  
>> 
>> It might be cleaner to define a local version of freeaddrinfo in
>> nfs-utils to reduce duplication of code and provide a place
>> in the code itself to document this issue with a comment.
>> 
>> 
>>> The free in support/export/hostname.c is removed too
>>> 
>>> See: https://www.openwall.com/lists/musl/2019/02/17/2  
>> 
>> Actually this seems like a separate issue because a distribution
>> that uses another C library might decide it is pertinent to apply
>> separately from the other changes here.
>> 
>> Please create a separate patch. It should remove the free(3)
>> call instead of commenting it out, and the patch description
>> should have its own copy of Rich’s email comments.
>> 
>> Thanks!
>> 
>> 
>>> From 43e27735553b4c1e75964f32b2f887e84398055f Mon Sep 17 00:00:00
>>> 2001 From: Peter Wagner <tripolar@gmx.at>
>>> Date: Sun, 17 Feb 2019 17:32:08 +0100
>>> Subject: [PATCH] fix addrinfo usage
>>> 
>>> Signed-off-by: Peter Wagner <tripolar@gmx.at>
>>> ---
>>> support/export/client.c   |  3 ++-
>>> support/export/hostname.c |  2 +-
>>> utils/exportfs/exportfs.c | 12 ++++++++----
>>> utils/mount/stropts.c     |  3 ++-
>>> utils/mountd/cache.c      |  6 ++++--
>>> utils/statd/hostname.c    |  6 ++++--
>>> 6 files changed, 21 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/support/export/client.c b/support/export/client.c
>>> index baf59c8..750eb7d 100644
>>> --- a/support/export/client.c
>>> +++ b/support/export/client.c
>>> @@ -309,7 +309,8 @@ client_lookup(char *hname, int canonical)
>>>      init_addrlist(clp, ai);
>>> 
>>> out:
>>> -    freeaddrinfo(ai);
>>> +    if (ai)
>>> +        freeaddrinfo(ai);
>>>  return clp;
>>> }
>>> 
>>> diff --git a/support/export/hostname.c b/support/export/hostname.c
>>> index 5c4c824..710bf61 100644
>>> --- a/support/export/hostname.c
>>> +++ b/support/export/hostname.c
>>> @@ -354,7 +354,7 @@ host_numeric_addrinfo(const struct sockaddr
>>> *sap)
>>>   * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
>>>   */
>>>  if (ai != NULL) {
>>> -        free(ai->ai_canonname);        /* just in case */
>>> +        //free(ai->ai_canonname);        /* just in case */
>>>      ai->ai_canonname = strdup(buf);
>>>      if (ai->ai_canonname == NULL) {
>>>          freeaddrinfo(ai);
>>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>>> index cd3c979..2f8d59a 100644
>>> --- a/utils/exportfs/exportfs.c
>>> +++ b/utils/exportfs/exportfs.c
>>> @@ -282,7 +282,8 @@ exportfs_parsed(char *hname, char *path, char
>>> *options, int verbose) validate_export(exp);
>>> 
>>> out:
>>> -    freeaddrinfo(ai);
>>> +    if (ai)
>>> +        freeaddrinfo(ai);
>>> }
>>> 
>>> static int exportfs_generic(char *arg, char *options, int verbose)
>>> @@ -395,7 +396,8 @@ unexportfs_parsed(char *hname, char *path, int
>>> verbose) if (!success)
>>>      xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname,
>>> path);
>>> 
>>> -    freeaddrinfo(ai);
>>> +    if (ai)
>>> +        freeaddrinfo(ai);
>>> }
>>> 
>>> static int unexportfs_generic(char *arg, int verbose)
>>> @@ -639,8 +641,10 @@ matchhostname(const char *hostname1, const
>>> char *hostname2) }
>>> 
>>> out:
>>> -    freeaddrinfo(results1);
>>> -    freeaddrinfo(results2);
>>> +    if (results1)
>>> +        freeaddrinfo(results1);
>>> +    if (results2)
>>> +        freeaddrinfo(results2);
>>>  return result;
>>> }
>>> 
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index 0a25b1f..8b7a0a8 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -1268,7 +1268,8 @@ int nfsmount_string(const char *spec, const
>>> char *node, char *type, } else
>>>      nfs_error(_("%s: internal option parsing error"), progname);
>>> 
>>> -    freeaddrinfo(mi.address);
>>> +    if (mi.address)
>>> +        freeaddrinfo(mi.address);
>>>  free(mi.hostname);
>>>  return retval;
>>> }
>>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>>> index 7e8d403..8cee1c8 100644
>>> --- a/utils/mountd/cache.c
>>> +++ b/utils/mountd/cache.c
>>> @@ -834,7 +834,8 @@ static void nfsd_fh(int f)
>>> out:
>>>  if (found_path)
>>>      free(found_path);
>>> -    freeaddrinfo(ai);
>>> +    if(ai)
>>> +        freeaddrinfo(ai);
>>>  free(dom);
>>>  xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ?
>>> found->e_path : NULL); }
>>> @@ -1355,7 +1356,8 @@ static void nfsd_export(int f)
>>>  xlog(D_CALL, "nfsd_export: found %p path %s", found, path ?
>>> path : NULL); if (dom) free(dom);
>>>  if (path) free(path);
>>> -    freeaddrinfo(ai);
>>> +    if (ai)
>>> +        freeaddrinfo(ai);
>>> }
>>> 
>>> 
>>> diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
>>> index 8cccdb8..6556ab1 100644
>>> --- a/utils/statd/hostname.c
>>> +++ b/utils/statd/hostname.c
>>> @@ -308,8 +308,10 @@ statd_matchhostname(const char *hostname1,
>>> const char *hostname2) }
>>> 
>>> out:
>>> -    freeaddrinfo(results2);
>>> -    freeaddrinfo(results1);
>>> +    if (results2)
>>> +        freeaddrinfo(results2);
>>> +    if (results1)
>>> +        freeaddrinfo(results1);
>>> 
>>>  xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
>>>          hostname1, hostname2,
>>> -- 
>>> 2.20.1
>>> 
>> 
> 

--
Chuck Lever

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Steve Dickson]

On 2/17/19 9:21 PM, Chuck Lever wrote:
> 
> 
>> On Feb 17, 2019, at 6:11 PM, Peter Wagner <tripolar@gmx.at> wrote:
>>
>> Hi Chuck,
>>
>>> It might be cleaner to define a local version of freeaddrinfo in
>>> nfs-utils to reduce duplication of code and provide a place
>>> in the code itself to document this issue with a comment.
>>
>> But why use a local freeaddrinfo function? The code is checking at
>> multiple places if an addrinfo* == NULL and if it's NULL it just returns
>> without freeing it and at some other places it just frees it. For 
>> censistence reasions i would propose to check if it's != NULL before 
>> freeing it.
> 
> I didn't think this would be a controversial suggestion.
> Here is what I meant:
> 
> Find a common location, say support/include/exportfs.h, and
> add this:
> 
> /*
>  * Some versions of freeaddrinfo(3) do not tolerate being
>  * passed a NULL pointer.
>  */
> static inline void nfs_freeaddrinfo(struct addrinfo *ai)
> {
> 	if (ai)
> 		freeaddrinfo(ai);
> }
> 
> Then replace the freeaddrinfo() call sites within nfs-utils
> with calls to nfs_freeaddrinfo().
+1

> 
> 
> 
> The logic in host_numeric_addrinfo() will need a closer
> look. If what Rich says is true then both freeing the
> ai_canonname string _and_ replacing it with strdup(3)
> will be problematic.
> 
> I see that host_reliable_addrinfo() has the same issue.
I just ran Rich's program that is suppose to
crash... It does not crash on Fedora 29.

> 
> 
>> The spec also just defines the != NULL case and we shouldn't depend on
>> undefined behavior imho.
> 
> Some counterpoints:
> 
> - It is good defensive coding that a library function should
> perform basic sanity checks on its argument(s) to prevent
> crashes.
> 
> - It's always possible for a spec to be wrong.
> 
> - If a spec says the behavior is "undefined" that generally
> means the spec writer is giving the implementer permission
> to choose a convenient and possibly useful behavior. Crashing
> is not especially useful. A better approach would be to ignore
> NULL, and add hooks to a tool like valgrind to report this
> kind of issue.
> 
> - nfs-utils is not careless about this. The design of the code
> clearly assumes that freeaddrinfo(3) will not crash in this
> case. It's not an unreasonable assumption. The goal here is
> to construct simple-to-read code.
> 
> - Strictly speaking, d1395c43 introduced a regression in
> freeaddrinfo(3) that should be fixed, because existing
> applications have depended on the old behavior for a long
> while. IMO a better fix here would be to make musl's
> freeaddrinfo(3) deal properly with a NULL argument instead
> of changing all of its callers, but that's not my call.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.... 

steved.
> 
> - The equivalent to free(3) in the Linux kernel, kfree(),
> explicitly allows callers to pass NULL because that eliminates
> the need to add "if (arg != NULL)" at every call site.
> 
> I'm not arguing that we should do nothing, but rather that
> there is a matter of subjectivity and taste here about what
> is correct behavior. The OG spec is just one opinion.
> 
> 
>> Regards,
>> Peter
>>
>>
>>
>> On Sun, 17 Feb 2019 12:40:10 -0500
>> Chuck Lever <chuck.lever@oracle.com> wrote:
>>
>>> Hi Peter-
>>>
>>>> On Feb 17, 2019, at 11:39 AM, Peter Wagner <tripolar@gmx.at> wrote:
>>>>
>>>> Afer the update to musl 1.1.21 freeaddrinfo is broken in some
>>>> places in the nfs-utils code because glibc seems to ignore when
>>>> freeaddrinfo is called with a NULL pointer which seems to be not
>>>> defined in the spec.
>>>>
>>>> See: https://www.openwall.com/lists/musl/2019/02/03/4  
>>>
>>> It might be cleaner to define a local version of freeaddrinfo in
>>> nfs-utils to reduce duplication of code and provide a place
>>> in the code itself to document this issue with a comment.
>>>
>>>
>>>> The free in support/export/hostname.c is removed too
>>>>
>>>> See: https://www.openwall.com/lists/musl/2019/02/17/2  
>>>
>>> Actually this seems like a separate issue because a distribution
>>> that uses another C library might decide it is pertinent to apply
>>> separately from the other changes here.
>>>
>>> Please create a separate patch. It should remove the free(3)
>>> call instead of commenting it out, and the patch description
>>> should have its own copy of Rich’s email comments.
>>>
>>> Thanks!
>>>
>>>
>>>> From 43e27735553b4c1e75964f32b2f887e84398055f Mon Sep 17 00:00:00
>>>> 2001 From: Peter Wagner <tripolar@gmx.at>
>>>> Date: Sun, 17 Feb 2019 17:32:08 +0100
>>>> Subject: [PATCH] fix addrinfo usage
>>>>
>>>> Signed-off-by: Peter Wagner <tripolar@gmx.at>
>>>> ---
>>>> support/export/client.c   |  3 ++-
>>>> support/export/hostname.c |  2 +-
>>>> utils/exportfs/exportfs.c | 12 ++++++++----
>>>> utils/mount/stropts.c     |  3 ++-
>>>> utils/mountd/cache.c      |  6 ++++--
>>>> utils/statd/hostname.c    |  6 ++++--
>>>> 6 files changed, 21 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/support/export/client.c b/support/export/client.c
>>>> index baf59c8..750eb7d 100644
>>>> --- a/support/export/client.c
>>>> +++ b/support/export/client.c
>>>> @@ -309,7 +309,8 @@ client_lookup(char *hname, int canonical)
>>>>      init_addrlist(clp, ai);
>>>>
>>>> out:
>>>> -    freeaddrinfo(ai);
>>>> +    if (ai)
>>>> +        freeaddrinfo(ai);
>>>>  return clp;
>>>> }
>>>>
>>>> diff --git a/support/export/hostname.c b/support/export/hostname.c
>>>> index 5c4c824..710bf61 100644
>>>> --- a/support/export/hostname.c
>>>> +++ b/support/export/hostname.c
>>>> @@ -354,7 +354,7 @@ host_numeric_addrinfo(const struct sockaddr
>>>> *sap)
>>>>   * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
>>>>   */
>>>>  if (ai != NULL) {
>>>> -        free(ai->ai_canonname);        /* just in case */
>>>> +        //free(ai->ai_canonname);        /* just in case */
>>>>      ai->ai_canonname = strdup(buf);
>>>>      if (ai->ai_canonname == NULL) {
>>>>          freeaddrinfo(ai);
>>>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>>>> index cd3c979..2f8d59a 100644
>>>> --- a/utils/exportfs/exportfs.c
>>>> +++ b/utils/exportfs/exportfs.c
>>>> @@ -282,7 +282,8 @@ exportfs_parsed(char *hname, char *path, char
>>>> *options, int verbose) validate_export(exp);
>>>>
>>>> out:
>>>> -    freeaddrinfo(ai);
>>>> +    if (ai)
>>>> +        freeaddrinfo(ai);
>>>> }
>>>>
>>>> static int exportfs_generic(char *arg, char *options, int verbose)
>>>> @@ -395,7 +396,8 @@ unexportfs_parsed(char *hname, char *path, int
>>>> verbose) if (!success)
>>>>      xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname,
>>>> path);
>>>>
>>>> -    freeaddrinfo(ai);
>>>> +    if (ai)
>>>> +        freeaddrinfo(ai);
>>>> }
>>>>
>>>> static int unexportfs_generic(char *arg, int verbose)
>>>> @@ -639,8 +641,10 @@ matchhostname(const char *hostname1, const
>>>> char *hostname2) }
>>>>
>>>> out:
>>>> -    freeaddrinfo(results1);
>>>> -    freeaddrinfo(results2);
>>>> +    if (results1)
>>>> +        freeaddrinfo(results1);
>>>> +    if (results2)
>>>> +        freeaddrinfo(results2);
>>>>  return result;
>>>> }
>>>>
>>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>>> index 0a25b1f..8b7a0a8 100644
>>>> --- a/utils/mount/stropts.c
>>>> +++ b/utils/mount/stropts.c
>>>> @@ -1268,7 +1268,8 @@ int nfsmount_string(const char *spec, const
>>>> char *node, char *type, } else
>>>>      nfs_error(_("%s: internal option parsing error"), progname);
>>>>
>>>> -    freeaddrinfo(mi.address);
>>>> +    if (mi.address)
>>>> +        freeaddrinfo(mi.address);
>>>>  free(mi.hostname);
>>>>  return retval;
>>>> }
>>>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>>>> index 7e8d403..8cee1c8 100644
>>>> --- a/utils/mountd/cache.c
>>>> +++ b/utils/mountd/cache.c
>>>> @@ -834,7 +834,8 @@ static void nfsd_fh(int f)
>>>> out:
>>>>  if (found_path)
>>>>      free(found_path);
>>>> -    freeaddrinfo(ai);
>>>> +    if(ai)
>>>> +        freeaddrinfo(ai);
>>>>  free(dom);
>>>>  xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ?
>>>> found->e_path : NULL); }
>>>> @@ -1355,7 +1356,8 @@ static void nfsd_export(int f)
>>>>  xlog(D_CALL, "nfsd_export: found %p path %s", found, path ?
>>>> path : NULL); if (dom) free(dom);
>>>>  if (path) free(path);
>>>> -    freeaddrinfo(ai);
>>>> +    if (ai)
>>>> +        freeaddrinfo(ai);
>>>> }
>>>>
>>>>
>>>> diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
>>>> index 8cccdb8..6556ab1 100644
>>>> --- a/utils/statd/hostname.c
>>>> +++ b/utils/statd/hostname.c
>>>> @@ -308,8 +308,10 @@ statd_matchhostname(const char *hostname1,
>>>> const char *hostname2) }
>>>>
>>>> out:
>>>> -    freeaddrinfo(results2);
>>>> -    freeaddrinfo(results1);
>>>> +    if (results2)
>>>> +        freeaddrinfo(results2);
>>>> +    if (results1)
>>>> +        freeaddrinfo(results1);
>>>>
>>>>  xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
>>>>          hostname1, hostname2,
>>>> -- 
>>>> 2.20.1
>>>>
>>>
>>
> 
> --
> Chuck Lever
> 
> 
> 

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Chuck Lever]

> On Feb 18, 2019, at 11:34 AM, Steve Dickson <SteveD@RedHat.com> wrote:
> 
> 
> 
> On 2/17/19 9:21 PM, Chuck Lever wrote:
>> 
>> 
>>> On Feb 17, 2019, at 6:11 PM, Peter Wagner <tripolar@gmx.at> wrote:
>>> 
>>> Hi Chuck,
>>> 
>>>> It might be cleaner to define a local version of freeaddrinfo in
>>>> nfs-utils to reduce duplication of code and provide a place
>>>> in the code itself to document this issue with a comment.
>>> 
>>> But why use a local freeaddrinfo function? The code is checking at
>>> multiple places if an addrinfo* == NULL and if it's NULL it just returns
>>> without freeing it and at some other places it just frees it. For 
>>> censistence reasions i would propose to check if it's != NULL before 
>>> freeing it.
>> 
>> I didn't think this would be a controversial suggestion.
>> Here is what I meant:
>> 
>> Find a common location, say support/include/exportfs.h, and
>> add this:
>> 
>> /*
>> * Some versions of freeaddrinfo(3) do not tolerate being
>> * passed a NULL pointer.
>> */
>> static inline void nfs_freeaddrinfo(struct addrinfo *ai)
>> {
>> 	if (ai)
>> 		freeaddrinfo(ai);
>> }
>> 
>> Then replace the freeaddrinfo() call sites within nfs-utils
>> with calls to nfs_freeaddrinfo().
> +1
> 
>> 
>> 
>> 
>> The logic in host_numeric_addrinfo() will need a closer
>> look. If what Rich says is true then both freeing the
>> ai_canonname string _and_ replacing it with strdup(3)
>> will be problematic.
>> 
>> I see that host_reliable_addrinfo() has the same issue.
> I just ran Rich's program that is suppose to
> crash... It does not crash on Fedora 29.

It probably does crash with musl 1.1.21. :-)


>>> The spec also just defines the != NULL case and we shouldn't depend on
>>> undefined behavior imho.
>> 
>> Some counterpoints:
>> 
>> - It is good defensive coding that a library function should
>> perform basic sanity checks on its argument(s) to prevent
>> crashes.
>> 
>> - It's always possible for a spec to be wrong.
>> 
>> - If a spec says the behavior is "undefined" that generally
>> means the spec writer is giving the implementer permission
>> to choose a convenient and possibly useful behavior. Crashing
>> is not especially useful. A better approach would be to ignore
>> NULL, and add hooks to a tool like valgrind to report this
>> kind of issue.
>> 
>> - nfs-utils is not careless about this. The design of the code
>> clearly assumes that freeaddrinfo(3) will not crash in this
>> case. It's not an unreasonable assumption. The goal here is
>> to construct simple-to-read code.
>> 
>> - Strictly speaking, d1395c43 introduced a regression in
>> freeaddrinfo(3) that should be fixed, because existing
>> applications have depended on the old behavior for a long
>> while. IMO a better fix here would be to make musl's
>> freeaddrinfo(3) deal properly with a NULL argument instead
>> of changing all of its callers, but that's not my call.

> 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 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. I would also like to
see his fix use a helper like nfs_freeaddrinfo(), but that
is only a suggestion.


> steved.
>> 
>> - The equivalent to free(3) in the Linux kernel, kfree(),
>> explicitly allows callers to pass NULL because that eliminates
>> the need to add "if (arg != NULL)" at every call site.
>> 
>> I'm not arguing that we should do nothing, but rather that
>> there is a matter of subjectivity and taste here about what
>> is correct behavior. The OG spec is just one opinion.
>> 
>> 
>>> Regards,
>>> Peter
>>> 
>>> 
>>> 
>>> On Sun, 17 Feb 2019 12:40:10 -0500
>>> Chuck Lever <chuck.lever@oracle.com> wrote:
>>> 
>>>> Hi Peter-
>>>> 
>>>>> On Feb 17, 2019, at 11:39 AM, Peter Wagner <tripolar@gmx.at> wrote:
>>>>> 
>>>>> Afer the update to musl 1.1.21 freeaddrinfo is broken in some
>>>>> places in the nfs-utils code because glibc seems to ignore when
>>>>> freeaddrinfo is called with a NULL pointer which seems to be not
>>>>> defined in the spec.
>>>>> 
>>>>> See: https://www.openwall.com/lists/musl/2019/02/03/4  
>>>> 
>>>> It might be cleaner to define a local version of freeaddrinfo in
>>>> nfs-utils to reduce duplication of code and provide a place
>>>> in the code itself to document this issue with a comment.
>>>> 
>>>> 
>>>>> The free in support/export/hostname.c is removed too
>>>>> 
>>>>> See: https://www.openwall.com/lists/musl/2019/02/17/2  
>>>> 
>>>> Actually this seems like a separate issue because a distribution
>>>> that uses another C library might decide it is pertinent to apply
>>>> separately from the other changes here.
>>>> 
>>>> Please create a separate patch. It should remove the free(3)
>>>> call instead of commenting it out, and the patch description
>>>> should have its own copy of Rich’s email comments.
>>>> 
>>>> Thanks!
>>>> 
>>>> 
>>>>> From 43e27735553b4c1e75964f32b2f887e84398055f Mon Sep 17 00:00:00
>>>>> 2001 From: Peter Wagner <tripolar@gmx.at>
>>>>> Date: Sun, 17 Feb 2019 17:32:08 +0100
>>>>> Subject: [PATCH] fix addrinfo usage
>>>>> 
>>>>> Signed-off-by: Peter Wagner <tripolar@gmx.at>
>>>>> ---
>>>>> support/export/client.c   |  3 ++-
>>>>> support/export/hostname.c |  2 +-
>>>>> utils/exportfs/exportfs.c | 12 ++++++++----
>>>>> utils/mount/stropts.c     |  3 ++-
>>>>> utils/mountd/cache.c      |  6 ++++--
>>>>> utils/statd/hostname.c    |  6 ++++--
>>>>> 6 files changed, 21 insertions(+), 11 deletions(-)
>>>>> 
>>>>> diff --git a/support/export/client.c b/support/export/client.c
>>>>> index baf59c8..750eb7d 100644
>>>>> --- a/support/export/client.c
>>>>> +++ b/support/export/client.c
>>>>> @@ -309,7 +309,8 @@ client_lookup(char *hname, int canonical)
>>>>>     init_addrlist(clp, ai);
>>>>> 
>>>>> out:
>>>>> -    freeaddrinfo(ai);
>>>>> +    if (ai)
>>>>> +        freeaddrinfo(ai);
>>>>> return clp;
>>>>> }
>>>>> 
>>>>> diff --git a/support/export/hostname.c b/support/export/hostname.c
>>>>> index 5c4c824..710bf61 100644
>>>>> --- a/support/export/hostname.c
>>>>> +++ b/support/export/hostname.c
>>>>> @@ -354,7 +354,7 @@ host_numeric_addrinfo(const struct sockaddr
>>>>> *sap)
>>>>>  * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
>>>>>  */
>>>>> if (ai != NULL) {
>>>>> -        free(ai->ai_canonname);        /* just in case */
>>>>> +        //free(ai->ai_canonname);        /* just in case */
>>>>>     ai->ai_canonname = strdup(buf);
>>>>>     if (ai->ai_canonname == NULL) {
>>>>>         freeaddrinfo(ai);
>>>>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>>>>> index cd3c979..2f8d59a 100644
>>>>> --- a/utils/exportfs/exportfs.c
>>>>> +++ b/utils/exportfs/exportfs.c
>>>>> @@ -282,7 +282,8 @@ exportfs_parsed(char *hname, char *path, char
>>>>> *options, int verbose) validate_export(exp);
>>>>> 
>>>>> out:
>>>>> -    freeaddrinfo(ai);
>>>>> +    if (ai)
>>>>> +        freeaddrinfo(ai);
>>>>> }
>>>>> 
>>>>> static int exportfs_generic(char *arg, char *options, int verbose)
>>>>> @@ -395,7 +396,8 @@ unexportfs_parsed(char *hname, char *path, int
>>>>> verbose) if (!success)
>>>>>     xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname,
>>>>> path);
>>>>> 
>>>>> -    freeaddrinfo(ai);
>>>>> +    if (ai)
>>>>> +        freeaddrinfo(ai);
>>>>> }
>>>>> 
>>>>> static int unexportfs_generic(char *arg, int verbose)
>>>>> @@ -639,8 +641,10 @@ matchhostname(const char *hostname1, const
>>>>> char *hostname2) }
>>>>> 
>>>>> out:
>>>>> -    freeaddrinfo(results1);
>>>>> -    freeaddrinfo(results2);
>>>>> +    if (results1)
>>>>> +        freeaddrinfo(results1);
>>>>> +    if (results2)
>>>>> +        freeaddrinfo(results2);
>>>>> return result;
>>>>> }
>>>>> 
>>>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>>>> index 0a25b1f..8b7a0a8 100644
>>>>> --- a/utils/mount/stropts.c
>>>>> +++ b/utils/mount/stropts.c
>>>>> @@ -1268,7 +1268,8 @@ int nfsmount_string(const char *spec, const
>>>>> char *node, char *type, } else
>>>>>     nfs_error(_("%s: internal option parsing error"), progname);
>>>>> 
>>>>> -    freeaddrinfo(mi.address);
>>>>> +    if (mi.address)
>>>>> +        freeaddrinfo(mi.address);
>>>>> free(mi.hostname);
>>>>> return retval;
>>>>> }
>>>>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>>>>> index 7e8d403..8cee1c8 100644
>>>>> --- a/utils/mountd/cache.c
>>>>> +++ b/utils/mountd/cache.c
>>>>> @@ -834,7 +834,8 @@ static void nfsd_fh(int f)
>>>>> out:
>>>>> if (found_path)
>>>>>     free(found_path);
>>>>> -    freeaddrinfo(ai);
>>>>> +    if(ai)
>>>>> +        freeaddrinfo(ai);
>>>>> free(dom);
>>>>> xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ?
>>>>> found->e_path : NULL); }
>>>>> @@ -1355,7 +1356,8 @@ static void nfsd_export(int f)
>>>>> xlog(D_CALL, "nfsd_export: found %p path %s", found, path ?
>>>>> path : NULL); if (dom) free(dom);
>>>>> if (path) free(path);
>>>>> -    freeaddrinfo(ai);
>>>>> +    if (ai)
>>>>> +        freeaddrinfo(ai);
>>>>> }
>>>>> 
>>>>> 
>>>>> diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
>>>>> index 8cccdb8..6556ab1 100644
>>>>> --- a/utils/statd/hostname.c
>>>>> +++ b/utils/statd/hostname.c
>>>>> @@ -308,8 +308,10 @@ statd_matchhostname(const char *hostname1,
>>>>> const char *hostname2) }
>>>>> 
>>>>> out:
>>>>> -    freeaddrinfo(results2);
>>>>> -    freeaddrinfo(results1);
>>>>> +    if (results2)
>>>>> +        freeaddrinfo(results2);
>>>>> +    if (results1)
>>>>> +        freeaddrinfo(results1);
>>>>> 
>>>>> xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
>>>>>         hostname1, hostname2,
>>>>> -- 
>>>>> 2.20.1
>>>>> 
>>>> 
>>> 
>> 
>> --
>> Chuck Lever

--
Chuck Lever

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Steve Dickson]

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....

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. 
> 
> 

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Chuck Lever]

> 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

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Peter Wagner]

Hi Chuck,

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

yeah I'm fine with this :)


The code in support/export/hostname.c

	if (ai->ai_canonname == NULL) {
		nfs_freeaddrinfo(ai);
		ai = NULL;
	}

could also be removed as the "if (ai-> if (ai->ai_canonname == NULL)"
just checks if the not anymore existing strdup was successful.

as it seems the "ai->ai_canonname = strdup(buf);" isn't necessary, as
it just copies the IP address which can be done in host_pton with the 

	.ai_flags       = AI_CANONNAME

directly.


From 260fb3ea7f06eca4498c00a07e097939652db6c2 Mon Sep 17 00:00:00 2001
From: Peter Wagner <tripolar@gmx.at>
Date: Mon, 18 Feb 2019 22:07:50 +0100
Subject: [PATCH] define and use wrapper function nfs_freeaddrinfo to
handle freeaddrinfo versions that don't tolerate NULL pointers set
AI_CANONNAME flag in host_pton to set the ip address of the connected
host to ai_canonname

---
 support/export/client.c       |  6 +++---
 support/export/hostname.c     | 12 +++++-------
 support/include/exportfs.h    | 11 +++++++++++
 support/nfs/getport.c         |  7 ++++---
 support/nfs/svc_create.c      |  8 +++++---
 support/nfsidmap/umich_ldap.c |  2 +-
 tests/nsm_client/nsm_client.c |  2 +-
 utils/exportfs/exportfs.c     | 10 +++++-----
 utils/gssd/gssd.c             |  4 ++--
 utils/gssd/krb5_util.c        |  2 +-
 utils/mount/network.c         |  7 ++++---
 utils/mount/stropts.c         |  3 ++-
 utils/mountd/auth.c           |  2 +-
 utils/mountd/cache.c          | 10 +++++-----
 utils/mountd/mountd.c         |  4 ++--
 utils/mountd/rmtab.c          |  2 +-
 utils/nfsd/nfssvc.c           |  4 ++--
 utils/statd/hostname.c        | 11 ++++++-----
 utils/statd/sm-notify.c       | 14 +++++++-------
 19 files changed, 68 insertions(+), 53 deletions(-)

diff --git a/support/export/client.c b/support/export/client.c
index baf59c8..a1fba01 100644
--- a/support/export/client.c
+++ b/support/export/client.c
@@ -210,7 +210,7 @@ init_subnetwork(nfs_client *clp)
 	set_addrlist(clp, 0, ai->ai_addr);
 	family = ai->ai_addr->sa_family;
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 
 	switch (family) {
 	case AF_INET:
@@ -309,7 +309,7 @@ client_lookup(char *hname, int canonical)
 		init_addrlist(clp, ai);
 
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return clp;
 }
 
@@ -674,7 +674,7 @@ check_netgroup(const nfs_client *clp, const struct
addrinfo *ai) tmp = host_pton(hname);
 	if (tmp != NULL) {
 		char *cname = host_canonname(tmp->ai_addr);
-		freeaddrinfo(tmp);
+		nfs_freeaddrinfo(tmp);
 
 		/* The resulting FQDN may be in our netgroup. */
 		if (cname != NULL) {
diff --git a/support/export/hostname.c b/support/export/hostname.c
index 5c4c824..a6032ad 100644
--- a/support/export/hostname.c
+++ b/support/export/hostname.c
@@ -101,6 +101,7 @@ host_pton(const char *paddr)
 		.ai_protocol	= (int)IPPROTO_UDP,
 		.ai_flags	= AI_NUMERICHOST,
 		.ai_family	= AF_UNSPEC,
+		.ai_flags	= AI_CANONNAME,
 	};
 	struct sockaddr_in sin;
 	int error, inet4;
@@ -130,7 +131,7 @@ host_pton(const char *paddr)
 		if (!inet4 && ai->ai_addr->sa_family == AF_INET) {
 			xlog(D_GENERAL, "%s: failed to convert %s",
 					__func__, paddr);
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			break;
 		}
 		return ai;
@@ -290,7 +291,7 @@ host_reliable_addrinfo(const struct sockaddr *sap)
 		if (nfs_compare_sockaddr(a->ai_addr, sap))
 			break;
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	if (!a)
 		goto out_free_hostname;
 
@@ -354,10 +355,8 @@ host_numeric_addrinfo(const struct sockaddr *sap)
 	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
 	 */
 	if (ai != NULL) {
-		free(ai->ai_canonname);		/* just in case
*/
-		ai->ai_canonname = strdup(buf);
 		if (ai->ai_canonname == NULL) {
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			ai = NULL;
 		}
 	}
@@ -388,9 +387,8 @@ host_numeric_addrinfo(const struct sockaddr *sap)
 	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
 	 */
 	if (ai != NULL) {
-		ai->ai_canonname = strdup(buf);
 		if (ai->ai_canonname == NULL) {
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			ai = NULL;
 		}
 	}
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 4e0d9d1..b81f963 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -47,6 +47,17 @@ typedef struct mclient {
 	int			m_count;
 } nfs_client;
 
+/*
+ * Some versions of freeaddrinfo(3) do not tolerate being
+ * passed a NULL pointer.
+ */
+static inline void nfs_freeaddrinfo(struct addrinfo *ai)
+{
+	if (ai) {
+		freeaddrinfo(ai);
+	}
+}
+
 static inline const struct sockaddr *
 get_addrlist(const nfs_client *clp, const int i)
 {
diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 081594c..26ec85e 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -47,6 +47,7 @@
 
 #include "sockaddr.h"
 #include "nfsrpc.h"
+#include "exportfs.h"
 
 /*
  * Try a local socket first to access the local rpcbind daemon
@@ -109,7 +110,7 @@ static int nfs_gp_loopback_address(struct sockaddr
*sap, socklen_t *salen) ret = 1;
 	}
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 	return ret;
 }
 
@@ -134,8 +135,8 @@ static in_port_t nfs_gp_getservbyname(const char
*service, 
 	sin = (const struct sockaddr_in *)gai_results->ai_addr;
 	port = sin->sin_port;
-	
-	freeaddrinfo(gai_results);
+
+	nfs_freeaddrinfo(gai_results);
 	return port;
 }
 
diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index ef7ff05..d0b747b 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -39,6 +39,8 @@
 #include <rpc/rpc.h>
 #include <rpc/svc.h>
 
+#include "exportfs.h"
+
 #ifdef HAVE_TCP_WRAPPER
 #include "tcpwrapper.h"
 #endif
@@ -273,7 +275,7 @@ svc_create_nconf_rand_port(const char *name, const
rpcprog_t program, bindaddr.qlen = SOMAXCONN;
 
 	xprt = svc_tli_create(RPC_ANYFD, nconf, &bindaddr, 0, 0);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	if (xprt == NULL) {
 		xlog(L_ERROR, "Failed to create listener xprt "
 			"(%s, %u, %s)", name, version,
nconf->nc_netid); @@ -364,11 +366,11 @@
svc_create_nconf_fixed_port(const char *name, const rpcprog_t program, 
 	svc_create_cache_xprt(xprt);
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return 1;
 
 out_free:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return 0;
 }
 
diff --git a/support/nfsidmap/umich_ldap.c
b/support/nfsidmap/umich_ldap.c index b661110..b8ee184 100644
--- a/support/nfsidmap/umich_ldap.c
+++ b/support/nfsidmap/umich_ldap.c
@@ -1089,7 +1089,7 @@ get_canonical_hostname(const char *inname)
 	return_name = strdup (tmphost);
 
 out_free:
-	freeaddrinfo(ap);
+	nfs_freeaddrinfo(ap);
 out_err:
 	return return_name;
 }
diff --git a/tests/nsm_client/nsm_client.c
b/tests/nsm_client/nsm_client.c index 0fa3422..8dc0591 100644
--- a/tests/nsm_client/nsm_client.c
+++ b/tests/nsm_client/nsm_client.c
@@ -243,7 +243,7 @@ nsm_client_get_rpcclient(const char *node)
 		printf("RPC client creation failed\n");
 	}
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return client;
 }
 
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index cd3c979..333eadc 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -282,7 +282,7 @@ exportfs_parsed(char *hname, char *path, char
*options, int verbose) validate_export(exp);
 
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 static int exportfs_generic(char *arg, char *options, int verbose)
@@ -395,7 +395,7 @@ unexportfs_parsed(char *hname, char *path, int
verbose) if (!success)
 		xlog(L_ERROR, "Could not find '%s:%s' to unexport.",
hname, path); 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 static int unexportfs_generic(char *arg, int verbose)
@@ -588,7 +588,7 @@ address_list(const char *hostname)
 	if (ai != NULL) {
 		/* @hostname was a presentation address */
 		cname = host_canonname(ai->ai_addr);
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (cname != NULL)
 			goto out;
 	}
@@ -639,8 +639,8 @@ matchhostname(const char *hostname1, const char
*hostname2) }
 
 out:
-	freeaddrinfo(results1);
-	freeaddrinfo(results2);
+	nfs_freeaddrinfo(results1);
+	nfs_freeaddrinfo(results2);
 	return result;
 }
 
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 2e92f28..7eeb05f 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -172,14 +172,14 @@ gssd_addrstr_to_sockaddr(struct sockaddr *sa,
const char *node, const char *port if (sin6->sin6_scope_id) {
 			printerr(0, "ERROR: address %s has non-zero "
 				    "sin6_scope_id!\n", node);
-			freeaddrinfo(res);
+			nfs_freeaddrinfo(res);
 			return false;
 		}
 	}
 #endif /* IPV6_SUPPORTED */
 
 	memcpy(sa, res->ai_addr, res->ai_addrlen);
-	freeaddrinfo(res);
+	nfs_freeaddrinfo(res);
 	return true;
 }
 
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index eba1aac..adbde93 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -587,7 +587,7 @@ get_full_hostname(const char *inhost, char
*outhost, int outhostlen) goto out;
 	}
 	strncpy(outhost, addrs->ai_canonname, outhostlen);
-	freeaddrinfo(addrs);
+	nfs_freeaddrinfo(addrs);
 	for (c = outhost; *c != '\0'; c++)
 	    *c = tolower(*c);
 
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 356f663..fcb0b9f 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -53,6 +53,7 @@
 #include <net/if.h>
 #include <ifaddrs.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "xcommon.h"
 #include "mount.h"
@@ -250,7 +251,7 @@ int nfs_lookup(const char *hostname, const
sa_family_t family, break;
 	}
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 	return ret;
 }
 
@@ -307,7 +308,7 @@ int nfs_string_to_sockaddr(const char *address,
struct sockaddr *sap, }
 			break;
 		}
-		freeaddrinfo(gai_results);
+		nfs_freeaddrinfo(gai_results);
 	}
 
 	return ret;
@@ -1180,7 +1181,7 @@ static int nfs_ca_gai(const struct sockaddr *sap,
 	*buflen = gai_results->ai_addrlen;
 	memcpy(buf, gai_results->ai_addr, *buflen);
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 
 	return 1;
 }
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 0a25b1f..b170552 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -35,6 +35,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "xcommon.h"
 #include "mount.h"
@@ -1268,7 +1269,7 @@ int nfsmount_string(const char *spec, const char
*node, char *type, } else
 		nfs_error(_("%s: internal option parsing error"),
progname); 
-	freeaddrinfo(mi.address);
+	nfs_freeaddrinfo(mi.address);
 	free(mi.hostname);
 	return retval;
 }
diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c
index 8299256..8f7cbd4 100644
--- a/utils/mountd/auth.c
+++ b/utils/mountd/auth.c
@@ -297,7 +297,7 @@ auth_authenticate(const char *what, const struct
sockaddr *caller, path, epath, error);
 	}
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return exp;
 }
 
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 7e8d403..2cb370f 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -113,7 +113,7 @@ static void auth_unix_ip(int f)
 		ai = client_resolve(tmp->ai_addr);
 		if (ai) {
 			client = client_compose(ai);
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 		}
 	}
 	bp = buf; blen = sizeof(buf);
@@ -133,7 +133,7 @@ static void auth_unix_ip(int f)
 	xlog(D_CALL, "auth_unix_ip: client %p '%s'", client,
client?client: "DEFAULT"); 
 	free(client);
-	freeaddrinfo(tmp);
+	nfs_freeaddrinfo(tmp);
 
 }
 
@@ -667,7 +667,7 @@ static struct addrinfo *lookup_client_addr(char
*dom) if (tmp == NULL)
 		return NULL;
 	ret = client_resolve(tmp->ai_addr);
-	freeaddrinfo(tmp);
+	nfs_freeaddrinfo(tmp);
 	return ret;
 }
 
@@ -834,7 +834,7 @@ static void nfsd_fh(int f)
 out:
 	if (found_path)
 		free(found_path);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	free(dom);
 	xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ?
found->e_path : NULL); }
@@ -1355,7 +1355,7 @@ static void nfsd_export(int f)
 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ?
path : NULL); if (dom) free(dom);
 	if (path) free(path);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 086c39b..fb7bba4 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -578,10 +578,10 @@ static void prune_clients(nfs_export *exp, struct
exportnode *e) *cp = c->gr_next;
 				xfree(c->gr_name);
 				xfree(c);
-				freeaddrinfo(ai);
+				nfs_freeaddrinfo(ai);
 				continue;
 			}
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 		}
 		cp = &(c->gr_next);
 	}
diff --git a/utils/mountd/rmtab.c b/utils/mountd/rmtab.c
index 3ae0dbb..c896243 100644
--- a/utils/mountd/rmtab.c
+++ b/utils/mountd/rmtab.c
@@ -226,7 +226,7 @@ mountlist_list(void)
 				ai = host_pton(rep->r_client);
 				if (ai != NULL) {
 					m->ml_hostname =
host_canonname(ai->ai_addr);
-					freeaddrinfo(ai);
+					nfs_freeaddrinfo(ai);
 				}
 			}
 			if (m->ml_hostname == NULL)
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 1e6ffd6..47b1882 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -25,6 +25,7 @@
 #include "nfslib.h"
 #include "xlog.h"
 #include "nfssvc.h"
+#include "exportfs.h"
 #include "../mount/version.h"
 
 #ifndef NFSD_FS_DIR
@@ -246,8 +247,7 @@ error:
 		close(fd);
 	if (sockfd >= 0)
 		close(sockfd);
-	if (addrhead)
-		freeaddrinfo(addrhead);
+	nfs_freeaddrinfo(addrhead);
 	return (bounded ? 0 : rc);
 }
 
diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
index 8cccdb8..c9e22d3 100644
--- a/utils/statd/hostname.c
+++ b/utils/statd/hostname.c
@@ -35,6 +35,7 @@
 #include <netdb.h>
 #include <arpa/inet.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "statd.h"
 #include "xlog.h"
@@ -203,7 +204,7 @@ statd_canonical_name(const char *hostname)
 		_Bool result;
 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
 					buf, (socklen_t)sizeof(buf));
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (!result || buf[0] == '\0')
 			/* OK to use presentation address,
 			 * if no reverse map exists */
@@ -217,7 +218,7 @@ statd_canonical_name(const char *hostname)
 	if (ai == NULL)
 		return NULL;
 	strcpy(buf, ai->ai_canonname);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 
 	return strdup(buf);
 }
@@ -253,7 +254,7 @@ statd_canonical_list(const char *hostname)
 		_Bool result;
 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
 					buf, (socklen_t)sizeof(buf));
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (result)
 			goto out;
 	}
@@ -308,8 +309,8 @@ statd_matchhostname(const char *hostname1, const
char *hostname2) }
 
 out:
-	freeaddrinfo(results2);
-	freeaddrinfo(results1);
+	nfs_freeaddrinfo(results2);
+	nfs_freeaddrinfo(results1);
 
 	xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
 			hostname1, hostname2,
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 29dad38..05d72a3 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -37,6 +37,7 @@
 #include "xlog.h"
 #include "nsm.h"
 #include "nfsrpc.h"
+#include "exportfs.h"
 
 /* glibc before 2.3.4 */
 #ifndef AI_NUMERICSERV
@@ -179,7 +180,7 @@ smn_verify_my_name(const char *name)
 	case 0:
 		/* @name was a presentation address */
 		retval = smn_get_hostname(ai->ai_addr, ai->ai_addrlen,
name);
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (retval == NULL)
 			return NULL;
 		break;
@@ -253,8 +254,7 @@ static void smn_forget_host(struct nsm_host *host)
 	free((void *)host->my_name);
 	free((void *)host->mon_name);
 	free(host->name);
-	if (host->ai)
-		freeaddrinfo(host->ai);
+	nfs_freeaddrinfo(host->ai);
 
 	free(host);
 }
@@ -430,7 +430,7 @@ retry:
 	if (srcport) {
 		if (bind(sock, ai->ai_addr, ai->ai_addrlen) == -1) {
 			xlog(L_ERROR, "Failed to bind RPC socket: %m");
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			return -1;
 		}
@@ -440,7 +440,7 @@ retry:
 		if (smn_bindresvport(sock, ai->ai_addr) == -1) {
 			xlog(L_ERROR,
 				"bindresvport on RPC socket failed:
%m");
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			return -1;
 		}
@@ -449,13 +449,13 @@ retry:
 		se = getservbyport((int)nfs_get_port(ai->ai_addr),
"udp"); if (se != NULL && retry_cnt < 100) {
 			retry_cnt++;
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			goto retry;
 		}
 	}
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return sock;
 }
 
-- 
2.20.1

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Peter Wagner]

resend because wrapping was on in the last mail

Hi Chuck,

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

yeah I'm fine with this :)


The code in support/export/hostname.c

	if (ai->ai_canonname == NULL) {
		nfs_freeaddrinfo(ai);
		ai = NULL;
	}

could also be removed (jsut the if) as the "if (ai-> if (ai->ai_canonname == NULL)" 
just checks if the not anymore existing strdup was successful.

as it seems the "ai->ai_canonname = strdup(buf);" isn't necessary, as
it just copies the IP address which can be done in host_pton with the 

	.ai_flags       = AI_CANONNAME

directly.


diff --git a/support/export/client.c b/support/export/client.c
index baf59c8..a1fba01 100644
--- a/support/export/client.c
+++ b/support/export/client.c
@@ -210,7 +210,7 @@ init_subnetwork(nfs_client *clp)
 	set_addrlist(clp, 0, ai->ai_addr);
 	family = ai->ai_addr->sa_family;
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 
 	switch (family) {
 	case AF_INET:
@@ -309,7 +309,7 @@ client_lookup(char *hname, int canonical)
 		init_addrlist(clp, ai);
 
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return clp;
 }
 
@@ -674,7 +674,7 @@ check_netgroup(const nfs_client *clp, const struct addrinfo *ai)
 	tmp = host_pton(hname);
 	if (tmp != NULL) {
 		char *cname = host_canonname(tmp->ai_addr);
-		freeaddrinfo(tmp);
+		nfs_freeaddrinfo(tmp);
 
 		/* The resulting FQDN may be in our netgroup. */
 		if (cname != NULL) {
diff --git a/support/export/hostname.c b/support/export/hostname.c
index 5c4c824..a6032ad 100644
--- a/support/export/hostname.c
+++ b/support/export/hostname.c
@@ -101,6 +101,7 @@ host_pton(const char *paddr)
 		.ai_protocol	= (int)IPPROTO_UDP,
 		.ai_flags	= AI_NUMERICHOST,
 		.ai_family	= AF_UNSPEC,
+		.ai_flags	= AI_CANONNAME,
 	};
 	struct sockaddr_in sin;
 	int error, inet4;
@@ -130,7 +131,7 @@ host_pton(const char *paddr)
 		if (!inet4 && ai->ai_addr->sa_family == AF_INET) {
 			xlog(D_GENERAL, "%s: failed to convert %s",
 					__func__, paddr);
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			break;
 		}
 		return ai;
@@ -290,7 +291,7 @@ host_reliable_addrinfo(const struct sockaddr *sap)
 		if (nfs_compare_sockaddr(a->ai_addr, sap))
 			break;
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	if (!a)
 		goto out_free_hostname;
 
@@ -354,10 +355,8 @@ host_numeric_addrinfo(const struct sockaddr *sap)
 	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
 	 */
 	if (ai != NULL) {
-		free(ai->ai_canonname);		/* just in case */
-		ai->ai_canonname = strdup(buf);
 		if (ai->ai_canonname == NULL) {
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			ai = NULL;
 		}
 	}
@@ -388,9 +387,8 @@ host_numeric_addrinfo(const struct sockaddr *sap)
 	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
 	 */
 	if (ai != NULL) {
-		ai->ai_canonname = strdup(buf);
 		if (ai->ai_canonname == NULL) {
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			ai = NULL;
 		}
 	}
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 4e0d9d1..b81f963 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -47,6 +47,17 @@ typedef struct mclient {
 	int			m_count;
 } nfs_client;
 
+/*
+ * Some versions of freeaddrinfo(3) do not tolerate being
+ * passed a NULL pointer.
+ */
+static inline void nfs_freeaddrinfo(struct addrinfo *ai)
+{
+	if (ai) {
+		freeaddrinfo(ai);
+	}
+}
+
 static inline const struct sockaddr *
 get_addrlist(const nfs_client *clp, const int i)
 {
diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 081594c..26ec85e 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -47,6 +47,7 @@
 
 #include "sockaddr.h"
 #include "nfsrpc.h"
+#include "exportfs.h"
 
 /*
  * Try a local socket first to access the local rpcbind daemon
@@ -109,7 +110,7 @@ static int nfs_gp_loopback_address(struct sockaddr *sap, socklen_t *salen)
 		ret = 1;
 	}
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 	return ret;
 }
 
@@ -134,8 +135,8 @@ static in_port_t nfs_gp_getservbyname(const char *service,
 
 	sin = (const struct sockaddr_in *)gai_results->ai_addr;
 	port = sin->sin_port;
-	
-	freeaddrinfo(gai_results);
+
+	nfs_freeaddrinfo(gai_results);
 	return port;
 }
 
diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index ef7ff05..d0b747b 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -39,6 +39,8 @@
 #include <rpc/rpc.h>
 #include <rpc/svc.h>
 
+#include "exportfs.h"
+
 #ifdef HAVE_TCP_WRAPPER
 #include "tcpwrapper.h"
 #endif
@@ -273,7 +275,7 @@ svc_create_nconf_rand_port(const char *name, const rpcprog_t program,
 	bindaddr.qlen = SOMAXCONN;
 
 	xprt = svc_tli_create(RPC_ANYFD, nconf, &bindaddr, 0, 0);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	if (xprt == NULL) {
 		xlog(L_ERROR, "Failed to create listener xprt "
 			"(%s, %u, %s)", name, version, nconf->nc_netid);
@@ -364,11 +366,11 @@ svc_create_nconf_fixed_port(const char *name, const rpcprog_t program,
 
 	svc_create_cache_xprt(xprt);
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return 1;
 
 out_free:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return 0;
 }
 
diff --git a/support/nfsidmap/umich_ldap.c b/support/nfsidmap/umich_ldap.c
index b661110..b8ee184 100644
--- a/support/nfsidmap/umich_ldap.c
+++ b/support/nfsidmap/umich_ldap.c
@@ -1089,7 +1089,7 @@ get_canonical_hostname(const char *inname)
 	return_name = strdup (tmphost);
 
 out_free:
-	freeaddrinfo(ap);
+	nfs_freeaddrinfo(ap);
 out_err:
 	return return_name;
 }
diff --git a/tests/nsm_client/nsm_client.c b/tests/nsm_client/nsm_client.c
index 0fa3422..8dc0591 100644
--- a/tests/nsm_client/nsm_client.c
+++ b/tests/nsm_client/nsm_client.c
@@ -243,7 +243,7 @@ nsm_client_get_rpcclient(const char *node)
 		printf("RPC client creation failed\n");
 	}
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return client;
 }
 
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index cd3c979..333eadc 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -282,7 +282,7 @@ exportfs_parsed(char *hname, char *path, char *options, int verbose)
 	validate_export(exp);
 
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 static int exportfs_generic(char *arg, char *options, int verbose)
@@ -395,7 +395,7 @@ unexportfs_parsed(char *hname, char *path, int verbose)
 	if (!success)
 		xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname, path);
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 static int unexportfs_generic(char *arg, int verbose)
@@ -588,7 +588,7 @@ address_list(const char *hostname)
 	if (ai != NULL) {
 		/* @hostname was a presentation address */
 		cname = host_canonname(ai->ai_addr);
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (cname != NULL)
 			goto out;
 	}
@@ -639,8 +639,8 @@ matchhostname(const char *hostname1, const char *hostname2)
 			}
 
 out:
-	freeaddrinfo(results1);
-	freeaddrinfo(results2);
+	nfs_freeaddrinfo(results1);
+	nfs_freeaddrinfo(results2);
 	return result;
 }
 
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 2e92f28..7eeb05f 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -172,14 +172,14 @@ gssd_addrstr_to_sockaddr(struct sockaddr *sa, const char *node, const char *port
 		if (sin6->sin6_scope_id) {
 			printerr(0, "ERROR: address %s has non-zero "
 				    "sin6_scope_id!\n", node);
-			freeaddrinfo(res);
+			nfs_freeaddrinfo(res);
 			return false;
 		}
 	}
 #endif /* IPV6_SUPPORTED */
 
 	memcpy(sa, res->ai_addr, res->ai_addrlen);
-	freeaddrinfo(res);
+	nfs_freeaddrinfo(res);
 	return true;
 }
 
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index eba1aac..adbde93 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -587,7 +587,7 @@ get_full_hostname(const char *inhost, char *outhost, int outhostlen)
 		goto out;
 	}
 	strncpy(outhost, addrs->ai_canonname, outhostlen);
-	freeaddrinfo(addrs);
+	nfs_freeaddrinfo(addrs);
 	for (c = outhost; *c != '\0'; c++)
 	    *c = tolower(*c);
 
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 356f663..fcb0b9f 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -53,6 +53,7 @@
 #include <net/if.h>
 #include <ifaddrs.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "xcommon.h"
 #include "mount.h"
@@ -250,7 +251,7 @@ int nfs_lookup(const char *hostname, const sa_family_t family,
 		break;
 	}
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 	return ret;
 }
 
@@ -307,7 +308,7 @@ int nfs_string_to_sockaddr(const char *address, struct sockaddr *sap,
 			}
 			break;
 		}
-		freeaddrinfo(gai_results);
+		nfs_freeaddrinfo(gai_results);
 	}
 
 	return ret;
@@ -1180,7 +1181,7 @@ static int nfs_ca_gai(const struct sockaddr *sap,
 	*buflen = gai_results->ai_addrlen;
 	memcpy(buf, gai_results->ai_addr, *buflen);
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 
 	return 1;
 }
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 0a25b1f..b170552 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -35,6 +35,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "xcommon.h"
 #include "mount.h"
@@ -1268,7 +1269,7 @@ int nfsmount_string(const char *spec, const char *node, char *type,
 	} else
 		nfs_error(_("%s: internal option parsing error"), progname);
 
-	freeaddrinfo(mi.address);
+	nfs_freeaddrinfo(mi.address);
 	free(mi.hostname);
 	return retval;
 }
diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c
index 8299256..8f7cbd4 100644
--- a/utils/mountd/auth.c
+++ b/utils/mountd/auth.c
@@ -297,7 +297,7 @@ auth_authenticate(const char *what, const struct sockaddr *caller,
 			path, epath, error);
 	}
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return exp;
 }
 
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 7e8d403..2cb370f 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -113,7 +113,7 @@ static void auth_unix_ip(int f)
 		ai = client_resolve(tmp->ai_addr);
 		if (ai) {
 			client = client_compose(ai);
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 		}
 	}
 	bp = buf; blen = sizeof(buf);
@@ -133,7 +133,7 @@ static void auth_unix_ip(int f)
 	xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");
 
 	free(client);
-	freeaddrinfo(tmp);
+	nfs_freeaddrinfo(tmp);
 
 }
 
@@ -667,7 +667,7 @@ static struct addrinfo *lookup_client_addr(char *dom)
 	if (tmp == NULL)
 		return NULL;
 	ret = client_resolve(tmp->ai_addr);
-	freeaddrinfo(tmp);
+	nfs_freeaddrinfo(tmp);
 	return ret;
 }
 
@@ -834,7 +834,7 @@ static void nfsd_fh(int f)
 out:
 	if (found_path)
 		free(found_path);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	free(dom);
 	xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ? found->e_path : NULL);
 }
@@ -1355,7 +1355,7 @@ static void nfsd_export(int f)
 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
 	if (dom) free(dom);
 	if (path) free(path);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 086c39b..fb7bba4 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -578,10 +578,10 @@ static void prune_clients(nfs_export *exp, struct exportnode *e)
 				*cp = c->gr_next;
 				xfree(c->gr_name);
 				xfree(c);
-				freeaddrinfo(ai);
+				nfs_freeaddrinfo(ai);
 				continue;
 			}
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 		}
 		cp = &(c->gr_next);
 	}
diff --git a/utils/mountd/rmtab.c b/utils/mountd/rmtab.c
index 3ae0dbb..c896243 100644
--- a/utils/mountd/rmtab.c
+++ b/utils/mountd/rmtab.c
@@ -226,7 +226,7 @@ mountlist_list(void)
 				ai = host_pton(rep->r_client);
 				if (ai != NULL) {
 					m->ml_hostname = host_canonname(ai->ai_addr);
-					freeaddrinfo(ai);
+					nfs_freeaddrinfo(ai);
 				}
 			}
 			if (m->ml_hostname == NULL)
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 1e6ffd6..47b1882 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -25,6 +25,7 @@
 #include "nfslib.h"
 #include "xlog.h"
 #include "nfssvc.h"
+#include "exportfs.h"
 #include "../mount/version.h"
 
 #ifndef NFSD_FS_DIR
@@ -246,8 +247,7 @@ error:
 		close(fd);
 	if (sockfd >= 0)
 		close(sockfd);
-	if (addrhead)
-		freeaddrinfo(addrhead);
+	nfs_freeaddrinfo(addrhead);
 	return (bounded ? 0 : rc);
 }
 
diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
index 8cccdb8..c9e22d3 100644
--- a/utils/statd/hostname.c
+++ b/utils/statd/hostname.c
@@ -35,6 +35,7 @@
 #include <netdb.h>
 #include <arpa/inet.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "statd.h"
 #include "xlog.h"
@@ -203,7 +204,7 @@ statd_canonical_name(const char *hostname)
 		_Bool result;
 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
 					buf, (socklen_t)sizeof(buf));
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (!result || buf[0] == '\0')
 			/* OK to use presentation address,
 			 * if no reverse map exists */
@@ -217,7 +218,7 @@ statd_canonical_name(const char *hostname)
 	if (ai == NULL)
 		return NULL;
 	strcpy(buf, ai->ai_canonname);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 
 	return strdup(buf);
 }
@@ -253,7 +254,7 @@ statd_canonical_list(const char *hostname)
 		_Bool result;
 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
 					buf, (socklen_t)sizeof(buf));
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (result)
 			goto out;
 	}
@@ -308,8 +309,8 @@ statd_matchhostname(const char *hostname1, const char *hostname2)
 			}
 
 out:
-	freeaddrinfo(results2);
-	freeaddrinfo(results1);
+	nfs_freeaddrinfo(results2);
+	nfs_freeaddrinfo(results1);
 
 	xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
 			hostname1, hostname2,
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 29dad38..05d72a3 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -37,6 +37,7 @@
 #include "xlog.h"
 #include "nsm.h"
 #include "nfsrpc.h"
+#include "exportfs.h"
 
 /* glibc before 2.3.4 */
 #ifndef AI_NUMERICSERV
@@ -179,7 +180,7 @@ smn_verify_my_name(const char *name)
 	case 0:
 		/* @name was a presentation address */
 		retval = smn_get_hostname(ai->ai_addr, ai->ai_addrlen, name);
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (retval == NULL)
 			return NULL;
 		break;
@@ -253,8 +254,7 @@ static void smn_forget_host(struct nsm_host *host)
 	free((void *)host->my_name);
 	free((void *)host->mon_name);
 	free(host->name);
-	if (host->ai)
-		freeaddrinfo(host->ai);
+	nfs_freeaddrinfo(host->ai);
 
 	free(host);
 }
@@ -430,7 +430,7 @@ retry:
 	if (srcport) {
 		if (bind(sock, ai->ai_addr, ai->ai_addrlen) == -1) {
 			xlog(L_ERROR, "Failed to bind RPC socket: %m");
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			return -1;
 		}
@@ -440,7 +440,7 @@ retry:
 		if (smn_bindresvport(sock, ai->ai_addr) == -1) {
 			xlog(L_ERROR,
 				"bindresvport on RPC socket failed: %m");
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			return -1;
 		}
@@ -449,13 +449,13 @@ retry:
 		se = getservbyport((int)nfs_get_port(ai->ai_addr), "udp");
 		if (se != NULL && retry_cnt < 100) {
 			retry_cnt++;
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			goto retry;
 		}
 	}
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return sock;
 }
 

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Peter Wagner]

Hi,

i updated to code to the latest version

From dc2c02c12cf66e7ac4ab12fad7ea21f83f10ee7a Mon Sep 17 00:00:00 2001
From: Peter Wagner <tripolar@gmx.at>
Date: Tue, 19 Feb 2019 09:53:07 +0100
Subject: [PATCH] define and use wrapper function nfs_freeaddrinfo to handle
 freeaddrinfo versions that don't tolerate NULL pointers

set AI_CANONNAME flag in host_pton to set the ip address of the connected
host to ai_canonname

Signed-off-by: Peter Wagner <tripolar@gmx.at>
---
 support/export/client.c       |  6 +++---
 support/export/hostname.c     | 28 +++-------------------------
 support/include/exportfs.h    | 11 +++++++++++
 support/nfs/getport.c         |  7 ++++---
 support/nfs/svc_create.c      |  8 +++++---
 support/nfsidmap/umich_ldap.c |  2 +-
 tests/nsm_client/nsm_client.c |  2 +-
 utils/exportfs/exportfs.c     | 10 +++++-----
 utils/gssd/gssd.c             |  4 ++--
 utils/gssd/krb5_util.c        |  2 +-
 utils/mount/network.c         |  7 ++++---
 utils/mount/stropts.c         |  3 ++-
 utils/mountd/auth.c           |  2 +-
 utils/mountd/cache.c          | 10 +++++-----
 utils/mountd/mountd.c         |  4 ++--
 utils/mountd/rmtab.c          |  2 +-
 utils/nfsd/nfssvc.c           |  4 ++--
 utils/statd/hostname.c        | 11 ++++++-----
 utils/statd/sm-notify.c       | 14 +++++++-------
 19 files changed, 66 insertions(+), 71 deletions(-)

diff --git a/support/export/client.c b/support/export/client.c
index baf59c8..a1fba01 100644
--- a/support/export/client.c
+++ b/support/export/client.c
@@ -210,7 +210,7 @@ init_subnetwork(nfs_client *clp)
 	set_addrlist(clp, 0, ai->ai_addr);
 	family = ai->ai_addr->sa_family;
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 
 	switch (family) {
 	case AF_INET:
@@ -309,7 +309,7 @@ client_lookup(char *hname, int canonical)
 		init_addrlist(clp, ai);
 
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return clp;
 }
 
@@ -674,7 +674,7 @@ check_netgroup(const nfs_client *clp, const struct addrinfo *ai)
 	tmp = host_pton(hname);
 	if (tmp != NULL) {
 		char *cname = host_canonname(tmp->ai_addr);
-		freeaddrinfo(tmp);
+		nfs_freeaddrinfo(tmp);
 
 		/* The resulting FQDN may be in our netgroup. */
 		if (cname != NULL) {
diff --git a/support/export/hostname.c b/support/export/hostname.c
index 5c4c824..9845a59 100644
--- a/support/export/hostname.c
+++ b/support/export/hostname.c
@@ -101,6 +101,7 @@ host_pton(const char *paddr)
 		.ai_protocol	= (int)IPPROTO_UDP,
 		.ai_flags	= AI_NUMERICHOST,
 		.ai_family	= AF_UNSPEC,
+		.ai_flags	= AI_CANONNAME,
 	};
 	struct sockaddr_in sin;
 	int error, inet4;
@@ -130,7 +131,7 @@ host_pton(const char *paddr)
 		if (!inet4 && ai->ai_addr->sa_family == AF_INET) {
 			xlog(D_GENERAL, "%s: failed to convert %s",
 					__func__, paddr);
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			break;
 		}
 		return ai;
@@ -290,7 +291,7 @@ host_reliable_addrinfo(const struct sockaddr *sap)
 		if (nfs_compare_sockaddr(a->ai_addr, sap))
 			break;
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	if (!a)
 		goto out_free_hostname;
 
@@ -350,18 +351,6 @@ host_numeric_addrinfo(const struct sockaddr *sap)
 
 	ai = host_pton(buf);
 
-	/*
-	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
-	 */
-	if (ai != NULL) {
-		free(ai->ai_canonname);		/* just in case */
-		ai->ai_canonname = strdup(buf);
-		if (ai->ai_canonname == NULL) {
-			freeaddrinfo(ai);
-			ai = NULL;
-		}
-	}
-
 	return ai;
 }
 #else	/* !HAVE_GETNAMEINFO */
@@ -384,17 +373,6 @@ host_numeric_addrinfo(const struct sockaddr *sap)
 
 	ai = host_pton(buf);
 
-	/*
-	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
-	 */
-	if (ai != NULL) {
-		ai->ai_canonname = strdup(buf);
-		if (ai->ai_canonname == NULL) {
-			freeaddrinfo(ai);
-			ai = NULL;
-		}
-	}
-
 	return ai;
 }
 #endif	/* !HAVE_GETNAMEINFO */
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 4e0d9d1..b81f963 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -47,6 +47,17 @@ typedef struct mclient {
 	int			m_count;
 } nfs_client;
 
+/*
+ * Some versions of freeaddrinfo(3) do not tolerate being
+ * passed a NULL pointer.
+ */
+static inline void nfs_freeaddrinfo(struct addrinfo *ai)
+{
+	if (ai) {
+		freeaddrinfo(ai);
+	}
+}
+
 static inline const struct sockaddr *
 get_addrlist(const nfs_client *clp, const int i)
 {
diff --git a/support/nfs/getport.c b/support/nfs/getport.c
index 081594c..26ec85e 100644
--- a/support/nfs/getport.c
+++ b/support/nfs/getport.c
@@ -47,6 +47,7 @@
 
 #include "sockaddr.h"
 #include "nfsrpc.h"
+#include "exportfs.h"
 
 /*
  * Try a local socket first to access the local rpcbind daemon
@@ -109,7 +110,7 @@ static int nfs_gp_loopback_address(struct sockaddr *sap, socklen_t *salen)
 		ret = 1;
 	}
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 	return ret;
 }
 
@@ -134,8 +135,8 @@ static in_port_t nfs_gp_getservbyname(const char *service,
 
 	sin = (const struct sockaddr_in *)gai_results->ai_addr;
 	port = sin->sin_port;
-	
-	freeaddrinfo(gai_results);
+
+	nfs_freeaddrinfo(gai_results);
 	return port;
 }
 
diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index ef7ff05..d0b747b 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -39,6 +39,8 @@
 #include <rpc/rpc.h>
 #include <rpc/svc.h>
 
+#include "exportfs.h"
+
 #ifdef HAVE_TCP_WRAPPER
 #include "tcpwrapper.h"
 #endif
@@ -273,7 +275,7 @@ svc_create_nconf_rand_port(const char *name, const rpcprog_t program,
 	bindaddr.qlen = SOMAXCONN;
 
 	xprt = svc_tli_create(RPC_ANYFD, nconf, &bindaddr, 0, 0);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	if (xprt == NULL) {
 		xlog(L_ERROR, "Failed to create listener xprt "
 			"(%s, %u, %s)", name, version, nconf->nc_netid);
@@ -364,11 +366,11 @@ svc_create_nconf_fixed_port(const char *name, const rpcprog_t program,
 
 	svc_create_cache_xprt(xprt);
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return 1;
 
 out_free:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return 0;
 }
 
diff --git a/support/nfsidmap/umich_ldap.c b/support/nfsidmap/umich_ldap.c
index b661110..b8ee184 100644
--- a/support/nfsidmap/umich_ldap.c
+++ b/support/nfsidmap/umich_ldap.c
@@ -1089,7 +1089,7 @@ get_canonical_hostname(const char *inname)
 	return_name = strdup (tmphost);
 
 out_free:
-	freeaddrinfo(ap);
+	nfs_freeaddrinfo(ap);
 out_err:
 	return return_name;
 }
diff --git a/tests/nsm_client/nsm_client.c b/tests/nsm_client/nsm_client.c
index 0fa3422..8dc0591 100644
--- a/tests/nsm_client/nsm_client.c
+++ b/tests/nsm_client/nsm_client.c
@@ -243,7 +243,7 @@ nsm_client_get_rpcclient(const char *node)
 		printf("RPC client creation failed\n");
 	}
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return client;
 }
 
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index cd3c979..333eadc 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -282,7 +282,7 @@ exportfs_parsed(char *hname, char *path, char *options, int verbose)
 	validate_export(exp);
 
 out:
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 static int exportfs_generic(char *arg, char *options, int verbose)
@@ -395,7 +395,7 @@ unexportfs_parsed(char *hname, char *path, int verbose)
 	if (!success)
 		xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname, path);
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 static int unexportfs_generic(char *arg, int verbose)
@@ -588,7 +588,7 @@ address_list(const char *hostname)
 	if (ai != NULL) {
 		/* @hostname was a presentation address */
 		cname = host_canonname(ai->ai_addr);
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (cname != NULL)
 			goto out;
 	}
@@ -639,8 +639,8 @@ matchhostname(const char *hostname1, const char *hostname2)
 			}
 
 out:
-	freeaddrinfo(results1);
-	freeaddrinfo(results2);
+	nfs_freeaddrinfo(results1);
+	nfs_freeaddrinfo(results2);
 	return result;
 }
 
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 2e92f28..7eeb05f 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -172,14 +172,14 @@ gssd_addrstr_to_sockaddr(struct sockaddr *sa, const char *node, const char *port
 		if (sin6->sin6_scope_id) {
 			printerr(0, "ERROR: address %s has non-zero "
 				    "sin6_scope_id!\n", node);
-			freeaddrinfo(res);
+			nfs_freeaddrinfo(res);
 			return false;
 		}
 	}
 #endif /* IPV6_SUPPORTED */
 
 	memcpy(sa, res->ai_addr, res->ai_addrlen);
-	freeaddrinfo(res);
+	nfs_freeaddrinfo(res);
 	return true;
 }
 
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index eba1aac..adbde93 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -587,7 +587,7 @@ get_full_hostname(const char *inhost, char *outhost, int outhostlen)
 		goto out;
 	}
 	strncpy(outhost, addrs->ai_canonname, outhostlen);
-	freeaddrinfo(addrs);
+	nfs_freeaddrinfo(addrs);
 	for (c = outhost; *c != '\0'; c++)
 	    *c = tolower(*c);
 
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 356f663..fcb0b9f 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -53,6 +53,7 @@
 #include <net/if.h>
 #include <ifaddrs.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "xcommon.h"
 #include "mount.h"
@@ -250,7 +251,7 @@ int nfs_lookup(const char *hostname, const sa_family_t family,
 		break;
 	}
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 	return ret;
 }
 
@@ -307,7 +308,7 @@ int nfs_string_to_sockaddr(const char *address, struct sockaddr *sap,
 			}
 			break;
 		}
-		freeaddrinfo(gai_results);
+		nfs_freeaddrinfo(gai_results);
 	}
 
 	return ret;
@@ -1180,7 +1181,7 @@ static int nfs_ca_gai(const struct sockaddr *sap,
 	*buflen = gai_results->ai_addrlen;
 	memcpy(buf, gai_results->ai_addr, *buflen);
 
-	freeaddrinfo(gai_results);
+	nfs_freeaddrinfo(gai_results);
 
 	return 1;
 }
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 0a25b1f..b170552 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -35,6 +35,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "xcommon.h"
 #include "mount.h"
@@ -1268,7 +1269,7 @@ int nfsmount_string(const char *spec, const char *node, char *type,
 	} else
 		nfs_error(_("%s: internal option parsing error"), progname);
 
-	freeaddrinfo(mi.address);
+	nfs_freeaddrinfo(mi.address);
 	free(mi.hostname);
 	return retval;
 }
diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c
index 8299256..8f7cbd4 100644
--- a/utils/mountd/auth.c
+++ b/utils/mountd/auth.c
@@ -297,7 +297,7 @@ auth_authenticate(const char *what, const struct sockaddr *caller,
 			path, epath, error);
 	}
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return exp;
 }
 
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 7e8d403..2cb370f 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -113,7 +113,7 @@ static void auth_unix_ip(int f)
 		ai = client_resolve(tmp->ai_addr);
 		if (ai) {
 			client = client_compose(ai);
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 		}
 	}
 	bp = buf; blen = sizeof(buf);
@@ -133,7 +133,7 @@ static void auth_unix_ip(int f)
 	xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");
 
 	free(client);
-	freeaddrinfo(tmp);
+	nfs_freeaddrinfo(tmp);
 
 }
 
@@ -667,7 +667,7 @@ static struct addrinfo *lookup_client_addr(char *dom)
 	if (tmp == NULL)
 		return NULL;
 	ret = client_resolve(tmp->ai_addr);
-	freeaddrinfo(tmp);
+	nfs_freeaddrinfo(tmp);
 	return ret;
 }
 
@@ -834,7 +834,7 @@ static void nfsd_fh(int f)
 out:
 	if (found_path)
 		free(found_path);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	free(dom);
 	xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ? found->e_path : NULL);
 }
@@ -1355,7 +1355,7 @@ static void nfsd_export(int f)
 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
 	if (dom) free(dom);
 	if (path) free(path);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 }
 
 
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 086c39b..fb7bba4 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -578,10 +578,10 @@ static void prune_clients(nfs_export *exp, struct exportnode *e)
 				*cp = c->gr_next;
 				xfree(c->gr_name);
 				xfree(c);
-				freeaddrinfo(ai);
+				nfs_freeaddrinfo(ai);
 				continue;
 			}
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 		}
 		cp = &(c->gr_next);
 	}
diff --git a/utils/mountd/rmtab.c b/utils/mountd/rmtab.c
index 3ae0dbb..c896243 100644
--- a/utils/mountd/rmtab.c
+++ b/utils/mountd/rmtab.c
@@ -226,7 +226,7 @@ mountlist_list(void)
 				ai = host_pton(rep->r_client);
 				if (ai != NULL) {
 					m->ml_hostname = host_canonname(ai->ai_addr);
-					freeaddrinfo(ai);
+					nfs_freeaddrinfo(ai);
 				}
 			}
 			if (m->ml_hostname == NULL)
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 1e6ffd6..47b1882 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -25,6 +25,7 @@
 #include "nfslib.h"
 #include "xlog.h"
 #include "nfssvc.h"
+#include "exportfs.h"
 #include "../mount/version.h"
 
 #ifndef NFSD_FS_DIR
@@ -246,8 +247,7 @@ error:
 		close(fd);
 	if (sockfd >= 0)
 		close(sockfd);
-	if (addrhead)
-		freeaddrinfo(addrhead);
+	nfs_freeaddrinfo(addrhead);
 	return (bounded ? 0 : rc);
 }
 
diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
index 8cccdb8..c9e22d3 100644
--- a/utils/statd/hostname.c
+++ b/utils/statd/hostname.c
@@ -35,6 +35,7 @@
 #include <netdb.h>
 #include <arpa/inet.h>
 
+#include "exportfs.h"
 #include "sockaddr.h"
 #include "statd.h"
 #include "xlog.h"
@@ -203,7 +204,7 @@ statd_canonical_name(const char *hostname)
 		_Bool result;
 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
 					buf, (socklen_t)sizeof(buf));
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (!result || buf[0] == '\0')
 			/* OK to use presentation address,
 			 * if no reverse map exists */
@@ -217,7 +218,7 @@ statd_canonical_name(const char *hostname)
 	if (ai == NULL)
 		return NULL;
 	strcpy(buf, ai->ai_canonname);
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 
 	return strdup(buf);
 }
@@ -253,7 +254,7 @@ statd_canonical_list(const char *hostname)
 		_Bool result;
 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
 					buf, (socklen_t)sizeof(buf));
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (result)
 			goto out;
 	}
@@ -308,8 +309,8 @@ statd_matchhostname(const char *hostname1, const char *hostname2)
 			}
 
 out:
-	freeaddrinfo(results2);
-	freeaddrinfo(results1);
+	nfs_freeaddrinfo(results2);
+	nfs_freeaddrinfo(results1);
 
 	xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
 			hostname1, hostname2,
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 29dad38..05d72a3 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -37,6 +37,7 @@
 #include "xlog.h"
 #include "nsm.h"
 #include "nfsrpc.h"
+#include "exportfs.h"
 
 /* glibc before 2.3.4 */
 #ifndef AI_NUMERICSERV
@@ -179,7 +180,7 @@ smn_verify_my_name(const char *name)
 	case 0:
 		/* @name was a presentation address */
 		retval = smn_get_hostname(ai->ai_addr, ai->ai_addrlen, name);
-		freeaddrinfo(ai);
+		nfs_freeaddrinfo(ai);
 		if (retval == NULL)
 			return NULL;
 		break;
@@ -253,8 +254,7 @@ static void smn_forget_host(struct nsm_host *host)
 	free((void *)host->my_name);
 	free((void *)host->mon_name);
 	free(host->name);
-	if (host->ai)
-		freeaddrinfo(host->ai);
+	nfs_freeaddrinfo(host->ai);
 
 	free(host);
 }
@@ -430,7 +430,7 @@ retry:
 	if (srcport) {
 		if (bind(sock, ai->ai_addr, ai->ai_addrlen) == -1) {
 			xlog(L_ERROR, "Failed to bind RPC socket: %m");
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			return -1;
 		}
@@ -440,7 +440,7 @@ retry:
 		if (smn_bindresvport(sock, ai->ai_addr) == -1) {
 			xlog(L_ERROR,
 				"bindresvport on RPC socket failed: %m");
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			return -1;
 		}
@@ -449,13 +449,13 @@ retry:
 		se = getservbyport((int)nfs_get_port(ai->ai_addr), "udp");
 		if (se != NULL && retry_cnt < 100) {
 			retry_cnt++;
-			freeaddrinfo(ai);
+			nfs_freeaddrinfo(ai);
 			(void)close(sock);
 			goto retry;
 		}
 	}
 
-	freeaddrinfo(ai);
+	nfs_freeaddrinfo(ai);
 	return sock;
 }
 
-- 
2.20.1

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Chuck Lever]

> On Feb 19, 2019, at 4:03 AM, Peter Wagner <tripolar@gmx.at> wrote:
> 
> Hi,
> 
> i updated to code to the latest version
> 
> From dc2c02c12cf66e7ac4ab12fad7ea21f83f10ee7a Mon Sep 17 00:00:00 2001
> From: Peter Wagner <tripolar@gmx.at>
> Date: Tue, 19 Feb 2019 09:53:07 +0100
> Subject: [PATCH] define and use wrapper function nfs_freeaddrinfo to handle
> freeaddrinfo versions that don't tolerate NULL pointers
> 
> set AI_CANONNAME flag in host_pton to set the ip address of the connected
> host to ai_canonname
> 
> Signed-off-by: Peter Wagner <tripolar@gmx.at>
> ---
> support/export/client.c       |  6 +++---
> support/export/hostname.c     | 28 +++-------------------------
> support/include/exportfs.h    | 11 +++++++++++
> support/nfs/getport.c         |  7 ++++---
> support/nfs/svc_create.c      |  8 +++++---
> support/nfsidmap/umich_ldap.c |  2 +-
> tests/nsm_client/nsm_client.c |  2 +-
> utils/exportfs/exportfs.c     | 10 +++++-----
> utils/gssd/gssd.c             |  4 ++--
> utils/gssd/krb5_util.c        |  2 +-
> utils/mount/network.c         |  7 ++++---
> utils/mount/stropts.c         |  3 ++-
> utils/mountd/auth.c           |  2 +-
> utils/mountd/cache.c          | 10 +++++-----
> utils/mountd/mountd.c         |  4 ++--
> utils/mountd/rmtab.c          |  2 +-
> utils/nfsd/nfssvc.c           |  4 ++--
> utils/statd/hostname.c        | 11 ++++++-----
> utils/statd/sm-notify.c       | 14 +++++++-------
> 19 files changed, 66 insertions(+), 71 deletions(-)
> 
> diff --git a/support/export/client.c b/support/export/client.c
> index baf59c8..a1fba01 100644
> --- a/support/export/client.c
> +++ b/support/export/client.c
> @@ -210,7 +210,7 @@ init_subnetwork(nfs_client *clp)
> 	set_addrlist(clp, 0, ai->ai_addr);
> 	family = ai->ai_addr->sa_family;
> 
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 
> 	switch (family) {
> 	case AF_INET:
> @@ -309,7 +309,7 @@ client_lookup(char *hname, int canonical)
> 		init_addrlist(clp, ai);
> 
> out:
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	return clp;
> }
> 
> @@ -674,7 +674,7 @@ check_netgroup(const nfs_client *clp, const struct addrinfo *ai)
> 	tmp = host_pton(hname);
> 	if (tmp != NULL) {
> 		char *cname = host_canonname(tmp->ai_addr);
> -		freeaddrinfo(tmp);
> +		nfs_freeaddrinfo(tmp);
> 
> 		/* The resulting FQDN may be in our netgroup. */
> 		if (cname != NULL) {
> diff --git a/support/export/hostname.c b/support/export/hostname.c
> index 5c4c824..9845a59 100644
> --- a/support/export/hostname.c
> +++ b/support/export/hostname.c
> @@ -101,6 +101,7 @@ host_pton(const char *paddr)
> 		.ai_protocol	= (int)IPPROTO_UDP,
> 		.ai_flags	= AI_NUMERICHOST,
> 		.ai_family	= AF_UNSPEC,
> +		.ai_flags	= AI_CANONNAME,

Now I wonder if this will force getaddrinfo(3) to do an on-the-wire
DNS query? The point of host_pton() and host_numeric_addrinfo() is
to avoid any on-the-wire activity.

In any event, please put fixes like this in a separate patch.

Also, what do you plan to do about host_reliable_addrinfo, which
also has a "free(ai->ai_canonname);" call site? In that case, we
are not trying to avoid DNS queries, which could make it easier
to fix (in a separate patch, of course).


> 	};
> 	struct sockaddr_in sin;
> 	int error, inet4;
> @@ -130,7 +131,7 @@ host_pton(const char *paddr)
> 		if (!inet4 && ai->ai_addr->sa_family == AF_INET) {
> 			xlog(D_GENERAL, "%s: failed to convert %s",
> 					__func__, paddr);
> -			freeaddrinfo(ai);
> +			nfs_freeaddrinfo(ai);
> 			break;
> 		}
> 		return ai;
> @@ -290,7 +291,7 @@ host_reliable_addrinfo(const struct sockaddr *sap)
> 		if (nfs_compare_sockaddr(a->ai_addr, sap))
> 			break;
> 
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	if (!a)
> 		goto out_free_hostname;
> 
> @@ -350,18 +351,6 @@ host_numeric_addrinfo(const struct sockaddr *sap)
> 
> 	ai = host_pton(buf);
> 
> -	/*
> -	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
> -	 */
> -	if (ai != NULL) {
> -		free(ai->ai_canonname);		/* just in case */
> -		ai->ai_canonname = strdup(buf);
> -		if (ai->ai_canonname == NULL) {
> -			freeaddrinfo(ai);
> -			ai = NULL;
> -		}
> -	}
> -
> 	return ai;
> }
> #else	/* !HAVE_GETNAMEINFO */
> @@ -384,17 +373,6 @@ host_numeric_addrinfo(const struct sockaddr *sap)
> 
> 	ai = host_pton(buf);
> 
> -	/*
> -	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
> -	 */
> -	if (ai != NULL) {
> -		ai->ai_canonname = strdup(buf);
> -		if (ai->ai_canonname == NULL) {
> -			freeaddrinfo(ai);
> -			ai = NULL;
> -		}
> -	}
> -
> 	return ai;
> }
> #endif	/* !HAVE_GETNAMEINFO */
> diff --git a/support/include/exportfs.h b/support/include/exportfs.h
> index 4e0d9d1..b81f963 100644
> --- a/support/include/exportfs.h
> +++ b/support/include/exportfs.h
> @@ -47,6 +47,17 @@ typedef struct mclient {
> 	int			m_count;
> } nfs_client;
> 
> +/*
> + * Some versions of freeaddrinfo(3) do not tolerate being
> + * passed a NULL pointer.
> + */
> +static inline void nfs_freeaddrinfo(struct addrinfo *ai)
> +{
> +	if (ai) {
> +		freeaddrinfo(ai);
> +	}
> +}
> +
> static inline const struct sockaddr *
> get_addrlist(const nfs_client *clp, const int i)
> {
> diff --git a/support/nfs/getport.c b/support/nfs/getport.c
> index 081594c..26ec85e 100644
> --- a/support/nfs/getport.c
> +++ b/support/nfs/getport.c
> @@ -47,6 +47,7 @@
> 
> #include "sockaddr.h"
> #include "nfsrpc.h"
> +#include "exportfs.h"
> 
> /*
>  * Try a local socket first to access the local rpcbind daemon
> @@ -109,7 +110,7 @@ static int nfs_gp_loopback_address(struct sockaddr *sap, socklen_t *salen)
> 		ret = 1;
> 	}
> 
> -	freeaddrinfo(gai_results);
> +	nfs_freeaddrinfo(gai_results);
> 	return ret;
> }
> 
> @@ -134,8 +135,8 @@ static in_port_t nfs_gp_getservbyname(const char *service,
> 
> 	sin = (const struct sockaddr_in *)gai_results->ai_addr;
> 	port = sin->sin_port;
> -	
> -	freeaddrinfo(gai_results);
> +
> +	nfs_freeaddrinfo(gai_results);
> 	return port;
> }
> 
> diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
> index ef7ff05..d0b747b 100644
> --- a/support/nfs/svc_create.c
> +++ b/support/nfs/svc_create.c
> @@ -39,6 +39,8 @@
> #include <rpc/rpc.h>
> #include <rpc/svc.h>
> 
> +#include "exportfs.h"
> +
> #ifdef HAVE_TCP_WRAPPER
> #include "tcpwrapper.h"
> #endif
> @@ -273,7 +275,7 @@ svc_create_nconf_rand_port(const char *name, const rpcprog_t program,
> 	bindaddr.qlen = SOMAXCONN;
> 
> 	xprt = svc_tli_create(RPC_ANYFD, nconf, &bindaddr, 0, 0);
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	if (xprt == NULL) {
> 		xlog(L_ERROR, "Failed to create listener xprt "
> 			"(%s, %u, %s)", name, version, nconf->nc_netid);
> @@ -364,11 +366,11 @@ svc_create_nconf_fixed_port(const char *name, const rpcprog_t program,
> 
> 	svc_create_cache_xprt(xprt);
> 
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	return 1;
> 
> out_free:
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	return 0;
> }
> 
> diff --git a/support/nfsidmap/umich_ldap.c b/support/nfsidmap/umich_ldap.c
> index b661110..b8ee184 100644
> --- a/support/nfsidmap/umich_ldap.c
> +++ b/support/nfsidmap/umich_ldap.c
> @@ -1089,7 +1089,7 @@ get_canonical_hostname(const char *inname)
> 	return_name = strdup (tmphost);
> 
> out_free:
> -	freeaddrinfo(ap);
> +	nfs_freeaddrinfo(ap);
> out_err:
> 	return return_name;
> }
> diff --git a/tests/nsm_client/nsm_client.c b/tests/nsm_client/nsm_client.c
> index 0fa3422..8dc0591 100644
> --- a/tests/nsm_client/nsm_client.c
> +++ b/tests/nsm_client/nsm_client.c
> @@ -243,7 +243,7 @@ nsm_client_get_rpcclient(const char *node)
> 		printf("RPC client creation failed\n");
> 	}
> out:
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	return client;
> }
> 
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index cd3c979..333eadc 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -282,7 +282,7 @@ exportfs_parsed(char *hname, char *path, char *options, int verbose)
> 	validate_export(exp);
> 
> out:
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> }
> 
> static int exportfs_generic(char *arg, char *options, int verbose)
> @@ -395,7 +395,7 @@ unexportfs_parsed(char *hname, char *path, int verbose)
> 	if (!success)
> 		xlog(L_ERROR, "Could not find '%s:%s' to unexport.", hname, path);
> 
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> }
> 
> static int unexportfs_generic(char *arg, int verbose)
> @@ -588,7 +588,7 @@ address_list(const char *hostname)
> 	if (ai != NULL) {
> 		/* @hostname was a presentation address */
> 		cname = host_canonname(ai->ai_addr);
> -		freeaddrinfo(ai);
> +		nfs_freeaddrinfo(ai);
> 		if (cname != NULL)
> 			goto out;
> 	}
> @@ -639,8 +639,8 @@ matchhostname(const char *hostname1, const char *hostname2)
> 			}
> 
> out:
> -	freeaddrinfo(results1);
> -	freeaddrinfo(results2);
> +	nfs_freeaddrinfo(results1);
> +	nfs_freeaddrinfo(results2);
> 	return result;
> }
> 
> diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
> index 2e92f28..7eeb05f 100644
> --- a/utils/gssd/gssd.c
> +++ b/utils/gssd/gssd.c
> @@ -172,14 +172,14 @@ gssd_addrstr_to_sockaddr(struct sockaddr *sa, const char *node, const char *port
> 		if (sin6->sin6_scope_id) {
> 			printerr(0, "ERROR: address %s has non-zero "
> 				    "sin6_scope_id!\n", node);
> -			freeaddrinfo(res);
> +			nfs_freeaddrinfo(res);
> 			return false;
> 		}
> 	}
> #endif /* IPV6_SUPPORTED */
> 
> 	memcpy(sa, res->ai_addr, res->ai_addrlen);
> -	freeaddrinfo(res);
> +	nfs_freeaddrinfo(res);
> 	return true;
> }
> 
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index eba1aac..adbde93 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -587,7 +587,7 @@ get_full_hostname(const char *inhost, char *outhost, int outhostlen)
> 		goto out;
> 	}
> 	strncpy(outhost, addrs->ai_canonname, outhostlen);
> -	freeaddrinfo(addrs);
> +	nfs_freeaddrinfo(addrs);
> 	for (c = outhost; *c != '\0'; c++)
> 	    *c = tolower(*c);
> 
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 356f663..fcb0b9f 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -53,6 +53,7 @@
> #include <net/if.h>
> #include <ifaddrs.h>
> 
> +#include "exportfs.h"
> #include "sockaddr.h"
> #include "xcommon.h"
> #include "mount.h"
> @@ -250,7 +251,7 @@ int nfs_lookup(const char *hostname, const sa_family_t family,
> 		break;
> 	}
> 
> -	freeaddrinfo(gai_results);
> +	nfs_freeaddrinfo(gai_results);
> 	return ret;
> }
> 
> @@ -307,7 +308,7 @@ int nfs_string_to_sockaddr(const char *address, struct sockaddr *sap,
> 			}
> 			break;
> 		}
> -		freeaddrinfo(gai_results);
> +		nfs_freeaddrinfo(gai_results);
> 	}
> 
> 	return ret;
> @@ -1180,7 +1181,7 @@ static int nfs_ca_gai(const struct sockaddr *sap,
> 	*buflen = gai_results->ai_addrlen;
> 	memcpy(buf, gai_results->ai_addr, *buflen);
> 
> -	freeaddrinfo(gai_results);
> +	nfs_freeaddrinfo(gai_results);
> 
> 	return 1;
> }
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 0a25b1f..b170552 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -35,6 +35,7 @@
> #include <netinet/in.h>
> #include <arpa/inet.h>
> 
> +#include "exportfs.h"
> #include "sockaddr.h"
> #include "xcommon.h"
> #include "mount.h"
> @@ -1268,7 +1269,7 @@ int nfsmount_string(const char *spec, const char *node, char *type,
> 	} else
> 		nfs_error(_("%s: internal option parsing error"), progname);
> 
> -	freeaddrinfo(mi.address);
> +	nfs_freeaddrinfo(mi.address);
> 	free(mi.hostname);
> 	return retval;
> }
> diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c
> index 8299256..8f7cbd4 100644
> --- a/utils/mountd/auth.c
> +++ b/utils/mountd/auth.c
> @@ -297,7 +297,7 @@ auth_authenticate(const char *what, const struct sockaddr *caller,
> 			path, epath, error);
> 	}
> 
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	return exp;
> }
> 
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 7e8d403..2cb370f 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -113,7 +113,7 @@ static void auth_unix_ip(int f)
> 		ai = client_resolve(tmp->ai_addr);
> 		if (ai) {
> 			client = client_compose(ai);
> -			freeaddrinfo(ai);
> +			nfs_freeaddrinfo(ai);
> 		}
> 	}
> 	bp = buf; blen = sizeof(buf);
> @@ -133,7 +133,7 @@ static void auth_unix_ip(int f)
> 	xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");
> 
> 	free(client);
> -	freeaddrinfo(tmp);
> +	nfs_freeaddrinfo(tmp);
> 
> }
> 
> @@ -667,7 +667,7 @@ static struct addrinfo *lookup_client_addr(char *dom)
> 	if (tmp == NULL)
> 		return NULL;
> 	ret = client_resolve(tmp->ai_addr);
> -	freeaddrinfo(tmp);
> +	nfs_freeaddrinfo(tmp);
> 	return ret;
> }
> 
> @@ -834,7 +834,7 @@ static void nfsd_fh(int f)
> out:
> 	if (found_path)
> 		free(found_path);
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	free(dom);
> 	xlog(D_CALL, "nfsd_fh: found %p path %s", found, found ? found->e_path : NULL);
> }
> @@ -1355,7 +1355,7 @@ static void nfsd_export(int f)
> 	xlog(D_CALL, "nfsd_export: found %p path %s", found, path ? path : NULL);
> 	if (dom) free(dom);
> 	if (path) free(path);
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> }
> 
> 
> diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
> index 086c39b..fb7bba4 100644
> --- a/utils/mountd/mountd.c
> +++ b/utils/mountd/mountd.c
> @@ -578,10 +578,10 @@ static void prune_clients(nfs_export *exp, struct exportnode *e)
> 				*cp = c->gr_next;
> 				xfree(c->gr_name);
> 				xfree(c);
> -				freeaddrinfo(ai);
> +				nfs_freeaddrinfo(ai);
> 				continue;
> 			}
> -			freeaddrinfo(ai);
> +			nfs_freeaddrinfo(ai);
> 		}
> 		cp = &(c->gr_next);
> 	}
> diff --git a/utils/mountd/rmtab.c b/utils/mountd/rmtab.c
> index 3ae0dbb..c896243 100644
> --- a/utils/mountd/rmtab.c
> +++ b/utils/mountd/rmtab.c
> @@ -226,7 +226,7 @@ mountlist_list(void)
> 				ai = host_pton(rep->r_client);
> 				if (ai != NULL) {
> 					m->ml_hostname = host_canonname(ai->ai_addr);
> -					freeaddrinfo(ai);
> +					nfs_freeaddrinfo(ai);
> 				}
> 			}
> 			if (m->ml_hostname == NULL)
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 1e6ffd6..47b1882 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -25,6 +25,7 @@
> #include "nfslib.h"
> #include "xlog.h"
> #include "nfssvc.h"
> +#include "exportfs.h"
> #include "../mount/version.h"
> 
> #ifndef NFSD_FS_DIR
> @@ -246,8 +247,7 @@ error:
> 		close(fd);
> 	if (sockfd >= 0)
> 		close(sockfd);
> -	if (addrhead)
> -		freeaddrinfo(addrhead);
> +	nfs_freeaddrinfo(addrhead);
> 	return (bounded ? 0 : rc);
> }
> 
> diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
> index 8cccdb8..c9e22d3 100644
> --- a/utils/statd/hostname.c
> +++ b/utils/statd/hostname.c
> @@ -35,6 +35,7 @@
> #include <netdb.h>
> #include <arpa/inet.h>
> 
> +#include "exportfs.h"
> #include "sockaddr.h"
> #include "statd.h"
> #include "xlog.h"
> @@ -203,7 +204,7 @@ statd_canonical_name(const char *hostname)
> 		_Bool result;
> 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
> 					buf, (socklen_t)sizeof(buf));
> -		freeaddrinfo(ai);
> +		nfs_freeaddrinfo(ai);
> 		if (!result || buf[0] == '\0')
> 			/* OK to use presentation address,
> 			 * if no reverse map exists */
> @@ -217,7 +218,7 @@ statd_canonical_name(const char *hostname)
> 	if (ai == NULL)
> 		return NULL;
> 	strcpy(buf, ai->ai_canonname);
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 
> 	return strdup(buf);
> }
> @@ -253,7 +254,7 @@ statd_canonical_list(const char *hostname)
> 		_Bool result;
> 		result = get_nameinfo(ai->ai_addr, ai->ai_addrlen,
> 					buf, (socklen_t)sizeof(buf));
> -		freeaddrinfo(ai);
> +		nfs_freeaddrinfo(ai);
> 		if (result)
> 			goto out;
> 	}
> @@ -308,8 +309,8 @@ statd_matchhostname(const char *hostname1, const char *hostname2)
> 			}
> 
> out:
> -	freeaddrinfo(results2);
> -	freeaddrinfo(results1);
> +	nfs_freeaddrinfo(results2);
> +	nfs_freeaddrinfo(results1);
> 
> 	xlog(D_CALL, "%s: hostnames %s and %s %s", __func__,
> 			hostname1, hostname2,
> diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
> index 29dad38..05d72a3 100644
> --- a/utils/statd/sm-notify.c
> +++ b/utils/statd/sm-notify.c
> @@ -37,6 +37,7 @@
> #include "xlog.h"
> #include "nsm.h"
> #include "nfsrpc.h"
> +#include "exportfs.h"
> 
> /* glibc before 2.3.4 */
> #ifndef AI_NUMERICSERV
> @@ -179,7 +180,7 @@ smn_verify_my_name(const char *name)
> 	case 0:
> 		/* @name was a presentation address */
> 		retval = smn_get_hostname(ai->ai_addr, ai->ai_addrlen, name);
> -		freeaddrinfo(ai);
> +		nfs_freeaddrinfo(ai);
> 		if (retval == NULL)
> 			return NULL;
> 		break;
> @@ -253,8 +254,7 @@ static void smn_forget_host(struct nsm_host *host)
> 	free((void *)host->my_name);
> 	free((void *)host->mon_name);
> 	free(host->name);
> -	if (host->ai)
> -		freeaddrinfo(host->ai);
> +	nfs_freeaddrinfo(host->ai);
> 
> 	free(host);
> }
> @@ -430,7 +430,7 @@ retry:
> 	if (srcport) {
> 		if (bind(sock, ai->ai_addr, ai->ai_addrlen) == -1) {
> 			xlog(L_ERROR, "Failed to bind RPC socket: %m");
> -			freeaddrinfo(ai);
> +			nfs_freeaddrinfo(ai);
> 			(void)close(sock);
> 			return -1;
> 		}
> @@ -440,7 +440,7 @@ retry:
> 		if (smn_bindresvport(sock, ai->ai_addr) == -1) {
> 			xlog(L_ERROR,
> 				"bindresvport on RPC socket failed: %m");
> -			freeaddrinfo(ai);
> +			nfs_freeaddrinfo(ai);
> 			(void)close(sock);
> 			return -1;
> 		}
> @@ -449,13 +449,13 @@ retry:
> 		se = getservbyport((int)nfs_get_port(ai->ai_addr), "udp");
> 		if (se != NULL && retry_cnt < 100) {
> 			retry_cnt++;
> -			freeaddrinfo(ai);
> +			nfs_freeaddrinfo(ai);
> 			(void)close(sock);
> 			goto retry;
> 		}
> 	}
> 
> -	freeaddrinfo(ai);
> +	nfs_freeaddrinfo(ai);
> 	return sock;
> }
> 
> -- 
> 2.20.1
> 

--
Chuck Lever

Re: [PATCH] nfs-utils: fix addrinfo usage with musl-1.1.21

[Steve Dickson]

Hey Peter,

On 2/19/19 4:03 AM, Peter Wagner wrote:
> i updated to code to the latest version

Would you please create a new thread with 
each patch version adding the version to
the subject line ex [PATCH V2].

It just make it much easier for me to
find the patch... tia,

steved.