2014-08-06 06:25:16

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 00/11] Porting nfs-utils to musl libc

Musl libc is a modern libc for Linux which focuses on correctness in
standards-conformance. We use this libc for Alpine Linux.

This patchset makes nfs-utils build with musl libc and should generally
improve portability.

It would be nice if at least some of the patches could be applied
upstream so we can reduce the number of patches for getting nfs working
on Alpine Linux.

Changes v1 -> v2 based on feedback:
- Use AC_USE_SYSTEM_EXTENSIONS in configure.ac instead of defining
_GNU_SOURCE various places. This means that the patch "include
libgen.h for basename" is strictly no longer needed, but I kept it
since this is more "correct".
- Fix typo in "exportfs: only do glibc specific hackery on glibc"

Natanael Copa (11):
conffile: use standard uint*_t and unsigned char
Fix header include for definition of NULL
configure.ac: enable GNU_SOURCE for stat64/statfs64
replace __attribute_malloc__ with the more portable
__attribute__((__malloc__))
mountd: use standard dev_t instead of glibc internals
nfsstat: replace the legacy SA_ONESHOT with standard SA_RESETHAND
Allow usage of getrpcbynumber() when getrpcbynumber_r() is unavailable
Only work around glibc bugs on glibc
include libgen.h for basename
exportfs: fix test of NULL pointer in host_pton()
exportfs: only do glibc specific hackery on glibc

configure.ac | 7 ++-----
support/export/hostname.c | 29 +++++++++++++++++------------
support/include/conffile.h | 2 +-
support/include/exportfs.h | 10 +++++-----
support/include/sockaddr.h | 1 +
support/nfs/conffile.c | 14 +++++++-------
support/nfs/svc_create.c | 2 +-
support/nfs/svc_socket.c | 6 ++++++
tools/rpcdebug/rpcdebug.c | 1 +
utils/mount/mount.c | 1 +
utils/mount/mount_libmount.c | 1 +
utils/mountd/cache.c | 2 +-
utils/mountd/svc_run.c | 2 +-
utils/nfsd/nfsd.c | 1 +
utils/nfsidmap/nfsidmap.c | 1 +
utils/nfsstat/nfsstat.c | 2 +-
utils/statd/hostname.c | 6 +++---
utils/statd/sm-notify.c | 8 ++++----
utils/statd/statd.h | 2 +-
19 files changed, 56 insertions(+), 42 deletions(-)

--
2.0.4



2014-08-06 06:25:21

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 07/11] Allow usage of getrpcbynumber() when getrpcbynumber_r() is unavailable

Signed-off-by: Natanael Copa <[email protected]>
---
configure.ac | 6 +-----
support/nfs/svc_socket.c | 6 ++++++
2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index b9682ed..bc48373 100644
--- a/configure.ac
+++ b/configure.ac
@@ -248,9 +248,6 @@ AC_CHECK_FUNC([connect], ,
AC_CHECK_FUNC([getaddrinfo], ,
[AC_MSG_ERROR([Function 'getaddrinfo' not found.])])

-AC_CHECK_FUNC([getrpcbynumber], ,
- [AC_MSG_ERROR([Function 'getrpcbynumber' not found.])])
-
AC_CHECK_FUNC([getservbyname], ,
[AC_MSG_ERROR([Function 'getservbyname' not found.])])

@@ -409,12 +406,11 @@ AC_FUNC_STAT
AC_FUNC_VPRINTF
AC_CHECK_FUNCS([alarm atexit dup2 fdatasync ftruncate getcwd \
gethostbyaddr gethostbyname gethostname getmntent \
- getnameinfo getrpcbyname getifaddrs \
+ getnameinfo getrpcbyname getrpcbynumber getrpcbynumber_r getifaddrs \
gettimeofday hasmntopt inet_ntoa innetgr memset mkdir pathconf \
ppoll realpath rmdir select socket strcasecmp strchr strdup \
strerror strrchr strtol strtoul sigprocmask name_to_handle_at])

-
dnl *************************************************************
dnl Check for data sizes
dnl *************************************************************
diff --git a/support/nfs/svc_socket.c b/support/nfs/svc_socket.c
index f56f310..61ccf5b 100644
--- a/support/nfs/svc_socket.c
+++ b/support/nfs/svc_socket.c
@@ -42,8 +42,14 @@ int getservport(u_long number, const char *proto)
struct servent servbuf, *servp = NULL;
int ret;

+#if HAVE_GETRPCBYNUMBER_R
ret = getrpcbynumber_r(number, &rpcbuf, rpcdata, sizeof rpcdata,
&rpcp);
+#else
+ rpcp = getrpcbynumber(number);
+ ret = 0;
+#endif
+
if (ret == 0 && rpcp != NULL) {
/* First try name. */
ret = getservbyname_r(rpcp->r_name, proto, &servbuf, servdata,
--
2.0.4


2014-08-06 06:25:16

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 01/11] conffile: use standard uint*_t and unsigned char

Use the standard integer types. This fixes compiling errors with musl
libc.

Signed-off-by: Natanael Copa <[email protected]>
---
support/include/conffile.h | 2 +-
support/nfs/conffile.c | 14 +++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/support/include/conffile.h b/support/include/conffile.h
index 05ea5d2..94fb005 100644
--- a/support/include/conffile.h
+++ b/support/include/conffile.h
@@ -49,7 +49,7 @@ struct conf_list {
extern char *conf_path;

extern int conf_begin(void);
-extern int conf_decode_base64(u_int8_t *, u_int32_t *, u_char *);
+extern int conf_decode_base64(uint8_t *, uint32_t *, unsigned char *);
extern int conf_end(int, int);
extern void conf_free_list(struct conf_list *);
extern struct sockaddr *conf_get_address(char *, char *);
diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index c3434d5..6b94ec0 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -72,10 +72,10 @@ TAILQ_HEAD (conf_trans_head, conf_trans) conf_trans_queue;
/*
* Radix-64 Encoding.
*/
-static const u_int8_t bin2asc[]
+static const uint8_t bin2asc[]
= "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";

-static const u_int8_t asc2bin[] =
+static const uint8_t asc2bin[] =
{
255, 255, 255, 255, 255, 255, 255, 255,
255, 255, 255, 255, 255, 255, 255, 255,
@@ -109,10 +109,10 @@ LIST_HEAD (conf_bindings, conf_binding) conf_bindings[256];

static char *conf_addr;

-static __inline__ u_int8_t
+static __inline__ uint8_t
conf_hash(char *s)
{
- u_int8_t hash = 0;
+ uint8_t hash = 0;

while (*s) {
hash = ((hash << 1) | (hash >> 7)) ^ tolower (*s);
@@ -603,10 +603,10 @@ cleanup:

/* Decode a PEM encoded buffer. */
int
-conf_decode_base64 (u_int8_t *out, u_int32_t *len, u_char *buf)
+conf_decode_base64 (uint8_t *out, uint32_t *len, unsigned char *buf)
{
- u_int32_t c = 0;
- u_int8_t c1, c2, c3, c4;
+ uint32_t c = 0;
+ uint8_t c1, c2, c3, c4;

while (*buf) {
if (*buf > 127 || (c1 = asc2bin[*buf]) == 255)
--
2.0.4


2014-08-12 11:04:06

by Steve Dickson

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



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 <[email protected]>
> ---
> 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?

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

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 */
>

2014-08-06 06:25:22

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 09/11] include libgen.h for basename

According POSIX basename(3) should have an #include <libgen.h>

There are a different GNU implementation too, that can be used with
_GNU_SOURCE, but the POSIX version is good enough and more portable.

Signed-off-by: Natanael Copa <[email protected]>
---
tools/rpcdebug/rpcdebug.c | 1 +
utils/mount/mount.c | 1 +
utils/mount/mount_libmount.c | 1 +
utils/nfsd/nfsd.c | 1 +
utils/nfsidmap/nfsidmap.c | 1 +
5 files changed, 5 insertions(+)

diff --git a/tools/rpcdebug/rpcdebug.c b/tools/rpcdebug/rpcdebug.c
index d6e10d3..18b1622 100644
--- a/tools/rpcdebug/rpcdebug.c
+++ b/tools/rpcdebug/rpcdebug.c
@@ -26,6 +26,7 @@
#include <malloc.h>
#include <fcntl.h>
#include <ctype.h>
+#include <libgen.h>
/* RPC debug flags
#include <sunrpc/debug.h> */
/* NFS debug flags
diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index eea00af..91f1087 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -33,6 +33,7 @@
#include <getopt.h>
#include <mntent.h>
#include <pwd.h>
+#include <libgen.h>

#include "fstab.h"
#include "xcommon.h"
diff --git a/utils/mount/mount_libmount.c b/utils/mount/mount_libmount.c
index 701d41e..6f85dc9 100644
--- a/utils/mount/mount_libmount.c
+++ b/utils/mount/mount_libmount.c
@@ -29,6 +29,7 @@
#include <string.h>
#include <errno.h>
#include <getopt.h>
+#include <libgen.h>

#include <libmount/libmount.h>

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 03e3c81..201bb13 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -19,6 +19,7 @@
#include <errno.h>
#include <getopt.h>
#include <netdb.h>
+#include <libgen.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 3f51b4d..e0d31e7 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -4,6 +4,7 @@
#include <stdlib.h>
#include <string.h>
#include <errno.h>
+#include <libgen.h>

#include <pwd.h>
#include <grp.h>
--
2.0.4


2014-08-06 06:25:17

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 02/11] Fix header include for definition of NULL

NULL is defined in stdlib.h so we need to include that.

Signed-off-by: Natanael Copa <[email protected]>
---
support/include/sockaddr.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/support/include/sockaddr.h b/support/include/sockaddr.h
index a1c30f9..446b537 100644
--- a/support/include/sockaddr.h
+++ b/support/include/sockaddr.h
@@ -27,6 +27,7 @@
#ifdef HAVE_LIBIO_H
#include <libio.h>
#endif
+#include <stdlib.h>
#include <stdbool.h>
#include <sys/socket.h>
#include <netinet/in.h>
--
2.0.4


2014-08-06 06:25:18

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 03/11] configure.ac: enable GNU_SOURCE for stat64/statfs64

Use AC_USE_SYSTEM_EXTENSIONS to enable GNU_SOURCE, which is needed
for:
- stat64 in utils/exportfs/exportfs.c
- statfs64 in utils/mountd/cache.c

Signed-off-by: Natanael Copa <[email protected]>
---
configure.ac | 1 +
1 file changed, 1 insertion(+)

diff --git a/configure.ac b/configure.ac
index 408f4c8..b9682ed 100644
--- a/configure.ac
+++ b/configure.ac
@@ -8,6 +8,7 @@ AM_INIT_AUTOMAKE
AC_PREREQ(2.59)
AC_PREFIX_DEFAULT(/usr)
AM_MAINTAINER_MODE
+AC_USE_SYSTEM_EXTENSIONS

dnl *************************************************************
dnl * Define the set of applicable options
--
2.0.4


2014-08-06 06:25:19

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 04/11] replace __attribute_malloc__ with the more portable __attribute__((__malloc__))

Signed-off-by: Natanael Copa <[email protected]>
---
support/export/hostname.c | 14 +++++++-------
support/include/exportfs.h | 10 +++++-----
support/nfs/svc_create.c | 2 +-
utils/statd/hostname.c | 6 +++---
utils/statd/sm-notify.c | 8 ++++----
utils/statd/statd.h | 2 +-
6 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/support/export/hostname.c b/support/export/hostname.c
index 3e949a1..ad595d1 100644
--- a/support/export/hostname.c
+++ b/support/export/hostname.c
@@ -91,7 +91,7 @@ host_ntop(const struct sockaddr *sap, char *buf, const size_t buflen)
* Returns address info structure, or NULL if an error occurs. Caller
* must free the returned structure with freeaddrinfo(3).
*/
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo *
host_pton(const char *paddr)
{
@@ -153,7 +153,7 @@ host_pton(const char *paddr)
* if no information is available for @hostname. Caller must free the
* returned structure with freeaddrinfo(3).
*/
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo *
host_addrinfo(const char *hostname)
{
@@ -199,7 +199,7 @@ host_addrinfo(const char *hostname)
* the string.
*/
#ifdef HAVE_GETNAMEINFO
-__attribute_malloc__
+__attribute__((__malloc__))
char *
host_canonname(const struct sockaddr *sap)
{
@@ -234,7 +234,7 @@ host_canonname(const struct sockaddr *sap)
return strdup(buf);
}
#else /* !HAVE_GETNAMEINFO */
-__attribute_malloc__
+__attribute__((__malloc__))
char *
host_canonname(const struct sockaddr *sap)
{
@@ -266,7 +266,7 @@ host_canonname(const struct sockaddr *sap)
*
* Caller must free the returned structure with freeaddrinfo(3).
*/
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo *
host_reliable_addrinfo(const struct sockaddr *sap)
{
@@ -313,7 +313,7 @@ out_free_hostname:
* Caller must free the returned structure with freeaddrinfo(3).
*/
#ifdef HAVE_GETNAMEINFO
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo *
host_numeric_addrinfo(const struct sockaddr *sap)
{
@@ -361,7 +361,7 @@ host_numeric_addrinfo(const struct sockaddr *sap)
return ai;
}
#else /* !HAVE_GETNAMEINFO */
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo *
host_numeric_addrinfo(const struct sockaddr *sap)
{
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 97b2327..9021fae 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -156,15 +156,15 @@ int secinfo_addflavor(struct flav_info *, struct exportent *);

char * host_ntop(const struct sockaddr *sap,
char *buf, const size_t buflen);
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo * host_pton(const char *paddr);
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo * host_addrinfo(const char *hostname);
-__attribute_malloc__
+__attribute__((__malloc__))
char * host_canonname(const struct sockaddr *sap);
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo * host_reliable_addrinfo(const struct sockaddr *sap);
-__attribute_malloc__
+__attribute__((__malloc__))
struct addrinfo * host_numeric_addrinfo(const struct sockaddr *sap);

int rmtab_read(void);
diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index 6b9e85b..a706f87 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -113,7 +113,7 @@ svc_create_find_xprt(const struct sockaddr *bindaddr, const struct netconfig *nc
*
* Otherwise NULL is returned if an error occurs.
*/
-__attribute_malloc__
+__attribute__((__malloc__))
static struct addrinfo *
svc_create_bindaddr(struct netconfig *nconf, const uint16_t port)
{
diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
index 746ecc7..c61087c 100644
--- a/utils/statd/hostname.c
+++ b/utils/statd/hostname.c
@@ -105,7 +105,7 @@ statd_present_address(const struct sockaddr *sap, char *buf, const size_t buflen
* Look up the hostname; report exceptional errors. Caller must
* call freeaddrinfo(3) if a valid addrinfo is returned.
*/
-__attribute_malloc__
+__attribute__((__malloc__))
static struct addrinfo *
get_addrinfo(const char *hostname, const struct addrinfo *hint)
{
@@ -184,7 +184,7 @@ get_nameinfo(const struct sockaddr *sap,
* We won't monitor peers that don't have a reverse map. The canonical
* name gives us a key for our monitor list.
*/
-__attribute_malloc__
+__attribute__((__malloc__))
char *
statd_canonical_name(const char *hostname)
{
@@ -234,7 +234,7 @@ statd_canonical_name(const char *hostname)
* NULL if some error occurs. Caller must free the returned
* list with freeaddrinfo(3).
*/
-__attribute_malloc__
+__attribute__((__malloc__))
static struct addrinfo *
statd_canonical_list(const char *hostname)
{
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 9dbe5d9..5994b2f 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -74,7 +74,7 @@ static int record_pid(void);

static struct nsm_host * hosts = NULL;

-__attribute_malloc__
+__attribute__((__malloc__))
static struct addrinfo *
smn_lookup(const char *name)
{
@@ -149,7 +149,7 @@ smn_get_hostname(const struct sockaddr *sap,
* if the canonical name doesn't exist or cannot be determined.
* The caller must free the result with free(3).
*/
-__attribute_malloc__
+__attribute__((__malloc__))
static char *
smn_verify_my_name(const char *name)
{
@@ -189,7 +189,7 @@ smn_verify_my_name(const char *name)
return retval;
}

-__attribute_malloc__
+__attribute__((__malloc__))
static struct nsm_host *
smn_alloc_host(const char *hostname, const char *mon_name,
const char *my_name, const time_t timestamp)
@@ -343,7 +343,7 @@ static int smn_socket(void)
* If admin specified a source address or srcport, then convert those
* to a sockaddr and return it. Otherwise, return an ANYADDR address.
*/
-__attribute_malloc__
+__attribute__((__malloc__))
static struct addrinfo *
smn_bind_address(const char *srcaddr, const char *srcport)
{
diff --git a/utils/statd/statd.h b/utils/statd/statd.h
index e89e666..a1d8035 100644
--- a/utils/statd/statd.h
+++ b/utils/statd/statd.h
@@ -25,7 +25,7 @@
extern _Bool statd_matchhostname(const char *hostname1, const char *hostname2);
extern _Bool statd_present_address(const struct sockaddr *sap, char *buf,
const size_t buflen);
-__attribute_malloc__
+__attribute__((__malloc__))
extern char * statd_canonical_name(const char *hostname);

extern void my_svc_run(void);
--
2.0.4


2014-08-06 06:25:19

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 05/11] mountd: use standard dev_t instead of glibc internals

The __dev_t is a GNU libc internal. Use the standard dev_t instead,
which is specified in POSIX.

Signed-off-by: Natanael Copa <[email protected]>
---
utils/mountd/cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index b0cc6a8..663a52a 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -1419,7 +1419,7 @@ static int cache_export_ent(char *domain, struct exportent *exp, char *path)
*/
struct stat stb;
size_t l = strlen(exp->e_path);
- __dev_t dev;
+ dev_t dev;

if (strlen(path) <= l || path[l] != '/' ||
strncmp(exp->e_path, path, l) != 0)
--
2.0.4


2014-08-06 06:25:24

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 11/11] exportfs: only do glibc specific hackery on glibc

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 <[email protected]>
---
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__)
/*
* 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 */
--
2.0.4


2014-08-06 06:25:20

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 06/11] nfsstat: replace the legacy SA_ONESHOT with standard SA_RESETHAND

Signed-off-by: Natanael Copa <[email protected]>
---
utils/nfsstat/nfsstat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/nfsstat/nfsstat.c b/utils/nfsstat/nfsstat.c
index 70f8d10..3612bfe 100644
--- a/utils/nfsstat/nfsstat.c
+++ b/utils/nfsstat/nfsstat.c
@@ -336,7 +336,7 @@ main(int argc, char **argv)

struct sigaction act = {
.sa_handler = unpause,
- .sa_flags = SA_ONESHOT,
+ .sa_flags = SA_RESETHAND,
};

if ((progname = strrchr(argv[0], '/')))
--
2.0.4


2014-08-06 06:25:21

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 08/11] Only work around glibc bugs on glibc

Signed-off-by: Natanael Copa <[email protected]>
---
utils/mountd/svc_run.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/mountd/svc_run.c b/utils/mountd/svc_run.c
index 1938a67..a572441 100644
--- a/utils/mountd/svc_run.c
+++ b/utils/mountd/svc_run.c
@@ -60,7 +60,7 @@
void cache_set_fds(fd_set *fdset);
int cache_process_req(fd_set *readfds);

-#if LONG_MAX != INT_MAX
+#if defined(__GLIBC__) && LONG_MAX != INT_MAX
/* bug in glibc 2.3.6 and earlier, we need
* our own svc_getreqset
*/
--
2.0.4


2014-08-06 06:25:23

by Natanael Copa

[permalink] [raw]
Subject: [PATCH v2 10/11] exportfs: fix test of NULL pointer in host_pton()

should fix https://bugzilla.redhat.com/show_bug.cgi?id=1083018

Signed-off-by: Natanael Copa <[email protected]>
---
support/export/hostname.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/support/export/hostname.c b/support/export/hostname.c
index ad595d1..d9153e1 100644
--- a/support/export/hostname.c
+++ b/support/export/hostname.c
@@ -115,6 +115,11 @@ host_pton(const char *paddr)
* have a real AF_INET presentation address, before invoking
* getaddrinfo(3) to generate the full addrinfo list.
*/
+ if (paddr == NULL) {
+ xlog(D_GENERAL, "%s: passed a NULL presentation address",
+ __func__);
+ return NULL;
+ }
inet4 = 1;
if (inet_pton(AF_INET, paddr, &sin.sin_addr) == 0)
inet4 = 0;
@@ -123,15 +128,12 @@ host_pton(const char *paddr)
switch (error) {
case 0:
if (!inet4 && ai->ai_addr->sa_family == AF_INET) {
+ xlog(D_GENERAL, "%s: failed to convert %s",
+ __func__, paddr);
freeaddrinfo(ai);
break;
}
return ai;
- case EAI_NONAME:
- if (paddr == NULL)
- xlog(D_GENERAL, "%s: passed a NULL presentation address",
- __func__);
- break;
case EAI_SYSTEM:
xlog(D_GENERAL, "%s: failed to convert %s: (%d) %m",
__func__, paddr, errno);
--
2.0.4


2014-08-13 09:04:21

by Natanael Copa

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

On Tue, 12 Aug 2014 07:03:54 -0400
Steve Dickson <[email protected]> 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 <[email protected]>
> > ---
> > 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 */
> >


2014-09-16 13:30:22

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Porting nfs-utils to musl libc



On 08/06/2014 02:24 AM, Natanael Copa wrote:
> Musl libc is a modern libc for Linux which focuses on correctness in
> standards-conformance. We use this libc for Alpine Linux.
>
> This patchset makes nfs-utils build with musl libc and should generally
> improve portability.
>
> It would be nice if at least some of the patches could be applied
> upstream so we can reduce the number of patches for getting nfs working
> on Alpine Linux.
>
> Changes v1 -> v2 based on feedback:
> - Use AC_USE_SYSTEM_EXTENSIONS in configure.ac instead of defining
> _GNU_SOURCE various places. This means that the patch "include
> libgen.h for basename" is strictly no longer needed, but I kept it
> since this is more "correct".
> - Fix typo in "exportfs: only do glibc specific hackery on glibc"
>
> Natanael Copa (11):
> conffile: use standard uint*_t and unsigned char
> Fix header include for definition of NULL
> configure.ac: enable GNU_SOURCE for stat64/statfs64
> replace __attribute_malloc__ with the more portable
> __attribute__((__malloc__))
> mountd: use standard dev_t instead of glibc internals
> nfsstat: replace the legacy SA_ONESHOT with standard SA_RESETHAND
> Allow usage of getrpcbynumber() when getrpcbynumber_r() is unavailable
> Only work around glibc bugs on glibc
> include libgen.h for basename
> exportfs: fix test of NULL pointer in host_pton()
> exportfs: only do glibc specific hackery on glibc
All the patches have been committed...

steved.

>
> configure.ac | 7 ++-----
> support/export/hostname.c | 29 +++++++++++++++++------------
> support/include/conffile.h | 2 +-
> support/include/exportfs.h | 10 +++++-----
> support/include/sockaddr.h | 1 +
> support/nfs/conffile.c | 14 +++++++-------
> support/nfs/svc_create.c | 2 +-
> support/nfs/svc_socket.c | 6 ++++++
> tools/rpcdebug/rpcdebug.c | 1 +
> utils/mount/mount.c | 1 +
> utils/mount/mount_libmount.c | 1 +
> utils/mountd/cache.c | 2 +-
> utils/mountd/svc_run.c | 2 +-
> utils/nfsd/nfsd.c | 1 +
> utils/nfsidmap/nfsidmap.c | 1 +
> utils/nfsstat/nfsstat.c | 2 +-
> utils/statd/hostname.c | 6 +++---
> utils/statd/sm-notify.c | 8 ++++----
> utils/statd/statd.h | 2 +-
> 19 files changed, 56 insertions(+), 42 deletions(-)
>