The getaddrinfo problem needs a proper fix and probably a
consideration of similar patterns elsewhere in the code.
The others are relatively straightforward and are more suitable for use as-is.
A fix not included would be to avoid using "-Werror" except when
developing-- or at least make its use optional. Without knowledge of
the compilers used by downstream, this option may cause builds to fail
for unimportant reasons (and even if it works with all compilers
today, it might not tomorrow...). Would be nice to have that in
before the next tagged release :). GCC 7.3.0 seems to be unhappy with
some prototypes for example, I don't have them handy but can send them
along if that'd be useful.
Thanks! LMK if you have any questions or concerns, hopefully these are helpful.
~Will
From 0c5e1c500a81314edbf668baaa76cc39053269a7 Mon Sep 17 00:00:00 2001
From: Will Dietz <w@xxxxxxxx>
Date: Sun, 29 Apr 2018 14:00:16 -0500
Subject: [PATCH 1/6] sm-notify: don't rely on non-standard getaddrinfo
behavior
getaddrinfo doesn't support both node=NULL and service=NULL,
one or both of these must be specified.
To workaround having to specify one of these, smn_bind_address()
uses getaddrinfo(NULL, "", ...) in the hopes that empty string
counts as specified (non-null) while not actually specifying
the service in terms of limiting the addresses used.
That's my best guess anyway.
Unfortunately this is not valid use of getaddrinfo,
and AFAICT standard says this should result in an error
(especially when specifying AI_NUMERICSERV).
Apparently some platforms don't return error in this case,
although none document this alternate behavior so
hopefully it does whatever NFS hopes it does :).
This invalid usage causes problems when using musl,
which returns an error (as the standards indicate).
From getaddrinfo(3):
-------
If AI_NUMERICSERV is specified in hints.ai_flags and service is not
NULL, then service must point to a string containing a numeric port
number.
-------
The string "" is not a numeric port number. It is not a number.
And later when describing return values and meaning:
-------
EAI_NONAME
The node or service is not known; or both node and service
are NULL; or AI_NUMERICSERV was specified in hints.ai_flags and service
was not a numeric port-number string.
-------
Which is what musl's getaddrinfo returns.
As a quick-fix, this uses 'getaddrinfo("0.0.0.0", NULL, ...)"
in this situation which isn't quite right but is enough to fix
sm-notify when built w/musl and using IPV4.
Fixing it "properly" is mostly a question of determining
what NFS "wants" to be done, and reworking code to
avoid relying on non-standard behavior.
There are other instances of this pattern in the codebase,
which should probably be fixing in the same effort.
This article explores this issue a bit on various platforms:
http://klickverbot.at/blog/2012/01/getaddrinfo-edge-case-behavior-on-windows-linux-and-osx/
---
utils/statd/sm-notify.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 6d19ec1..6e978d3 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -368,8 +368,8 @@ smn_bind_address(const char *srcaddr, const char *srcport)
hint.ai_flags |= AI_PASSIVE;
/* Do not allow "node" and "service" parameters both to be NULL */
- if (srcport == NULL)
- error = getaddrinfo(srcaddr, "", &hint, &ai);
+ if (srcaddr == NULL && srcport == NULL)
+ error = getaddrinfo("0.0.0.0", srcport, &hint, &ai);
else
error = getaddrinfo(srcaddr, srcport, &hint, &ai);
if (error != 0) {
--
2.17.0
From f543dcd50f66cf79df5fe963df2abb94a13b3589 Mon Sep 17 00:00:00 2001
From: Will Dietz <w@xxxxxxxx>
Date: Sun, 29 Apr 2018 15:59:37 -0500
Subject: [PATCH 2/6] fix HAVE_* checks so they work when functions aren't
found
The current checks are of form `#if HAVE_X`,
which causes an error when `HAVE_X` isn't defined.
AC_CHECK_FUNCS defines these if they're found,
and leaves them undefined otherwise.
---
support/nfs/svc_socket.c | 2 +-
utils/mountd/cache.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/support/nfs/svc_socket.c b/support/nfs/svc_socket.c
index 1239712..d56507a 100644
--- a/support/nfs/svc_socket.c
+++ b/support/nfs/svc_socket.c
@@ -46,7 +46,7 @@ int getservport(u_long number, const char *proto)
struct rpcent *rpcp;
struct servent servbuf, *servp = NULL;
int ret = 0;
-#if HAVE_GETRPCBYNUMBER_R
+#ifdef HAVE_GETRPCBYNUMBER_R
char rpcdata[1024];
struct rpcent rpcbuf;
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 6f42512..c87ee8d 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -426,7 +426,7 @@ static int same_path(char *child, char *parent, int len)
if (count_slashes(p) != count_slashes(parent))
return 0;
-#if HAVE_NAME_TO_HANDLE_AT
+#ifdef HAVE_NAME_TO_HANDLE_AT
struct {
struct file_handle fh;
unsigned char handle[128];
--
2.17.0
From 0cd5ca3741980ee484876b37de23c3d4cd7a330f Mon Sep 17 00:00:00 2001
From: Will Dietz <w@xxxxxxxx>
Date: Sun, 29 Apr 2018 16:11:41 -0500
Subject: [PATCH 3/6] network.c: fix build w/non-glibc
---
utils/mount/network.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/utils/mount/network.c b/utils/mount/network.c
index e490399..75ef399 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -39,13 +39,21 @@
#include <sys/socket.h>
#include <sys/wait.h>
#include <sys/stat.h>
-#if defined(__GLIBC__) && (__GLIBC__ < 2) || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 24)
+
+#if defined(__GLIBC__)
+#if (__GLIBC__ < 2) || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 24)
+#define CANNOT_INCLUDE_IN6
+#endif
+#endif
+
+#ifndef CANNOT_INCLUDE_IN6
/* Cannot safely include linux/in6.h in old glibc, so hardcode the needed values */
# define IPV6_PREFER_SRC_PUBLIC 2
# define IPV6_ADDR_PREFERENCES 72
#else
# include <linux/in6.h>
#endif
+
#include <netinet/in.h>
#include <rpc/rpc.h>
#include <rpc/pmap_prot.h>
--
2.17.0
From e6e89e41aba9e57f64207ec9a9ee750c3cf63647 Mon Sep 17 00:00:00 2001
From: Will Dietz <w@xxxxxxxx>
Date: Sun, 29 Apr 2018 16:14:01 -0500
Subject: [PATCH 4/6] misc/file.c: include limits.h for PATH_MAX
---
support/misc/file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/support/misc/file.c b/support/misc/file.c
index 63597df..d47e8be 100644
--- a/support/misc/file.c
+++ b/support/misc/file.c
@@ -27,6 +27,7 @@
#include <dirent.h>
#include <stdlib.h>
#include <stdbool.h>
+#include <limits.h>
#include "xlog.h"
#include "misc.h"
--
2.17.0
From 1ec3e40c5f8055e27b802ce52e330cd1dc7ecd37 Mon Sep 17 00:00:00 2001
From: Will Dietz <w@xxxxxxxx>
Date: Sun, 29 Apr 2018 16:16:22 -0500
Subject: [PATCH 5/6] network.c: add cast to appease warning
---
utils/mount/network.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 75ef399..3e4b9b6 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1068,7 +1068,7 @@ int clnt_ping(struct sockaddr_in *saddr, const unsigned long prog,
if (caddr) {
/* Get the address of our end of this connection */
socklen_t len = sizeof(*caddr);
- if (getsockname(sock, caddr, &len) != 0)
+ if (getsockname(sock, (struct sockaddr *)caddr, &len) != 0)
caddr->sin_family = 0;
}
--
2.17.0
From 665a0e665a0e526a5d4021de28c3a3613cbe02db Mon Sep 17 00:00:00 2001
From: Will Dietz <w@xxxxxxxx>
Date: Sun, 29 Apr 2018 16:21:56 -0500
Subject: [PATCH 6/6] configure.ac: check for res_querydomain instead of
__res_querydomain
---
configure.ac | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 5a11636..8d5d9a1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -408,7 +408,7 @@ if test "$enable_gss" = yes; then
fi
dnl libdnsidmap specific checks
-AC_CHECK_LIB([resolv], [__res_querydomain], , AC_MSG_ERROR(res_querydomain needed))
+AC_CHECK_LIB([resolv], [res_querydomain], , AC_MSG_ERROR(res_querydomain needed))
AC_ARG_ENABLE([ldap],
[AS_HELP_STRING([--disable-ldap],[Disable support for LDAP @<:default=detect@:>@])])
--
2.17.0
