Re: [PATCH v2 11/11] exportfs: only do glibc specific hackery on glibc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 12 Aug 2014 07:03:54 -0400
Steve Dickson <SteveD@xxxxxxxxxx> wrote:

> 
> 
> On 08/06/2014 02:25 AM, Natanael Copa wrote:
> > We should not depend on the libc do free(3) on ai_canonname as that is
> > completely up to implementation and known o break things on uclibc and
> > musl libc.
> > 
> > Signed-off-by: Natanael Copa <ncopa@xxxxxxxxxxxxxxx>
> > ---
> >  support/export/hostname.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/support/export/hostname.c b/support/export/hostname.c
> > index d9153e1..30584b4 100644
> > --- a/support/export/hostname.c
> > +++ b/support/export/hostname.c
> > @@ -382,6 +382,7 @@ host_numeric_addrinfo(const struct sockaddr *sap)
> >  
> >  	ai = host_pton(buf);
> >  
> > +#if !definded(__UCLIBC__) && defined(__GLIBC__)
>         ^^^^^^^^
> Are you kidding me???? How are you testing these patches? 

my bad sorry.

> 
> I'm all for this port but I'm a bit concern about the 
> stability since things like this don't even compile... 

I have compile tested them and run tested them on musl libc. Without
those patches code does not compile with musl libc and without some of
the patches the application segfaults with musl libc (due to the way
basename(3) is used)

Patch 11 is supposed to fix a segfault due to nfs-utils expects
getaddrinfo(3)/freeaddrinfo(3) to malloc/free the ai_canonname field,
but i messed up in the countless rebases I have done. Patch 11/11 is bad
and can be discarded.

The problem is still there though and I believe it leads to a memleak
if you ai->ai_canonname = strdup(buf), depending on the
getaddrinfo/freeaddrinfo implementation.

I know for sure that this line used to case a segfault on uclibc:
                free(ai->ai_canonname);         /* just in case */

I think the proper way to fix it requires some refactoring so I think
we can do that separately.

The other 10 patches should be good though.

Note that even with those patches nfs-utils does not actually work with
musl libc due to the use of FILE* IO to access of /proc/net/rpc. It
does work with Timos "rework access to /proc/net/rpc" patch (but I
believe it still leaks memory)


> steved.
>  
> >  	/*
> >  	 * getaddrinfo(AI_NUMERICHOST) never fills in ai_canonname
> >  	 */
> > @@ -392,7 +393,9 @@ host_numeric_addrinfo(const struct sockaddr *sap)
> >  			ai = NULL;
> >  		}
> >  	}
> > +#endif
> >  
> >  	return ai;
> >  }
> > +
> >  #endif	/* !HAVE_GETNAMEINFO */
> > 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux