All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Misc fixes & cleanups for nfs-utils
@ 2020-07-01 18:27 Doug Nazar
  2020-07-01 18:27 ` [PATCH 01/10] gssd: Refcount struct clnt_info to protect multithread usage Doug Nazar
                   ` (12 more replies)
  0 siblings, 13 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Most of this work centers around gssd, however a few items I did tree
wide. It's been compile tested with both gcc & clang on x86_64 & arm32
and runtime tested on x86_64.


Doug Nazar (10):
  gssd: Refcount struct clnt_info to protect multithread usage
  Update to libevent 2.x apis.
  gssd: Cleanup on exit to support valgrind.
  gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release.
  gssd: Fix locking for machine principal list
  gssd: Add a few debug statements to help track client_info lifetimes.
  gssd: Lookup local hostname when srchost is '*'
  gssd: We never use the nocache param of gssd_check_if_cc_exists()
  Cleanup printf format attribute handling and fix format strings
  Fix various clang warnings.

 aclocal/libevent.m4                |   6 +-
 configure.ac                       |   6 +-
 support/include/compiler.h         |  14 +
 support/include/xcommon.h          |  12 +-
 support/include/xlog.h             |  20 +-
 support/nfs/xcommon.c              |   2 +
 support/nfsidmap/gums.c            |   2 +
 support/nfsidmap/libnfsidmap.c     |   8 +-
 support/nfsidmap/nfsidmap.h        |  10 +-
 support/nfsidmap/nfsidmap_common.c |   1 +
 support/nfsidmap/nss.c             |   4 +-
 support/nfsidmap/regex.c           |   6 +-
 support/nfsidmap/static.c          |   1 +
 support/nfsidmap/umich_ldap.c      |  10 +-
 tools/locktest/testlk.c            |   6 +-
 utils/exportfs/exportfs.c          |   5 +-
 utils/gssd/err_util.h              |   4 +-
 utils/gssd/gss_names.c             |   9 +-
 utils/gssd/gss_util.c              |   2 +-
 utils/gssd/gssd.c                  | 165 ++++++++---
 utils/gssd/gssd.h                  |  10 +-
 utils/gssd/gssd_proc.c             |  14 +-
 utils/gssd/krb5_util.c             | 422 +++++++++++++++++------------
 utils/gssd/krb5_util.h             |  16 +-
 utils/gssd/svcgssd.c               |   4 +-
 utils/gssd/svcgssd_proc.c          |   9 +-
 utils/idmapd/idmapd.c              |  65 +++--
 utils/mount/network.c              |   4 +-
 utils/mount/stropts.c              |   2 -
 utils/mountd/cache.c               |   2 +-
 utils/nfsdcld/cld-internal.h       |   2 +-
 utils/nfsdcld/nfsdcld.c            |  29 +-
 utils/nfsdcld/sqlite.c             |   1 -
 utils/nfsdcltrack/sqlite.c         |   2 +-
 utils/nfsidmap/nfsidmap.c          |   3 +-
 35 files changed, 536 insertions(+), 342 deletions(-)
 create mode 100644 support/include/compiler.h

-- 
2.26.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/10] gssd: Refcount struct clnt_info to protect multithread usage
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-01 18:27 ` [PATCH 02/10] Update to libevent 2.x apis Doug Nazar
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Struct clnt_info is shared with the various upcall threads so
we need to ensure that it stays around even if the client dir
gets removed.

Reported-by: Sebastian Kraus <sebastian.kraus@tu-berlin.de>
Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/gssd.c      | 67 ++++++++++++++++++++++++++++++++----------
 utils/gssd/gssd.h      |  5 ++--
 utils/gssd/gssd_proc.c |  4 +--
 3 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 588da0fb..b40c3220 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -90,9 +90,7 @@ char *ccachedir = NULL;
 /* Avoid DNS reverse lookups on server names */
 static bool avoid_dns = true;
 static bool use_gssproxy = false;
-int thread_started = false;
-pthread_mutex_t pmutex = PTHREAD_MUTEX_INITIALIZER;
-pthread_cond_t pcond = PTHREAD_COND_INITIALIZER;
+pthread_mutex_t clp_lock = PTHREAD_MUTEX_INITIALIZER;
 
 TAILQ_HEAD(topdir_list_head, topdir) topdir_list;
 
@@ -359,20 +357,28 @@ out:
 	free(port);
 }
 
+/* Actually frees clp and fields that might be used from other
+ * threads if was last reference.
+ */
 static void
-gssd_destroy_client(struct clnt_info *clp)
+gssd_free_client(struct clnt_info *clp)
 {
-	if (clp->krb5_fd >= 0) {
+	int refcnt;
+
+	pthread_mutex_lock(&clp_lock);
+	refcnt = --clp->refcount;
+	pthread_mutex_unlock(&clp_lock);
+	if (refcnt > 0)
+		return;
+
+	printerr(3, "freeing client %s\n", clp->relpath);
+
+	if (clp->krb5_fd >= 0)
 		close(clp->krb5_fd);
-		event_del(&clp->krb5_ev);
-	}
 
-	if (clp->gssd_fd >= 0) {
+	if (clp->gssd_fd >= 0)
 		close(clp->gssd_fd);
-		event_del(&clp->gssd_ev);
-	}
 
-	inotify_rm_watch(inotify_fd, clp->wd);
 	free(clp->relpath);
 	free(clp->servicename);
 	free(clp->servername);
@@ -380,6 +386,24 @@ gssd_destroy_client(struct clnt_info *clp)
 	free(clp);
 }
 
+/* Called when removing from clnt_list to tear down event handling.
+ * Will then free clp if was last reference.
+ */
+static void
+gssd_destroy_client(struct clnt_info *clp)
+{
+	printerr(3, "destroying client %s\n", clp->relpath);
+
+	if (clp->krb5_fd >= 0)
+		event_del(&clp->krb5_ev);
+
+	if (clp->gssd_fd >= 0)
+		event_del(&clp->gssd_ev);
+
+	inotify_rm_watch(inotify_fd, clp->wd);
+	gssd_free_client(clp);
+}
+
 static void gssd_scan(void);
 
 static int
@@ -416,11 +440,21 @@ static struct clnt_upcall_info *alloc_upcall_info(struct clnt_info *clp)
 	info = malloc(sizeof(struct clnt_upcall_info));
 	if (info == NULL)
 		return NULL;
+
+	pthread_mutex_lock(&clp_lock);
+	clp->refcount++;
+	pthread_mutex_unlock(&clp_lock);
 	info->clp = clp;
 
 	return info;
 }
 
+void free_upcall_info(struct clnt_upcall_info *info)
+{
+	gssd_free_client(info->clp);
+	free(info);
+}
+
 /* For each upcall read the upcall info into the buffer, then create a
  * thread in a detached state so that resources are released back into
  * the system without the need for a join.
@@ -438,13 +472,13 @@ gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data)
 	info->lbuflen = read(clp->gssd_fd, info->lbuf, sizeof(info->lbuf));
 	if (info->lbuflen <= 0 || info->lbuf[info->lbuflen-1] != '\n') {
 		printerr(0, "WARNING: %s: failed reading request\n", __func__);
-		free(info);
+		free_upcall_info(info);
 		return;
 	}
 	info->lbuf[info->lbuflen-1] = 0;
 
 	if (start_upcall_thread(handle_gssd_upcall, info))
-		free(info);
+		free_upcall_info(info);
 }
 
 static void
@@ -461,12 +495,12 @@ gssd_clnt_krb5_cb(int UNUSED(fd), short UNUSED(which), void *data)
 			sizeof(info->uid)) < (ssize_t)sizeof(info->uid)) {
 		printerr(0, "WARNING: %s: failed reading uid from krb5 "
 			 "upcall pipe: %s\n", __func__, strerror(errno));
-		free(info);
+		free_upcall_info(info);
 		return;
 	}
 
 	if (start_upcall_thread(handle_krb5_upcall, info))
-		free(info);
+		free_upcall_info(info);
 }
 
 static struct clnt_info *
@@ -501,6 +535,7 @@ gssd_get_clnt(struct topdir *tdi, const char *name)
 	clp->name = clp->relpath + strlen(tdi->name) + 1;
 	clp->krb5_fd = -1;
 	clp->gssd_fd = -1;
+	clp->refcount = 1;
 
 	TAILQ_INSERT_HEAD(&tdi->clnt_list, clp, list);
 	return clp;
@@ -651,7 +686,7 @@ gssd_scan_topdir(const char *name)
 		if (clp->scanned)
 			continue;
 
-		printerr(3, "destroying client %s\n", clp->relpath);
+		printerr(3, "orphaned client %s\n", clp->relpath);
 		saveprev = clp->list.tqe_prev;
 		TAILQ_REMOVE(&tdi->clnt_list, clp, list);
 		gssd_destroy_client(clp);
diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h
index f4f59754..d33e4dff 100644
--- a/utils/gssd/gssd.h
+++ b/utils/gssd/gssd.h
@@ -63,12 +63,10 @@ extern unsigned int 		context_timeout;
 extern unsigned int rpc_timeout;
 extern char			*preferred_realm;
 extern pthread_mutex_t ple_lock;
-extern pthread_cond_t pcond;
-extern pthread_mutex_t pmutex;
-extern int thread_started;
 
 struct clnt_info {
 	TAILQ_ENTRY(clnt_info)	list;
+	int			refcount;
 	int			wd;
 	bool			scanned;
 	char			*name;
@@ -94,6 +92,7 @@ struct clnt_upcall_info {
 
 void handle_krb5_upcall(struct clnt_upcall_info *clp);
 void handle_gssd_upcall(struct clnt_upcall_info *clp);
+void free_upcall_info(struct clnt_upcall_info *info);
 
 
 #endif /* _RPC_GSSD_H_ */
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 8fe6605b..05c1da64 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -730,7 +730,7 @@ handle_krb5_upcall(struct clnt_upcall_info *info)
 	printerr(2, "\n%s: uid %d (%s)\n", __func__, info->uid, clp->relpath);
 
 	process_krb5_upcall(clp, info->uid, clp->krb5_fd, NULL, NULL, NULL);
-	free(info);
+	free_upcall_info(info);
 }
 
 void
@@ -830,6 +830,6 @@ handle_gssd_upcall(struct clnt_upcall_info *info)
 out:
 	free(upcall_str);
 out_nomem:
-	free(info);
+	free_upcall_info(info);
 	return;
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 02/10] Update to libevent 2.x apis.
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
  2020-07-01 18:27 ` [PATCH 01/10] gssd: Refcount struct clnt_info to protect multithread usage Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-01 18:27 ` [PATCH 03/10] gssd: Cleanup on exit to support valgrind Doug Nazar
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 aclocal/libevent.m4          |  6 ++--
 utils/gssd/gssd.c            | 58 +++++++++++++++++++++------------
 utils/gssd/gssd.h            |  4 +--
 utils/idmapd/idmapd.c        | 62 ++++++++++++++++++++----------------
 utils/nfsdcld/cld-internal.h |  2 +-
 utils/nfsdcld/nfsdcld.c      | 29 +++++++++--------
 utils/nfsdcld/sqlite.c       |  1 -
 utils/nfsdcltrack/sqlite.c   |  2 +-
 8 files changed, 94 insertions(+), 70 deletions(-)

diff --git a/aclocal/libevent.m4 b/aclocal/libevent.m4
index b5ac00ff..e0b820b2 100644
--- a/aclocal/libevent.m4
+++ b/aclocal/libevent.m4
@@ -1,12 +1,12 @@
 dnl Checks for libevent
 AC_DEFUN([AC_LIBEVENT], [
 
-  dnl Check for libevent, but do not add -levent to LIBS
-  AC_CHECK_LIB([event], [event_dispatch], [LIBEVENT=-levent],
+  dnl Check for libevent, but do not add -levent_core to LIBS
+  AC_CHECK_LIB([event_core], [event_base_dispatch], [LIBEVENT=-levent_core],
                [AC_MSG_ERROR([libevent not found.])])
   AC_SUBST(LIBEVENT)
 
-  AC_CHECK_HEADERS([event.h], ,
+  AC_CHECK_HEADERS([event2/event.h], ,
                    [AC_MSG_ERROR([libevent headers not found.])])
 
 ])dnl
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index b40c3220..f8f21f74 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -64,7 +64,7 @@
 #include <fcntl.h>
 #include <dirent.h>
 #include <netdb.h>
-#include <event.h>
+#include <event2/event.h>
 
 #include "gssd.h"
 #include "err_util.h"
@@ -77,7 +77,7 @@ static char *pipefs_path = GSSD_PIPEFS_DIR;
 static DIR *pipefs_dir;
 static int pipefs_fd;
 static int inotify_fd;
-struct event inotify_ev;
+struct event *inotify_ev;
 
 char *keytabfile = GSSD_DEFAULT_KEYTAB_FILE;
 char **ccachesearch;
@@ -91,6 +91,7 @@ char *ccachedir = NULL;
 static bool avoid_dns = true;
 static bool use_gssproxy = false;
 pthread_mutex_t clp_lock = PTHREAD_MUTEX_INITIALIZER;
+static struct event_base *evbase = NULL;
 
 TAILQ_HEAD(topdir_list_head, topdir) topdir_list;
 
@@ -394,11 +395,17 @@ gssd_destroy_client(struct clnt_info *clp)
 {
 	printerr(3, "destroying client %s\n", clp->relpath);
 
-	if (clp->krb5_fd >= 0)
-		event_del(&clp->krb5_ev);
+	if (clp->krb5_ev) {
+		event_del(clp->krb5_ev);
+		event_free(clp->krb5_ev);
+		clp->krb5_ev = NULL;
+	}
 
-	if (clp->gssd_fd >= 0)
-		event_del(&clp->gssd_ev);
+	if (clp->gssd_ev) {
+		event_del(clp->gssd_ev);
+		event_free(clp->gssd_ev);
+		clp->gssd_ev = NULL;
+	}
 
 	inotify_rm_watch(inotify_fd, clp->wd);
 	gssd_free_client(clp);
@@ -571,15 +578,15 @@ gssd_scan_clnt(struct clnt_info *clp)
 		clp->krb5_fd = openat(clntfd, "krb5", O_RDWR | O_NONBLOCK);
 
 	if (gssd_was_closed && clp->gssd_fd >= 0) {
-		event_set(&clp->gssd_ev, clp->gssd_fd, EV_READ | EV_PERSIST,
-			  gssd_clnt_gssd_cb, clp);
-		event_add(&clp->gssd_ev, NULL);
+		clp->gssd_ev = event_new(evbase, clp->gssd_fd, EV_READ | EV_PERSIST,
+					 gssd_clnt_gssd_cb, clp);
+		event_add(clp->gssd_ev, NULL);
 	}
 
 	if (krb5_was_closed && clp->krb5_fd >= 0) {
-		event_set(&clp->krb5_ev, clp->krb5_fd, EV_READ | EV_PERSIST,
-			  gssd_clnt_krb5_cb, clp);
-		event_add(&clp->krb5_ev, NULL);
+		clp->krb5_ev = event_new(evbase, clp->krb5_fd, EV_READ | EV_PERSIST,
+					 gssd_clnt_krb5_cb, clp);
+		event_add(clp->krb5_ev, NULL);
 	}
 
 	if (clp->krb5_fd == -1 && clp->gssd_fd == -1)
@@ -783,12 +790,16 @@ gssd_inotify_clnt(struct topdir *tdi, struct clnt_info *clp, const struct inotif
 	} else if (ev->mask & IN_DELETE) {
 		if (!strcmp(ev->name, "gssd") && clp->gssd_fd >= 0) {
 			close(clp->gssd_fd);
-			event_del(&clp->gssd_ev);
+			event_del(clp->gssd_ev);
+			event_free(clp->gssd_ev);
+			clp->gssd_ev = NULL;
 			clp->gssd_fd = -1;
 
 		} else if (!strcmp(ev->name, "krb5") && clp->krb5_fd >= 0) {
 			close(clp->krb5_fd);
-			event_del(&clp->krb5_ev);
+			event_del(clp->krb5_ev);
+			event_free(clp->krb5_ev);
+			clp->krb5_ev = NULL;
 			clp->krb5_fd = -1;
 		}
 
@@ -923,7 +934,7 @@ main(int argc, char *argv[])
 	int i;
 	extern char *optarg;
 	char *progname;
-	struct event sighup_ev;
+	struct event *sighup_ev;
 
 	read_gss_conf();
 
@@ -1062,7 +1073,11 @@ main(int argc, char *argv[])
 	if (gssd_check_mechs() != 0)
 		errx(1, "Problem with gssapi library");
 
-	event_init();
+	evbase = event_base_new();
+	if (!evbase) {
+		printerr(0, "ERROR: failed to create event base\n");
+		exit(EXIT_FAILURE);
+	}
 
 	pipefs_dir = opendir(pipefs_path);
 	if (!pipefs_dir) {
@@ -1084,16 +1099,17 @@ main(int argc, char *argv[])
 
 	signal(SIGINT, sig_die);
 	signal(SIGTERM, sig_die);
-	signal_set(&sighup_ev, SIGHUP, gssd_scan_cb, NULL);
-	signal_add(&sighup_ev, NULL);
-	event_set(&inotify_ev, inotify_fd, EV_READ | EV_PERSIST, gssd_inotify_cb, NULL);
-	event_add(&inotify_ev, NULL);
+	sighup_ev = evsignal_new(evbase, SIGHUP, gssd_scan_cb, NULL);
+	evsignal_add(sighup_ev, NULL);
+	inotify_ev = event_new(evbase, inotify_fd, EV_READ | EV_PERSIST,
+			       gssd_inotify_cb, NULL);
+	event_add(inotify_ev, NULL);
 
 	TAILQ_INIT(&topdir_list);
 	gssd_scan();
 	daemon_ready();
 
-	event_dispatch();
+	event_base_dispatch(evbase);
 
 	printerr(0, "ERROR: event_dispatch() returned!\n");
 	return EXIT_FAILURE;
diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h
index d33e4dff..ae0355bb 100644
--- a/utils/gssd/gssd.h
+++ b/utils/gssd/gssd.h
@@ -77,9 +77,9 @@ struct clnt_info {
 	int			vers;
 	char			*protocol;
 	int			krb5_fd;
-	struct event		krb5_ev;
+	struct event		*krb5_ev;
 	int			gssd_fd;
-	struct event		gssd_ev;
+	struct event		*gssd_ev;
 	struct			sockaddr_storage addr;
 };
 
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 893159f1..12648f67 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -49,7 +49,7 @@
 
 #include <err.h>
 #include <errno.h>
-#include <event.h>
+#include <event2/event.h>
 #include <fcntl.h>
 #include <dirent.h>
 #include <unistd.h>
@@ -115,7 +115,7 @@ struct idmap_client {
 	int                        ic_fd;
 	int                        ic_dirfd;
 	int                        ic_scanned;
-	struct event               ic_event;
+	struct event              *ic_event;
 	TAILQ_ENTRY(idmap_client)  ic_next;
 };
 static struct idmap_client nfsd_ic[2] = {
@@ -166,6 +166,7 @@ static char pipefsdir[PATH_MAX];
 static char *nobodyuser, *nobodygroup;
 static uid_t nobodyuid;
 static gid_t nobodygid;
+static struct event_base *evbase = NULL;
 
 static int
 flush_nfsd_cache(char *path, time_t now)
@@ -209,8 +210,8 @@ main(int argc, char **argv)
 {
 	int wd = -1, opt, fg = 0, nfsdret = -1;
 	struct idmap_clientq icq;
-	struct event rootdirev, clntdirev, svrdirev, inotifyev;
-	struct event initialize;
+	struct event *rootdirev, *clntdirev, *svrdirev, *inotifyev;
+	struct event *initialize;
 	struct passwd *pw;
 	struct group *gr;
 	struct stat sb;
@@ -341,7 +342,9 @@ main(int argc, char **argv)
 	if (nfs4_init_name_mapping(conf_path))
 		errx(1, "Unable to create name to user id mappings.");
 
-	event_init();
+	evbase = event_base_new();
+	if (evbase == NULL)
+		errx(1, "Failed to create event base.");
 
 	if (verbose > 0)
 		xlog_warn("Expiration time is %d seconds.",
@@ -396,22 +399,22 @@ main(int argc, char **argv)
 		TAILQ_INIT(&icq);
 
 		/* These events are persistent */
-		signal_set(&rootdirev, SIGUSR1, dirscancb, &icq);
-		signal_add(&rootdirev, NULL);
-		signal_set(&clntdirev, SIGUSR2, clntscancb, &icq);
-		signal_add(&clntdirev, NULL);
-		signal_set(&svrdirev, SIGHUP, svrreopen, NULL);
-		signal_add(&svrdirev, NULL);
+		rootdirev = evsignal_new(evbase, SIGUSR1, dirscancb, &icq);
+		evsignal_add(rootdirev, NULL);
+		clntdirev = evsignal_new(evbase, SIGUSR2, clntscancb, &icq);
+		evsignal_add(clntdirev, NULL);
+		svrdirev = evsignal_new(evbase, SIGHUP, svrreopen, NULL);
+		evsignal_add(svrdirev, NULL);
 		if ( wd >= 0) {
-			event_set(&inotifyev, inotify_fd, EV_READ, dirscancb, &icq);
-			event_add(&inotifyev, NULL);
+			inotifyev = event_new(evbase, inotify_fd, EV_READ, dirscancb, &icq);
+			event_add(inotifyev, NULL);
 		}
 
 		/* Fetch current state */
 		/* (Delay till start of event_dispatch to avoid possibly losing
 		 * a SIGUSR1 between here and the call to event_dispatch().) */
-		evtimer_set(&initialize, dirscancb, &icq);
-		evtimer_add(&initialize, &now);
+		initialize = evtimer_new(evbase, dirscancb, &icq);
+		evtimer_add(initialize, &now);
 	}
 
 	if (nfsdret != 0 && wd < 0)
@@ -419,7 +422,7 @@ main(int argc, char **argv)
 
 	daemon_ready();
 
-	if (event_dispatch() < 0)
+	if (event_base_dispatch(evbase) < 0)
 		xlog_err("main: event_dispatch returns errno %d (%s)",
 			    errno, strerror(errno));
 	/* NOTREACHED */
@@ -490,7 +493,8 @@ dirscancb(int UNUSED(fd), short UNUSED(which), void *data)
 	while(ic != NULL) {
 		nextic=TAILQ_NEXT(ic, ic_next);
 		if (!ic->ic_scanned) {
-			event_del(&ic->ic_event);
+			event_del(ic->ic_event);
+			event_free(ic->ic_event);
 			close(ic->ic_fd);
 			close(ic->ic_dirfd);
 			TAILQ_REMOVE(icq, ic, ic_next);
@@ -677,7 +681,7 @@ nfsdcb(int UNUSED(fd), short which, void *data)
 			     ic->ic_path, errno, strerror(errno));
 
 out:
-	event_add(&ic->ic_event, NULL);
+	event_add(ic->ic_event, NULL);
 }
 
 static void
@@ -743,7 +747,7 @@ nfscb(int UNUSED(fd), short which, void *data)
 	if (atomicio((void*)write, ic->ic_fd, &im, sizeof(im)) != sizeof(im))
 		xlog_warn("nfscb: write(%s): %s", ic->ic_path, strerror(errno));
 out:
-	event_add(&ic->ic_event, NULL);
+	event_add(ic->ic_event, NULL);
 }
 
 static void
@@ -755,14 +759,16 @@ nfsdreopen_one(struct idmap_client *ic)
 		xlog_warn("ReOpening %s", ic->ic_path);
 
 	if ((fd = open(ic->ic_path, O_RDWR, 0)) != -1) {
-		if ((event_initialized(&ic->ic_event)))
-			event_del(&ic->ic_event);
+		if (ic->ic_event && event_initialized(ic->ic_event)) {
+			event_del(ic->ic_event);
+			event_free(ic->ic_event);
+		}
 		if (ic->ic_fd != -1)
 			close(ic->ic_fd);
 
-		ic->ic_event.ev_fd = ic->ic_fd = fd;
-		event_set(&ic->ic_event, ic->ic_fd, EV_READ, nfsdcb, ic);
-		event_add(&ic->ic_event, NULL);
+		ic->ic_fd = fd;
+		ic->ic_event = event_new(evbase, ic->ic_fd, EV_READ, nfsdcb, ic);
+		event_add(ic->ic_event, NULL);
 	} else {
 		xlog_warn("nfsdreopen: Opening '%s' failed: errno %d (%s)",
 			ic->ic_path, errno, strerror(errno));
@@ -795,8 +801,8 @@ nfsdopenone(struct idmap_client *ic)
 		return (-1);
 	}
 
-	event_set(&ic->ic_event, ic->ic_fd, EV_READ, nfsdcb, ic);
-	event_add(&ic->ic_event, NULL);
+	ic->ic_event = event_new(evbase, ic->ic_fd, EV_READ, nfsdcb, ic);
+	event_add(ic->ic_event, NULL);
 
 	if (verbose > 0)
 		xlog_warn("Opened %s", ic->ic_path);
@@ -819,8 +825,8 @@ nfsopen(struct idmap_client *ic)
 			return (-1);
 		}
 	} else {
-		event_set(&ic->ic_event, ic->ic_fd, EV_READ, nfscb, ic);
-		event_add(&ic->ic_event, NULL);
+		ic->ic_event = event_new(evbase, ic->ic_fd, EV_READ, nfscb, ic);
+		event_add(ic->ic_event, NULL);
 		fcntl(ic->ic_dirfd, F_NOTIFY, 0);
 		fcntl(ic->ic_dirfd, F_SETSIG, 0);
 		if (verbose > 0)
diff --git a/utils/nfsdcld/cld-internal.h b/utils/nfsdcld/cld-internal.h
index cc283dae..35765157 100644
--- a/utils/nfsdcld/cld-internal.h
+++ b/utils/nfsdcld/cld-internal.h
@@ -26,7 +26,7 @@
 
 struct cld_client {
 	int			cl_fd;
-	struct event		cl_event;
+	struct event		*cl_event;
 	union {
 		struct cld_msg		cl_msg;
 #if UPCALL_VERSION >= 2
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index be655626..d6e1dff9 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -24,7 +24,7 @@
 #endif /* HAVE_CONFIG_H */
 
 #include <errno.h>
-#include <event.h>
+#include <event2/event.h>
 #include <stdbool.h>
 #include <getopt.h>
 #include <string.h>
@@ -66,7 +66,8 @@
 static char pipefs_dir[PATH_MAX] = DEFAULT_PIPEFS_DIR;
 static char pipepath[PATH_MAX];
 static int 		inotify_fd = -1;
-static struct event	pipedir_event;
+static struct event	 *pipedir_event;
+static struct event_base *evbase;
 static bool old_kernel = false;
 
 uint64_t current_epoch;
@@ -149,13 +150,15 @@ cld_pipe_open(struct cld_client *clnt)
 		return -errno;
 	}
 
-	if (event_initialized(&clnt->cl_event))
-		event_del(&clnt->cl_event);
+	if (clnt->cl_event && event_initialized(clnt->cl_event)) {
+		event_del(clnt->cl_event);
+		event_free(clnt->cl_event);
+	}
 	if (clnt->cl_fd >= 0)
 		close(clnt->cl_fd);
 
 	clnt->cl_fd = fd;
-	event_set(&clnt->cl_event, clnt->cl_fd, EV_READ, cldcb, clnt);
+	clnt->cl_event = event_new(evbase, clnt->cl_fd, EV_READ, cldcb, clnt);
 	/* event_add is done by the caller */
 	return 0;
 }
@@ -208,7 +211,7 @@ cld_inotify_cb(int UNUSED(fd), short which, void *data)
 	switch (ret) {
 	case 0:
 		/* readd the event for the cl_event pipe */
-		event_add(&clnt->cl_event, NULL);
+		event_add(clnt->cl_event, NULL);
 		break;
 	case -ENOENT:
 		/* pipe must have disappeared, wait for it to come back */
@@ -221,7 +224,7 @@ cld_inotify_cb(int UNUSED(fd), short which, void *data)
 	}
 
 out:
-	event_add(&pipedir_event, NULL);
+	event_add(pipedir_event, NULL);
 	free(dirc);
 }
 
@@ -286,7 +289,7 @@ cld_pipe_init(struct cld_client *clnt)
 	switch (ret) {
 	case 0:
 		/* add the event and we're good to go */
-		event_add(&clnt->cl_event, NULL);
+		event_add(clnt->cl_event, NULL);
 		break;
 	case -ENOENT:
 		/* ignore this error -- cld_inotify_cb will handle it */
@@ -299,8 +302,8 @@ cld_pipe_init(struct cld_client *clnt)
 	}
 
 	/* set event for inotify read */
-	event_set(&pipedir_event, inotify_fd, EV_READ, cld_inotify_cb, clnt);
-	event_add(&pipedir_event, NULL);
+	pipedir_event = event_new(evbase, inotify_fd, EV_READ, cld_inotify_cb, clnt);
+	event_add(pipedir_event, NULL);
 out:
 	return ret;
 }
@@ -737,7 +740,7 @@ cldcb(int UNUSED(fd), short which, void *data)
 		cld_not_implemented(clnt);
 	}
 out:
-	event_add(&clnt->cl_event, NULL);
+	event_add(clnt->cl_event, NULL);
 }
 
 int
@@ -762,7 +765,7 @@ main(int argc, char **argv)
 		return 1;
 	}
 
-	event_init();
+	evbase = event_base_new();
 	xlog_syslog(0);
 	xlog_stderr(1);
 
@@ -860,7 +863,7 @@ main(int argc, char **argv)
 		goto out;
 
 	xlog(D_GENERAL, "%s: Starting event dispatch handler.", __func__);
-	rc = event_dispatch();
+	rc = event_base_dispatch(evbase);
 	if (rc < 0)
 		xlog(L_ERROR, "%s: event_dispatch failed: %m", __func__);
 
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index 6666c867..2ba3a5f6 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -48,7 +48,6 @@
 
 #include <dirent.h>
 #include <errno.h>
-#include <event.h>
 #include <stdbool.h>
 #include <string.h>
 #include <sys/stat.h>
diff --git a/utils/nfsdcltrack/sqlite.c b/utils/nfsdcltrack/sqlite.c
index 28012016..f79aebb3 100644
--- a/utils/nfsdcltrack/sqlite.c
+++ b/utils/nfsdcltrack/sqlite.c
@@ -40,7 +40,7 @@
 
 #include <dirent.h>
 #include <errno.h>
-#include <event.h>
+#include <stdio.h>
 #include <stdbool.h>
 #include <string.h>
 #include <sys/stat.h>
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 03/10] gssd: Cleanup on exit to support valgrind.
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
  2020-07-01 18:27 ` [PATCH 01/10] gssd: Refcount struct clnt_info to protect multithread usage Doug Nazar
  2020-07-01 18:27 ` [PATCH 02/10] Update to libevent 2.x apis Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-01 18:27 ` [PATCH 04/10] gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release Doug Nazar
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/gssd.c      | 46 +++++++++++++++++++++++++++++++------
 utils/gssd/krb5_util.c | 52 ++++++++++++++++++++++++------------------
 utils/gssd/krb5_util.h |  2 +-
 3 files changed, 70 insertions(+), 30 deletions(-)

diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index f8f21f74..54d9b5de 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -91,6 +91,7 @@ char *ccachedir = NULL;
 static bool avoid_dns = true;
 static bool use_gssproxy = false;
 pthread_mutex_t clp_lock = PTHREAD_MUTEX_INITIALIZER;
+static bool signal_received = false;
 static struct event_base *evbase = NULL;
 
 TAILQ_HEAD(topdir_list_head, topdir) topdir_list;
@@ -872,10 +873,16 @@ found:
 static void
 sig_die(int signal)
 {
-	if (root_uses_machine_creds)
-		gssd_destroy_krb5_machine_creds();
+	if (signal_received)
+	{
+		gssd_destroy_krb5_principals(root_uses_machine_creds);
+		printerr(1, "forced exiting on signal %d\n", signal);
+		exit(0);
+	}
+
+	signal_received = true;
 	printerr(1, "exiting on signal %d\n", signal);
-	exit(0);
+	event_base_loopexit(evbase, NULL);
 }
 
 static void
@@ -932,6 +939,7 @@ main(int argc, char *argv[])
 	int rpc_verbosity = 0;
 	int opt;
 	int i;
+	int rc;
 	extern char *optarg;
 	char *progname;
 	struct event *sighup_ev;
@@ -1109,9 +1117,33 @@ main(int argc, char *argv[])
 	gssd_scan();
 	daemon_ready();
 
-	event_base_dispatch(evbase);
+	rc = event_base_dispatch(evbase);
 
-	printerr(0, "ERROR: event_dispatch() returned!\n");
-	return EXIT_FAILURE;
-}
+	printerr(0, "event_dispatch() returned %i!\n", rc);
 
+	gssd_destroy_krb5_principals(root_uses_machine_creds);
+
+	while (!TAILQ_EMPTY(&topdir_list)) {
+		struct topdir *tdi = TAILQ_FIRST(&topdir_list);
+		TAILQ_REMOVE(&topdir_list, tdi, list);
+		while (!TAILQ_EMPTY(&tdi->clnt_list)) {
+			struct clnt_info *clp = TAILQ_FIRST(&tdi->clnt_list);
+			TAILQ_REMOVE(&tdi->clnt_list, clp, list);
+			gssd_destroy_client(clp);
+		}
+		free(tdi);
+	}
+
+	event_free(inotify_ev);
+	event_free(sighup_ev);
+	event_base_free(evbase);
+
+	close(inotify_fd);
+	close(pipefs_fd);
+	closedir(pipefs_dir);
+
+	free(preferred_realm);
+	free(ccachesearch);
+
+	return rc < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 8c73748c..c49c6672 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -1226,7 +1226,7 @@ gssd_free_krb5_machine_cred_list(char **list)
  * Called upon exit.  Destroys machine credentials.
  */
 void
-gssd_destroy_krb5_machine_creds(void)
+gssd_destroy_krb5_principals(int destroy_machine_creds)
 {
 	krb5_context context;
 	krb5_error_code code = 0;
@@ -1238,33 +1238,41 @@ gssd_destroy_krb5_machine_creds(void)
 	if (code) {
 		k5err = gssd_k5_err_msg(NULL, code);
 		printerr(0, "ERROR: %s while initializing krb5\n", k5err);
-		goto out;
+		free(k5err);
+		return;
 	}
 
-	for (ple = gssd_k5_kt_princ_list; ple; ple = ple->next) {
-		if (!ple->ccname)
-			continue;
-		if ((code = krb5_cc_resolve(context, ple->ccname, &ccache))) {
-			k5err = gssd_k5_err_msg(context, code);
-			printerr(0, "WARNING: %s while resolving credential "
-				    "cache '%s' for destruction\n", k5err,
-				    ple->ccname);
-			krb5_free_string(context, k5err);
-			k5err = NULL;
-			continue;
-		}
+	pthread_mutex_lock(&ple_lock);
+	while (gssd_k5_kt_princ_list) {
+		ple = gssd_k5_kt_princ_list;
+		gssd_k5_kt_princ_list = ple->next;
 
-		if ((code = krb5_cc_destroy(context, ccache))) {
-			k5err = gssd_k5_err_msg(context, code);
-			printerr(0, "WARNING: %s while destroying credential "
-				    "cache '%s'\n", k5err, ple->ccname);
-			krb5_free_string(context, k5err);
-			k5err = NULL;
+		if (destroy_machine_creds && ple->ccname) {
+			if ((code = krb5_cc_resolve(context, ple->ccname, &ccache))) {
+				k5err = gssd_k5_err_msg(context, code);
+				printerr(0, "WARNING: %s while resolving credential "
+					    "cache '%s' for destruction\n", k5err,
+					    ple->ccname);
+				free(k5err);
+				k5err = NULL;
+			}
+
+			if (!code && (code = krb5_cc_destroy(context, ccache))) {
+				k5err = gssd_k5_err_msg(context, code);
+				printerr(0, "WARNING: %s while destroying credential "
+					    "cache '%s'\n", k5err, ple->ccname);
+				free(k5err);
+				k5err = NULL;
+			}
 		}
+
+		krb5_free_principal(context, ple->princ);
+		free(ple->ccname);
+		free(ple->realm);
+		free(ple);
 	}
+	pthread_mutex_unlock(&ple_lock);
 	krb5_free_context(context);
-  out:
-	krb5_free_string(context, k5err);
 }
 
 /*
diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
index b000b444..e127cc84 100644
--- a/utils/gssd/krb5_util.h
+++ b/utils/gssd/krb5_util.h
@@ -27,7 +27,7 @@ int gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername,
 				     char *dirname);
 int  gssd_get_krb5_machine_cred_list(char ***list);
 void gssd_free_krb5_machine_cred_list(char **list);
-void gssd_destroy_krb5_machine_creds(void);
+void gssd_destroy_krb5_principals(int destroy_machine_creds);
 int  gssd_refresh_krb5_machine_credential(char *hostname,
 					  struct gssd_k5_kt_princ *ple, 
 					  char *service, char *srchost);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 04/10] gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release.
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (2 preceding siblings ...)
  2020-07-01 18:27 ` [PATCH 03/10] gssd: Cleanup on exit to support valgrind Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-08 14:50   ` [PATCH 04/10] gssd: gssd_k5_err_msg() returns a ". " Steve Dickson
  2020-07-01 18:27 ` [PATCH 05/10] gssd: Fix locking for machine principal list Doug Nazar
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/krb5_util.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index c49c6672..b1e48241 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -484,7 +484,7 @@ gssd_get_single_krb5_cred(krb5_context context,
 	if (ccache)
 		krb5_cc_close(context, ccache);
 	krb5_free_cred_contents(context, &my_creds);
-	krb5_free_string(context, k5err);
+	free(k5err);
 	return (code);
 }
 
@@ -723,7 +723,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
 				 "we failed to unparse principal name: %s\n",
 				 k5err);
 			k5_free_kt_entry(context, kte);
-			krb5_free_string(context, k5err);
+			free(k5err);
 			k5err = NULL;
 			continue;
 		}
@@ -770,7 +770,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
 	if (retval < 0)
 		retval = 0;
   out:
-	krb5_free_string(context, k5err);
+	free(k5err);
 	return retval;
 }
 
@@ -927,7 +927,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt,
 				k5err = gssd_k5_err_msg(context, code);
 				printerr(1, "%s while building principal for '%s'\n",
 					 k5err, spn);
-				krb5_free_string(context, k5err);
+				free(k5err);
 				k5err = NULL;
 				continue;
 			}
@@ -937,7 +937,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt,
 				k5err = gssd_k5_err_msg(context, code);
 				printerr(3, "%s while getting keytab entry for '%s'\n",
 					 k5err, spn);
-				krb5_free_string(context, k5err);
+				free(k5err);
 				k5err = NULL;
 				/*
 				 * We tried the active directory machine account
@@ -986,7 +986,7 @@ out:
 		k5_free_default_realm(context, default_realm);
 	if (realmnames)
 		krb5_free_host_realm(context, realmnames);
-	krb5_free_string(context, k5err);
+	free(k5err);
 	return retval;
 }
 
@@ -1355,7 +1355,7 @@ out_free_kt:
 out_free_context:
 	krb5_free_context(context);
 out:
-	krb5_free_string(context, k5err);
+	free(k5err);
 	return retval;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 05/10] gssd: Fix locking for machine principal list
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (3 preceding siblings ...)
  2020-07-01 18:27 ` [PATCH 04/10] gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-01 18:27 ` [PATCH 06/10] gssd: Add a few debug statements to help track client_info lifetimes Doug Nazar
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Add missing locking for some scans of the global list. There was
also no prevention of ple->ccname being changed concurrently so
use the same lock to protect that. Reference counting was also added
to ensure that the ple is not freed out from under us in the few
places we now drop the lock while doing work.

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/gssd.h      |   1 -
 utils/gssd/gssd_proc.c |   2 +-
 utils/gssd/krb5_util.c | 294 ++++++++++++++++++++++++++---------------
 utils/gssd/krb5_util.h |  14 --
 4 files changed, 185 insertions(+), 126 deletions(-)

diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h
index ae0355bb..1e8c58d4 100644
--- a/utils/gssd/gssd.h
+++ b/utils/gssd/gssd.h
@@ -62,7 +62,6 @@ extern int			root_uses_machine_creds;
 extern unsigned int 		context_timeout;
 extern unsigned int rpc_timeout;
 extern char			*preferred_realm;
-extern pthread_mutex_t ple_lock;
 
 struct clnt_info {
 	TAILQ_ENTRY(clnt_info)	list;
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 05c1da64..addac318 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -548,7 +548,7 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid,
 		uid, tgtname);
 
 	do {
-		gssd_refresh_krb5_machine_credential(clp->servername, NULL,
+		gssd_refresh_krb5_machine_credential(clp->servername,
 						     service, srchost);
 	/*
 	 * Get a list of credential cache names and try each
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index b1e48241..7908c10f 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -130,9 +130,28 @@
 #include "gss_util.h"
 #include "krb5_util.h"
 
+/*
+ * List of principals from our keytab that we
+ * will try to use to obtain credentials
+ * (known as a principal list entry (ple))
+ */
+struct gssd_k5_kt_princ {
+	struct gssd_k5_kt_princ *next;
+	// Only protect against deletion, not modification
+	int refcount;
+	// Only set during creation in new_ple()
+	krb5_principal princ;
+	char *realm;
+	// Modified during usage by gssd_get_single_krb5_cred()
+	char *ccname;
+	krb5_timestamp endtime;
+};
+
+
 /* Global list of principals/cache file names for machine credentials */
-struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL;
-pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER;
+static struct gssd_k5_kt_princ *gssd_k5_kt_princ_list = NULL;
+/* This mutex protects list modification & ple->ccname */
+static pthread_mutex_t ple_lock = PTHREAD_MUTEX_INITIALIZER;
 
 #ifdef HAVE_SET_ALLOWABLE_ENCTYPES
 int limit_to_legacy_enctypes = 0;
@@ -150,6 +169,18 @@ static int gssd_get_single_krb5_cred(krb5_context context,
 static int query_krb5_ccache(const char* cred_cache, char **ret_princname,
 		char **ret_realm);
 
+static void release_ple(krb5_context context, struct gssd_k5_kt_princ *ple)
+{
+	if (--ple->refcount)
+		return;
+
+	printerr(3, "freeing cached principal (ccname=%s, realm=%s)\n", ple->ccname, ple->realm);
+	krb5_free_principal(context, ple->princ);
+	free(ple->ccname);
+	free(ple->realm);
+	free(ple);
+}
+
 /*
  * Called from the scandir function to weed out potential krb5
  * credentials cache files
@@ -377,12 +408,15 @@ gssd_get_single_krb5_cred(krb5_context context,
 	 * 300 because clock skew must be within 300sec for kerberos
 	 */
 	now += 300;
+	pthread_mutex_lock(&ple_lock);
 	if (ple->ccname && ple->endtime > now && !nocache) {
 		printerr(3, "INFO: Credentials in CC '%s' are good until %d\n",
 			 ple->ccname, ple->endtime);
 		code = 0;
+		pthread_mutex_unlock(&ple_lock);
 		goto out;
 	}
+	pthread_mutex_unlock(&ple_lock);
 
 	if ((code = krb5_kt_get_name(context, kt, kt_name, BUFSIZ))) {
 		printerr(0, "ERROR: Unable to get keytab name in "
@@ -435,6 +469,7 @@ gssd_get_single_krb5_cred(krb5_context context,
 	 * Initialize cache file which we're going to be using
 	 */
 
+	pthread_mutex_lock(&ple_lock);
 	if (use_memcache)
 	    cache_type = "MEMORY";
 	else
@@ -444,15 +479,18 @@ gssd_get_single_krb5_cred(krb5_context context,
 		ccachesearch[0], GSSD_DEFAULT_CRED_PREFIX,
 		GSSD_DEFAULT_MACHINE_CRED_SUFFIX, ple->realm);
 	ple->endtime = my_creds.times.endtime;
-	if (ple->ccname != NULL)
+	if (ple->ccname == NULL || strcmp(ple->ccname, cc_name) != 0) {
 		free(ple->ccname);
-	ple->ccname = strdup(cc_name);
-	if (ple->ccname == NULL) {
-		printerr(0, "ERROR: no storage to duplicate credentials "
-			    "cache name '%s'\n", cc_name);
-		code = ENOMEM;
-		goto out;
+		ple->ccname = strdup(cc_name);
+		if (ple->ccname == NULL) {
+			printerr(0, "ERROR: no storage to duplicate credentials "
+				    "cache name '%s'\n", cc_name);
+			code = ENOMEM;
+			pthread_mutex_unlock(&ple_lock);
+			goto out;
+		}
 	}
+	pthread_mutex_unlock(&ple_lock);
 	if ((code = krb5_cc_resolve(context, cc_name, &ccache))) {
 		k5err = gssd_k5_err_msg(context, code);
 		printerr(0, "ERROR: %s while opening credential cache '%s'\n",
@@ -490,6 +528,7 @@ gssd_get_single_krb5_cred(krb5_context context,
 
 /*
  * Given a principal, find a matching ple structure
+ * Called with mutex held
  */
 static struct gssd_k5_kt_princ *
 find_ple_by_princ(krb5_context context, krb5_principal princ)
@@ -506,6 +545,7 @@ find_ple_by_princ(krb5_context context, krb5_principal princ)
 
 /*
  * Create, initialize, and add a new ple structure to the global list
+ * Called with mutex held
  */
 static struct gssd_k5_kt_princ *
 new_ple(krb5_context context, krb5_principal princ)
@@ -557,6 +597,7 @@ new_ple(krb5_context context, krb5_principal princ)
 			p->next = ple;
 	}
 
+	ple->refcount = 1;
 	return ple;
 outerr:
 	if (ple) {
@@ -575,13 +616,14 @@ get_ple_by_princ(krb5_context context, krb5_principal princ)
 {
 	struct gssd_k5_kt_princ *ple;
 
-	/* Need to serialize list if we ever become multi-threaded! */
-
 	pthread_mutex_lock(&ple_lock);
 	ple = find_ple_by_princ(context, princ);
 	if (ple == NULL) {
 		ple = new_ple(context, princ);
 	}
+	if (ple != NULL) {
+		ple->refcount++;
+	}
 	pthread_mutex_unlock(&ple_lock);
 
 	return ple;
@@ -746,6 +788,8 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
 				retval = ENOMEM;
 				k5_free_kt_entry(context, kte);
 			} else {
+				release_ple(context, ple);
+				ple = NULL;
 				retval = 0;
 				*found = 1;
 			}
@@ -1078,6 +1122,93 @@ err_cache:
 	return (*ret_princname && *ret_realm);
 }
 
+/*
+ * Obtain (or refresh if necessary) Kerberos machine credentials
+ * If a ple is passed in, it's reference will be released
+ */
+static int
+gssd_refresh_krb5_machine_credential_internal(char *hostname,
+				     struct gssd_k5_kt_princ *ple,
+				     char *service, char *srchost)
+{
+	krb5_error_code code = 0;
+	krb5_context context;
+	krb5_keytab kt = NULL;;
+	int retval = 0;
+	char *k5err = NULL;
+	const char *svcnames[] = { "$", "root", "nfs", "host", NULL };
+
+	printerr(2, "%s: hostname=%s ple=%p service=%s srchost=%s\n",
+		__func__, hostname, ple, service, srchost);
+
+	/*
+	 * If a specific service name was specified, use it.
+	 * Otherwise, use the default list.
+	 */
+	if (service != NULL && strcmp(service, "*") != 0) {
+		svcnames[0] = service;
+		svcnames[1] = NULL;
+	}
+	if (hostname == NULL && ple == NULL)
+		return EINVAL;
+
+	code = krb5_init_context(&context);
+	if (code) {
+		k5err = gssd_k5_err_msg(NULL, code);
+		printerr(0, "ERROR: %s: %s while initializing krb5 context\n",
+			 __func__, k5err);
+		retval = code;
+		goto out;
+	}
+
+	if ((code = krb5_kt_resolve(context, keytabfile, &kt))) {
+		k5err = gssd_k5_err_msg(context, code);
+		printerr(0, "ERROR: %s: %s while resolving keytab '%s'\n",
+			 __func__, k5err, keytabfile);
+		goto out_free_context;
+	}
+
+	if (ple == NULL) {
+		krb5_keytab_entry kte;
+
+		code = find_keytab_entry(context, kt, srchost, hostname,
+					 &kte, svcnames);
+		if (code) {
+			printerr(0, "ERROR: %s: no usable keytab entry found "
+				 "in keytab %s for connection with host %s\n",
+				 __FUNCTION__, keytabfile, hostname);
+			retval = code;
+			goto out_free_kt;
+		}
+
+		ple = get_ple_by_princ(context, kte.principal);
+		k5_free_kt_entry(context, &kte);
+		if (ple == NULL) {
+			char *pname;
+			if ((krb5_unparse_name(context, kte.principal, &pname))) {
+				pname = NULL;
+			}
+			printerr(0, "ERROR: %s: Could not locate or create "
+				 "ple struct for principal %s for connection "
+				 "with host %s\n",
+				 __FUNCTION__, pname ? pname : "<unparsable>",
+				 hostname);
+			if (pname) k5_free_unparsed_name(context, pname);
+			goto out_free_kt;
+		}
+	}
+	retval = gssd_get_single_krb5_cred(context, kt, ple, 0);
+out_free_kt:
+	krb5_kt_close(context, kt);
+out_free_context:
+	if (ple)
+		release_ple(context, ple);
+	krb5_free_context(context);
+out:
+	free(k5err);
+	return retval;
+}
+
 /*==========================*/
 /*===  External routines ===*/
 /*==========================*/
@@ -1171,37 +1302,56 @@ gssd_get_krb5_machine_cred_list(char ***list)
 		goto out;
 	}
 
-	/* Need to serialize list if we ever become multi-threaded! */
-
+	pthread_mutex_lock(&ple_lock);
 	for (ple = gssd_k5_kt_princ_list; ple; ple = ple->next) {
-		if (ple->ccname) {
-			/* Make sure cred is up-to-date before returning it */
-			retval = gssd_refresh_krb5_machine_credential(NULL, ple,
-								      NULL, NULL);
-			if (retval)
-				continue;
-			if (i + 1 > listsize) {
-				listsize += listinc;
-				l = (char **)
-					realloc(l, listsize * sizeof(char *));
-				if (l == NULL) {
-					retval = ENOMEM;
-					goto out;
-				}
-			}
-			if ((l[i++] = strdup(ple->ccname)) == NULL) {
+		if (!ple->ccname)
+			continue;
+
+		/* Take advantage of the fact we only remove the ple
+		 * from the list during shutdown. If it's modified
+		 * concurrently at worst we'll just miss a new entry
+		 * before the current ple
+		 *
+		 * gssd_refresh_krb5_machine_credential_internal() will
+		 * release the ple refcount
+		 */
+		ple->refcount++;
+		pthread_mutex_unlock(&ple_lock);
+		/* Make sure cred is up-to-date before returning it */
+		retval = gssd_refresh_krb5_machine_credential_internal(NULL, ple,
+								       NULL, NULL);
+		pthread_mutex_lock(&ple_lock);
+		if (gssd_k5_kt_princ_list == NULL) {
+			/* Looks like we did shutdown... abort */
+			l[i] = NULL;
+			gssd_free_krb5_machine_cred_list(l);
+			retval = ENOMEM;
+			goto out_lock;
+		}
+		if (retval)
+			continue;
+		if (i + 1 > listsize) {
+			listsize += listinc;
+			l = (char **)
+				realloc(l, listsize * sizeof(char *));
+			if (l == NULL) {
 				retval = ENOMEM;
-				goto out;
+				goto out_lock;
 			}
 		}
+		if ((l[i++] = strdup(ple->ccname)) == NULL) {
+			retval = ENOMEM;
+			goto out_lock;
+		}
 	}
 	if (i > 0) {
 		l[i] = NULL;
 		*list = l;
 		retval = 0;
-		goto out;
 	} else
 		free((void *)l);
+out_lock:
+	pthread_mutex_unlock(&ple_lock);
   out:
 	return retval;
 }
@@ -1266,10 +1416,7 @@ gssd_destroy_krb5_principals(int destroy_machine_creds)
 			}
 		}
 
-		krb5_free_principal(context, ple->princ);
-		free(ple->ccname);
-		free(ple->realm);
-		free(ple);
+		release_ple(context, ple);
 	}
 	pthread_mutex_unlock(&ple_lock);
 	krb5_free_context(context);
@@ -1280,83 +1427,10 @@ gssd_destroy_krb5_principals(int destroy_machine_creds)
  */
 int
 gssd_refresh_krb5_machine_credential(char *hostname,
-				     struct gssd_k5_kt_princ *ple, 
 				     char *service, char *srchost)
 {
-	krb5_error_code code = 0;
-	krb5_context context;
-	krb5_keytab kt = NULL;;
-	int retval = 0;
-	char *k5err = NULL;
-	const char *svcnames[] = { "$", "root", "nfs", "host", NULL };
-
-	printerr(2, "%s: hostname=%s ple=%p service=%s srchost=%s\n",
-		__func__, hostname, ple, service, srchost);
-
-	/*
-	 * If a specific service name was specified, use it.
-	 * Otherwise, use the default list.
-	 */
-	if (service != NULL && strcmp(service, "*") != 0) {
-		svcnames[0] = service;
-		svcnames[1] = NULL;
-	}
-	if (hostname == NULL && ple == NULL)
-		return EINVAL;
-
-	code = krb5_init_context(&context);
-	if (code) {
-		k5err = gssd_k5_err_msg(NULL, code);
-		printerr(0, "ERROR: %s: %s while initializing krb5 context\n",
-			 __func__, k5err);
-		retval = code;
-		goto out;
-	}
-
-	if ((code = krb5_kt_resolve(context, keytabfile, &kt))) {
-		k5err = gssd_k5_err_msg(context, code);
-		printerr(0, "ERROR: %s: %s while resolving keytab '%s'\n",
-			 __func__, k5err, keytabfile);
-		goto out_free_context;
-	}
-
-	if (ple == NULL) {
-		krb5_keytab_entry kte;
-
-		code = find_keytab_entry(context, kt, srchost, hostname,
-					 &kte, svcnames);
-		if (code) {
-			printerr(0, "ERROR: %s: no usable keytab entry found "
-				 "in keytab %s for connection with host %s\n",
-				 __FUNCTION__, keytabfile, hostname);
-			retval = code;
-			goto out_free_kt;
-		}
-
-		ple = get_ple_by_princ(context, kte.principal);
-		k5_free_kt_entry(context, &kte);
-		if (ple == NULL) {
-			char *pname;
-			if ((krb5_unparse_name(context, kte.principal, &pname))) {
-				pname = NULL;
-			}
-			printerr(0, "ERROR: %s: Could not locate or create "
-				 "ple struct for principal %s for connection "
-				 "with host %s\n",
-				 __FUNCTION__, pname ? pname : "<unparsable>",
-				 hostname);
-			if (pname) k5_free_unparsed_name(context, pname);
-			goto out_free_kt;
-		}
-	}
-	retval = gssd_get_single_krb5_cred(context, kt, ple, 0);
-out_free_kt:
-	krb5_kt_close(context, kt);
-out_free_context:
-	krb5_free_context(context);
-out:
-	free(k5err);
-	return retval;
+    return gssd_refresh_krb5_machine_credential_internal(hostname, NULL,
+							 service, srchost);
 }
 
 /*
diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
index e127cc84..2415205a 100644
--- a/utils/gssd/krb5_util.h
+++ b/utils/gssd/krb5_util.h
@@ -9,19 +9,6 @@
 #include "gss_oids.h"
 #endif
 
-/*
- * List of principals from our keytab that we
- * will try to use to obtain credentials
- * (known as a principal list entry (ple))
- */
-struct gssd_k5_kt_princ {
-	struct gssd_k5_kt_princ *next;
-	krb5_principal princ;
-	char *ccname;
-	char *realm;
-	krb5_timestamp endtime;
-};
-
 
 int gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername,
 				     char *dirname);
@@ -29,7 +16,6 @@ int  gssd_get_krb5_machine_cred_list(char ***list);
 void gssd_free_krb5_machine_cred_list(char **list);
 void gssd_destroy_krb5_principals(int destroy_machine_creds);
 int  gssd_refresh_krb5_machine_credential(char *hostname,
-					  struct gssd_k5_kt_princ *ple, 
 					  char *service, char *srchost);
 char *gssd_k5_err_msg(krb5_context context, krb5_error_code code);
 void gssd_k5_get_default_realm(char **def_realm);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 06/10] gssd: Add a few debug statements to help track client_info lifetimes.
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (4 preceding siblings ...)
  2020-07-01 18:27 ` [PATCH 05/10] gssd: Fix locking for machine principal list Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-01 18:27 ` [PATCH 07/10] gssd: Lookup local hostname when srchost is '*' Doug Nazar
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/gssd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 54d9b5de..7ad38b6f 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -520,6 +520,8 @@ gssd_get_clnt(struct topdir *tdi, const char *name)
 		if (!strcmp(clp->name, name))
 			return clp;
 
+	printerr(3, "creating client %s/%s\n", tdi->name, name);
+
 	clp = calloc(1, sizeof(struct clnt_info));
 	if (!clp) {
 		printerr(0, "ERROR: can't malloc clnt_info: %s\n",
@@ -561,6 +563,8 @@ gssd_scan_clnt(struct clnt_info *clp)
 	bool gssd_was_closed;
 	bool krb5_was_closed;
 
+	printerr(3, "scanning client %s\n", clp->relpath);
+
 	gssd_was_closed = clp->gssd_fd < 0 ? true : false;
 	krb5_was_closed = clp->krb5_fd < 0 ? true : false;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 07/10] gssd: Lookup local hostname when srchost is '*'
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (5 preceding siblings ...)
  2020-07-01 18:27 ` [PATCH 06/10] gssd: Add a few debug statements to help track client_info lifetimes Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-01 18:27 ` [PATCH 08/10] gssd: We never use the nocache param of gssd_check_if_cc_exists() Doug Nazar
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Currently when we receive a '*' srchost, we scan our keytab for a matching
host but of course none match. We then fall back to scanning for any
service/realm match and eventually find our hostname. Let's lookup our
hostname instead and quickly find our specific match.

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/krb5_util.c | 52 ++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 7908c10f..560e4a87 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -757,6 +757,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
 		goto out;
 	}
 
+	printerr(4, "Scanning keytab for %s/*@%s\n", service, realm);
 	while ((code = krb5_kt_next_entry(context, kt, kte, &cursor)) == 0) {
 		if ((code = krb5_unparse_name(context, kte->principal,
 					      &pname))) {
@@ -853,43 +854,44 @@ find_keytab_entry(krb5_context context, krb5_keytab kt,
 		goto out;
 
 	/* Get full local hostname */
-	if (srchost) {
+	if (srchost && strcmp(srchost, "*") != 0) {
 		strcpy(myhostname, srchost);
-	} else if (gethostname(myhostname, sizeof(myhostname)) == -1) {
-		retval = errno;
-		k5err = gssd_k5_err_msg(context, retval);
-		printerr(1, "%s while getting local hostname\n", k5err);
-		goto out;
+	        strcpy(myhostad, myhostname);
+	} else {
+		/* Borrow myhostad for gethostname(), we need it later anyways */
+		if (gethostname(myhostad, sizeof(myhostad)-1) == -1) {
+			retval = errno;
+			k5err = gssd_k5_err_msg(context, retval);
+			printerr(1, "%s while getting local hostname\n", k5err);
+			goto out;
+		}
+		retval = get_full_hostname(myhostad, myhostname, sizeof(myhostname));
+		if (retval) {
+			/* Don't use myhostname */
+			myhostname[0] = 0;
+		}
 	}
 
 	/* Compute the active directory machine name HOST$ */
-	krb5_appdefault_string(context, "nfs", NULL, "ad_principal_name", 
+	krb5_appdefault_string(context, "nfs", NULL, "ad_principal_name",
 		notsetstr, &adhostoverride);
 	if (strcmp(adhostoverride, notsetstr) != 0) {
-	        printerr (1, 
-				"AD host string overridden with \"%s\" from appdefaults\n", 
+		printerr (1,
+				"AD host string overridden with \"%s\" from appdefaults\n",
 				adhostoverride);
-	        /* No overflow: Windows cannot handle strings longer than 19 chars */
-	        strcpy(myhostad, adhostoverride);
+		/* No overflow: Windows cannot handle strings longer than 19 chars */
+		strcpy(myhostad, adhostoverride);
 	} else {
-	        strcpy(myhostad, myhostname);
-	        for (i = 0; myhostad[i] != 0; ++i) {
-	          if (myhostad[i] == '.') break;
-	        }
-	        myhostad[i] = '$';
-	        myhostad[i+1] = 0;
+		/* In this case, it's been pre-filled above */
+		for (i = 0; myhostad[i] != 0; ++i) {
+			if (myhostad[i] == '.') break;
+		}
+		myhostad[i] = '$';
+		myhostad[i+1] = 0;
 	}
 	if (adhostoverride)
 		krb5_free_string(context, adhostoverride);
 
-	if (!srchost) {
-		retval = get_full_hostname(myhostname, myhostname, sizeof(myhostname));
-		if (retval) {
-			/* Don't use myhostname */
-			myhostname[0] = 0;
-		}
-	}
-
 	code = krb5_get_default_realm(context, &default_realm);
 	if (code) {
 		retval = code;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 08/10] gssd: We never use the nocache param of gssd_check_if_cc_exists()
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (6 preceding siblings ...)
  2020-07-01 18:27 ` [PATCH 07/10] gssd: Lookup local hostname when srchost is '*' Doug Nazar
@ 2020-07-01 18:27 ` Doug Nazar
  2020-07-01 18:28 ` [PATCH 09/10] Cleanup printf format attribute handling and fix format strings Doug Nazar
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:27 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/krb5_util.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 560e4a87..e5b81823 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -165,7 +165,7 @@ static int select_krb5_ccache(const struct dirent *d);
 static int gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 		const char **cctype, struct dirent **d);
 static int gssd_get_single_krb5_cred(krb5_context context,
-		krb5_keytab kt, struct gssd_k5_kt_princ *ple, int nocache);
+		krb5_keytab kt, struct gssd_k5_kt_princ *ple);
 static int query_krb5_ccache(const char* cred_cache, char **ret_princname,
 		char **ret_realm);
 
@@ -380,8 +380,7 @@ gssd_check_if_cc_exists(struct gssd_k5_kt_princ *ple)
 static int
 gssd_get_single_krb5_cred(krb5_context context,
 			  krb5_keytab kt,
-			  struct gssd_k5_kt_princ *ple,
-			  int nocache)
+			  struct gssd_k5_kt_princ *ple)
 {
 #ifdef HAVE_KRB5_GET_INIT_CREDS_OPT_SET_ADDRESSLESS
 	krb5_get_init_creds_opt *init_opts = NULL;
@@ -398,10 +397,11 @@ gssd_get_single_krb5_cred(krb5_context context,
 	char *cache_type;
 	char *pname = NULL;
 	char *k5err = NULL;
+	int nocache = 0;
 
 	memset(&my_creds, 0, sizeof(my_creds));
 
-	if (!nocache && !use_memcache)
+	if (!use_memcache)
 		nocache = gssd_check_if_cc_exists(ple);
 	/*
 	 * Workaround for clock skew among NFS server, NFS client and KDC
@@ -1199,7 +1199,7 @@ gssd_refresh_krb5_machine_credential_internal(char *hostname,
 			goto out_free_kt;
 		}
 	}
-	retval = gssd_get_single_krb5_cred(context, kt, ple, 0);
+	retval = gssd_get_single_krb5_cred(context, kt, ple);
 out_free_kt:
 	krb5_kt_close(context, kt);
 out_free_context:
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/10] Cleanup printf format attribute handling and fix format strings
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (7 preceding siblings ...)
  2020-07-01 18:27 ` [PATCH 08/10] gssd: We never use the nocache param of gssd_check_if_cc_exists() Doug Nazar
@ 2020-07-01 18:28 ` Doug Nazar
  2020-07-01 18:28 ` [PATCH 09/10] Cleanup printf format attribute handling and fix various " Doug Nazar
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:28 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 support/include/compiler.h         | 14 ++++++++++++++
 support/include/xcommon.h          | 10 +---------
 support/include/xlog.h             | 20 ++++++--------------
 support/nfs/xcommon.c              |  2 ++
 support/nfsidmap/gums.c            |  2 ++
 support/nfsidmap/libnfsidmap.c     |  8 +++++---
 support/nfsidmap/nfsidmap.h        | 10 +++++++++-
 support/nfsidmap/nfsidmap_common.c |  1 +
 support/nfsidmap/nss.c             |  4 +++-
 support/nfsidmap/regex.c           |  6 ++++--
 support/nfsidmap/static.c          |  1 +
 support/nfsidmap/umich_ldap.c      | 10 ++++++----
 tools/locktest/testlk.c            |  6 ++++--
 utils/exportfs/exportfs.c          |  5 ++---
 utils/gssd/err_util.h              |  4 +++-
 utils/gssd/gss_names.c             |  9 +++++----
 utils/gssd/gss_util.c              |  2 +-
 utils/gssd/gssd_proc.c             |  8 ++++----
 utils/gssd/krb5_util.c             | 12 ++++++++----
 utils/gssd/svcgssd.c               |  4 +++-
 utils/gssd/svcgssd_proc.c          |  9 +++++----
 utils/idmapd/idmapd.c              |  3 ++-
 utils/nfsidmap/nfsidmap.c          |  3 ++-
 23 files changed, 93 insertions(+), 60 deletions(-)
 create mode 100644 support/include/compiler.h

diff --git a/support/include/compiler.h b/support/include/compiler.h
new file mode 100644
index 00000000..950c1ecd
--- /dev/null
+++ b/support/include/compiler.h
@@ -0,0 +1,14 @@
+#ifndef COMPILER_H
+#define COMPILER_H
+
+#ifndef CONFIG_H
+#include <config.h>
+#endif
+
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+#define X_FORMAT(_x) __attribute__((__format__ _x))
+#else
+#define X_FORMAT(_x)
+#endif
+
+#endif
diff --git a/support/include/xcommon.h b/support/include/xcommon.h
index 30b0403b..0001e609 100644
--- a/support/include/xcommon.h
+++ b/support/include/xcommon.h
@@ -9,9 +9,7 @@
 #ifndef _XMALLOC_H
 #define _MALLOC_H
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
+#include "compiler.h"
 
 #include <sys/types.h>
 #include <fcntl.h>
@@ -29,12 +27,6 @@
 
 #define streq(s, t)	(strcmp ((s), (t)) == 0)
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-#define X_FORMAT(_x) __attribute__((__format__ _x))
-#else
-#define X_FORMAT(_x)
-#endif
-
 /* Functions in sundries.c that are used in mount.c and umount.c  */
 char *canonicalize (const char *path);
 void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
diff --git a/support/include/xlog.h b/support/include/xlog.h
index 32ff5a1b..b79fe641 100644
--- a/support/include/xlog.h
+++ b/support/include/xlog.h
@@ -7,9 +7,7 @@
 #ifndef XLOG_H
 #define XLOG_H
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
+#include "compiler.h"
 
 #include <stdarg.h>
 
@@ -39,12 +37,6 @@ struct xlog_debugfac {
 	int		df_fac;
 };
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
-#else
-#define XLOG_FORMAT(_x)
-#endif
-
 extern int export_errno;
 void			xlog_open(char *progname);
 void			xlog_stderr(int on);
@@ -53,10 +45,10 @@ void			xlog_config(int fac, int on);
 void			xlog_sconfig(char *, int on);
 void			xlog_from_conffile(char *);
 int			xlog_enabled(int fac);
-void			xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
-void			xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
-void			xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
-void			xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
-void			xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
+void			xlog(int fac, const char *fmt, ...) X_FORMAT((printf, 2, 3));
+void			xlog_warn(const char *fmt, ...) X_FORMAT((printf, 1, 2));
+void			xlog_err(const char *fmt, ...) X_FORMAT((printf, 1, 2));
+void			xlog_errno(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
+void			xlog_backend(int fac, const char *fmt, va_list args) X_FORMAT((printf, 2, 0));
 
 #endif /* XLOG_H */
diff --git a/support/nfs/xcommon.c b/support/nfs/xcommon.c
index 3989f0bc..5974974d 100644
--- a/support/nfs/xcommon.c
+++ b/support/nfs/xcommon.c
@@ -98,7 +98,9 @@ nfs_error (const char *fmt, ...) {
 
      fmt2 = xstrconcat2 (fmt, "\n");
      va_start (args, fmt);
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
      vfprintf (stderr, fmt2, args);
+#pragma GCC diagnostic warning "-Wformat-nonliteral"
      va_end (args);
      free (fmt2);
 }
diff --git a/support/nfsidmap/gums.c b/support/nfsidmap/gums.c
index 1d6eb318..e46f24b7 100644
--- a/support/nfsidmap/gums.c
+++ b/support/nfsidmap/gums.c
@@ -41,6 +41,8 @@
 #include <grp.h>
 #include <err.h>
 #include <syslog.h>
+
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 
diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
index bce448cf..e1b52918 100644
--- a/support/nfsidmap/libnfsidmap.c
+++ b/support/nfsidmap/libnfsidmap.c
@@ -57,6 +57,7 @@
 #include <arpa/nameser.h>
 #include <arpa/nameser_compat.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_private.h"
 #include "nfsidmap_plugin.h"
@@ -94,6 +95,7 @@ gid_t nobody_gid = (gid_t)-1;
 #endif
 
 /* Default logging fuction */
+X_FORMAT((__printf__, 1, 2))
 static void default_logger(const char *fmt, ...)
 {
 	va_list vp;
@@ -258,7 +260,7 @@ static int load_translation_plugin(char *method, struct mapping_plugin *plgn)
 	}
 	trans = init_func();
 	if (trans == NULL) {
-		IDMAP_LOG(1, ("libnfsidmap: Failed to initialize plugin %s",
+		IDMAP_LOG(1, ("libnfsidmap: %s failed to initialize plugin %s",
 			  PLUGIN_INIT_FUNC, plgname));
 		dlclose(dl);
 		return -1;
@@ -451,7 +453,7 @@ int nfs4_init_name_mapping(char *conffile)
 					nobody_user, strerror(errno)));
 			free(buf);
 		} else
-			IDMAP_LOG(0,("libnfsidmap: Nobody-User: no memory : %s", 
+			IDMAP_LOG(0,("libnfsidmap: Nobody-User (%s): no memory : %s",
 					nobody_user, strerror(errno)));
 	}
 
@@ -472,7 +474,7 @@ int nfs4_init_name_mapping(char *conffile)
 					nobody_group, strerror(errno)));
 			free(buf);
 		} else
-			IDMAP_LOG(0,("libnfsidmap: Nobody-Group: no memory : %s", 
+			IDMAP_LOG(0,("libnfsidmap: Nobody-Group (%s): no memory : %s",
 					nobody_group, strerror(errno)));
 	}
 
diff --git a/support/nfsidmap/nfsidmap.h b/support/nfsidmap/nfsidmap.h
index 10630654..dcd42eda 100644
--- a/support/nfsidmap/nfsidmap.h
+++ b/support/nfsidmap/nfsidmap.h
@@ -35,6 +35,14 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef X_FORMAT
+# ifdef __GNUC__
+#  define X_FORMAT(_x) __attribute__((__format__ _x))
+# else
+#  define X_FORMAT(_x)
+# endif
+#endif
+
 /* XXX arbitrary */
 #define NFS4_MAX_DOMAIN_LEN 512
 typedef enum {
@@ -47,7 +55,7 @@ typedef struct _extra_mapping_params {
 	int content_len;
 } extra_mapping_params;
 
-typedef void (*nfs4_idmap_log_function_t)(const char *, ...);
+typedef void (* nfs4_idmap_log_function_t)(const char *, ...) X_FORMAT((printf, 1, 2));
 
 int nfs4_init_name_mapping(char *conffile);
 int nfs4_get_default_domain(char *server, char *domain, size_t len);
diff --git a/support/nfsidmap/nfsidmap_common.c b/support/nfsidmap/nfsidmap_common.c
index f89b82ee..66390af2 100644
--- a/support/nfsidmap/nfsidmap_common.c
+++ b/support/nfsidmap/nfsidmap_common.c
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <string.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_private.h"
 #include "nfsidmap_plugin.h"
diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c
index 9d46499c..f422a9f8 100644
--- a/support/nfsidmap/nss.c
+++ b/support/nfsidmap/nss.c
@@ -47,10 +47,12 @@
 #include <grp.h>
 #include <limits.h>
 #include <ctype.h>
+#include <syslog.h>
+
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 #include "nfsidmap_private.h"
-#include <syslog.h>
 
 static char *get_default_domain(void)
 {
diff --git a/support/nfsidmap/regex.c b/support/nfsidmap/regex.c
index fdbb2e2f..a2e13148 100644
--- a/support/nfsidmap/regex.c
+++ b/support/nfsidmap/regex.c
@@ -44,6 +44,7 @@
 #include <err.h>
 #include <regex.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 
@@ -229,8 +230,9 @@ static struct group *regex_getgrnam(const char *name, const char *UNUSED(domain)
 
 		if (err)
 		{
-			IDMAP_LOG(4, ("regexp_getgrnam: removing prefix '%s' (%d long) from group '%s'", group_name_prefix, group_name_prefix_length, localgroup));
-				groupname += group_name_prefix_length;
+			IDMAP_LOG(4, ("regexp_getgrnam: removing prefix '%s' (%zd long) from group '%s'",
+				      group_name_prefix, group_name_prefix_length, localgroup));
+			groupname += group_name_prefix_length;
 		}
 		else
 		{
diff --git a/support/nfsidmap/static.c b/support/nfsidmap/static.c
index 8ac4a398..a6e1e275 100644
--- a/support/nfsidmap/static.c
+++ b/support/nfsidmap/static.c
@@ -41,6 +41,7 @@
 #include <errno.h>
 #include <err.h>
 
+#include "compiler.h"
 #include "conffile.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
diff --git a/support/nfsidmap/umich_ldap.c b/support/nfsidmap/umich_ldap.c
index c475d379..2954b61c 100644
--- a/support/nfsidmap/umich_ldap.c
+++ b/support/nfsidmap/umich_ldap.c
@@ -56,6 +56,8 @@
 /* We are using deprecated functions, get the prototypes... */
 #define LDAP_DEPRECATED 1
 #include <ldap.h>
+
+#include "compiler.h"
 #include "nfslib.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
@@ -795,8 +797,8 @@ umich_id_to_name(uid_t id, int idtype, char **name, size_t len,
 	 */
 	if (strlen(names[0]) >= len) {
 		/* not enough space to return the name */
-		IDMAP_LOG(1, ("umich_id_to_name: output buffer size (%d) "
-			  "too small to return string, '%s', of length %d",
+		IDMAP_LOG(1, ("umich_id_to_name: output buffer size (%zd) "
+			  "too small to return string, '%s', of length %zd",
 			  len, names[0], strlen(names[0])));
 		goto out_memfree;
 	}
@@ -1307,7 +1309,7 @@ get_canonical_hostname(const char *inname)
 			     sizeof(tmphost), NULL, 0, 0);
 	if (error) {
 		IDMAP_LOG(1, ("%s: getnameinfo for host '%s' failed (%d)",
-			  __FUNCTION__, inname));
+			  __FUNCTION__, inname, error));
 		goto out_free;
 	}
 	return_name = strdup (tmphost);
@@ -1356,7 +1358,7 @@ umichldap_init(void)
 			ldap_info.tls_reqcert = LDAP_OPT_X_TLS_NEVER;
 		else {
 			IDMAP_LOG(0, ("umichldap_init: Invalid value(%s) for "
-				      "LDAP_tls_reqcert."));
+				      "LDAP_tls_reqcert.", cert_req));
 			goto fail;
 		}
 	}
diff --git a/tools/locktest/testlk.c b/tools/locktest/testlk.c
index ea51f788..5dbc39f1 100644
--- a/tools/locktest/testlk.c
+++ b/tools/locktest/testlk.c
@@ -2,6 +2,7 @@
 #include <config.h>
 #endif
 
+#include <inttypes.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
@@ -81,8 +82,9 @@ main(int argc, char **argv)
 		if (fl.l_type == F_UNLCK) {
 			printf("%s: no conflicting lock\n", fname);
 		} else {
-			printf("%s: conflicting lock by %d on (%zd;%zd)\n",
-				fname, fl.l_pid, fl.l_start, fl.l_len);
+			printf("%s: conflicting lock by %d on "
+			       "(%" PRId64 ";%" PRId64 ")\n",
+			       fname, fl.l_pid, fl.l_start, fl.l_len);
 		}
 		return 0;
 	}
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index a04a7898..6f5243f2 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -38,6 +38,7 @@
 #include "exportfs.h"
 #include "xlog.h"
 #include "conffile.h"
+#include "compiler.h"
 
 static void	export_all(int verbose);
 static void	exportfs(char *arg, char *options, int verbose);
@@ -651,9 +652,7 @@ out:
 	return result;
 }
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-__attribute__((format (printf, 2, 3)))
-#endif
+X_FORMAT((printf, 2, 3))
 static char
 dumpopt(char c, char *fmt, ...)
 {
diff --git a/utils/gssd/err_util.h b/utils/gssd/err_util.h
index c4df32da..b4f5f40e 100644
--- a/utils/gssd/err_util.h
+++ b/utils/gssd/err_util.h
@@ -31,8 +31,10 @@
 #ifndef _ERR_UTIL_H_
 #define _ERR_UTIL_H_
 
+#include "compiler.h"
+
 void initerr(char *progname, int verbosity, int fg);
-void printerr(int priority, char *format, ...);
+void printerr(int priority, char *format, ...) X_FORMAT((printf, 2, 3));
 int get_verbosity(void);
 
 #endif /* _ERR_UTIL_H_ */
diff --git a/utils/gssd/gss_names.c b/utils/gssd/gss_names.c
index 2a7f3a13..5d2f2987 100644
--- a/utils/gssd/gss_names.c
+++ b/utils/gssd/gss_names.c
@@ -45,10 +45,11 @@
 #include <string.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <nfsidmap.h>
-#include <nfslib.h>
 #include <time.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
+#include "nfslib.h"
 #include "svcgssd.h"
 #include "gss_util.h"
 #include "gss_names.h"
@@ -65,7 +66,7 @@ get_krb5_hostbased_name(gss_buffer_desc *name, char **hostbased_name)
 	if (strchr(name->value, '@') && strchr(name->value, '/')) {
 		if ((sname = calloc(name->length, 1)) == NULL) {
 			printerr(0, "ERROR: get_krb5_hostbased_name failed "
-				 "to allocate %d bytes\n", name->length);
+				 "to allocate %zu bytes\n", name->length);
 			return -1;
 		}
 		/* read in name and instance and replace '/' with '@' */
@@ -102,7 +103,7 @@ get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
 	}
 	if (name.length >= 0xffff) {	    /* don't overflow */
 		printerr(0, "ERROR: get_hostbased_client_name: "
-			 "received gss_name is too long (%d bytes)\n",
+			 "received gss_name is too long (%zu bytes)\n",
 			 name.length);
 		goto out_rel_buf;
 	}
diff --git a/utils/gssd/gss_util.c b/utils/gssd/gss_util.c
index 2e6d40f0..e1714fa0 100644
--- a/utils/gssd/gss_util.c
+++ b/utils/gssd/gss_util.c
@@ -304,7 +304,7 @@ gssd_acquire_cred(char *server_name, const gss_OID oid)
 				target_name, &pbuf, NULL);
 		if (ignore_maj_stat == GSS_S_COMPLETE) {
 			printerr(1, "Unable to obtain credentials for '%.*s'\n",
-				 pbuf.length, pbuf.value);
+				 (int)pbuf.length, (char*)pbuf.value);
 			ignore_maj_stat = gss_release_buffer(&ignore_min_stat,
 							     &pbuf);
 		}
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index addac318..342bf693 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -151,7 +151,7 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 	unsigned int buf_size = 0;
 
 	printerr(2, "doing downcall: lifetime_rec=%u acceptor=%.*s\n",
-		lifetime_rec, acceptor->length, acceptor->value);
+		lifetime_rec, (int)acceptor->length, (char*)acceptor->value);
 	buf_size = sizeof(uid) + sizeof(timeout) + sizeof(pd->pd_seq_win) +
 		sizeof(pd->pd_ctx_hndl.length) + pd->pd_ctx_hndl.length +
 		sizeof(context_token->length) + context_token->length +
@@ -266,13 +266,13 @@ populate_port(struct sockaddr *sa, const socklen_t salen,
 
 	port = nfs_getport(sa, salen, program, version, protocol);
 	if (!port) {
-		printerr(0, "ERROR: unable to obtain port for prog %ld "
-			    "vers %ld\n", program, version);
+		printerr(0, "ERROR: unable to obtain port for prog %u "
+			    "vers %u\n", program, version);
 		return 0;
 	}
 
 set_port:
-	printerr(2, "DEBUG: setting port to %hu for prog %lu vers %lu\n", port,
+	printerr(2, "DEBUG: setting port to %hu for prog %u vers %u\n", port,
 		 program, version);
 
 	switch (sa->sa_family) {
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index e5b81823..cb959425 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -293,7 +293,7 @@ gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 				score++;
 
 			printerr(3, "CC '%s'(%s@%s) passed all checks and"
-				    " has mtime of %u\n",
+				    " has mtime of %ld\n",
 				 buf, princname, realm, 
 				 tmp_stat.st_mtime);
 			/*
@@ -330,8 +330,8 @@ gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 				}
 				printerr(3, "CC '%s:%s/%s' is our "
 					    "current best match "
-					    "with mtime of %u\n",
-					 cctype, dirname,
+					    "with mtime of %lu\n",
+					 *cctype, dirname,
 					 best_match_dir->d_name,
 					 best_match_stat.st_mtime);
 			}
@@ -1260,8 +1260,12 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)
 	if (err)
 		return err;
 
-	snprintf(buf, sizeof(buf), "%s:%s/%s", cctype, dirname, d->d_name);
+	err = snprintf(buf, sizeof(buf), "%s:%s/%s", cctype, dirname, d->d_name);
 	free(d);
+	if (err < 0) {
+		printerr(0, "WARNING: buffer to small for krb5 ccache");
+		return -ENOMEM;
+	}
 
 	printerr(2, "using %s as credentials cache for client with "
 		    "uid %u for server %s\n", buf, uid, servername);
diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index ec49b616..cde4ffc9 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -56,7 +56,9 @@
 #include <stdlib.h>
 #include <string.h>
 #include <signal.h>
-#include <nfsidmap.h>
+
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "nfslib.h"
 #include "svcgssd.h"
 #include "gss_util.h"
diff --git a/utils/gssd/svcgssd_proc.c b/utils/gssd/svcgssd_proc.c
index 72ec2540..5c0e46a8 100644
--- a/utils/gssd/svcgssd_proc.c
+++ b/utils/gssd/svcgssd_proc.c
@@ -48,10 +48,11 @@
 #include <string.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <nfsidmap.h>
 #include <nfslib.h>
 #include <time.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "svcgssd.h"
 #include "gss_util.h"
 #include "err_util.h"
@@ -102,7 +103,7 @@ do_svc_downcall(gss_buffer_desc *out_handle, struct svc_cred *cred,
 	qword_addint(&bp, &blen, cred->cr_uid);
 	qword_addint(&bp, &blen, cred->cr_gid);
 	qword_addint(&bp, &blen, cred->cr_ngroups);
-	printerr(2, "mech: %s, hndl len: %d, ctx len %d, timeout: %d (%d from now), "
+	printerr(2, "mech: %s, hndl len: %zd, ctx len %zd, timeout: %d (%ld from now), "
 		 "clnt: %s, uid: %d, gid: %d, num aux grps: %d:\n",
 		 fname, out_handle->length, context_token->length,
 		 endtime, endtime - time(0),
@@ -232,7 +233,7 @@ get_ids(gss_name_t client_name, gss_OID mech, struct svc_cred *cred)
 	}
 	if (name.length >= 0xffff || /* be certain name.length+1 doesn't overflow */
 	    !(sname = calloc(name.length + 1, 1))) {
-		printerr(0, "WARNING: get_ids: error allocating %d bytes "
+		printerr(0, "WARNING: get_ids: error allocating %zd bytes "
 			"for sname\n", name.length + 1);
 		gss_release_buffer(&min_stat, &name);
 		goto out;
@@ -373,7 +374,7 @@ handle_nullreq(int f) {
 	if (in_handle.length != 0) { /* CONTINUE_INIT case */
 		if (in_handle.length != sizeof(ctx)) {
 			printerr(0, "WARNING: handle_nullreq: "
-				    "input handle has unexpected length %d\n",
+				    "input handle has unexpected length %zd\n",
 				    in_handle.length);
 			goto out_err;
 		}
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 12648f67..fa6d920a 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -65,8 +65,9 @@
 #include <limits.h>
 #include <ctype.h>
 #include <libgen.h>
-#include <nfsidmap.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "xlog.h"
 #include "conffile.h"
 #include "queue.h"
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index cf7f65e9..792f832c 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -10,9 +10,10 @@
 #include <pwd.h>
 #include <grp.h>
 #include <keyutils.h>
-#include <nfsidmap.h>
 
 #include <unistd.h>
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "xlog.h"
 #include "conffile.h"
 #include "xcommon.h"
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/10] Cleanup printf format attribute handling and fix various format strings
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (8 preceding siblings ...)
  2020-07-01 18:28 ` [PATCH 09/10] Cleanup printf format attribute handling and fix format strings Doug Nazar
@ 2020-07-01 18:28 ` Doug Nazar
  2020-07-01 18:28 ` [PATCH 09/10] Consolidate " Doug Nazar
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:28 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 support/include/compiler.h         | 14 ++++++++++++++
 support/include/xcommon.h          | 10 +---------
 support/include/xlog.h             | 20 ++++++--------------
 support/nfs/xcommon.c              |  2 ++
 support/nfsidmap/gums.c            |  2 ++
 support/nfsidmap/libnfsidmap.c     |  8 +++++---
 support/nfsidmap/nfsidmap.h        | 10 +++++++++-
 support/nfsidmap/nfsidmap_common.c |  1 +
 support/nfsidmap/nss.c             |  4 +++-
 support/nfsidmap/regex.c           |  6 ++++--
 support/nfsidmap/static.c          |  1 +
 support/nfsidmap/umich_ldap.c      | 10 ++++++----
 tools/locktest/testlk.c            |  6 ++++--
 utils/exportfs/exportfs.c          |  5 ++---
 utils/gssd/err_util.h              |  4 +++-
 utils/gssd/gss_names.c             |  9 +++++----
 utils/gssd/gss_util.c              |  2 +-
 utils/gssd/gssd_proc.c             |  8 ++++----
 utils/gssd/krb5_util.c             | 12 ++++++++----
 utils/gssd/svcgssd.c               |  4 +++-
 utils/gssd/svcgssd_proc.c          |  9 +++++----
 utils/idmapd/idmapd.c              |  3 ++-
 utils/nfsidmap/nfsidmap.c          |  3 ++-
 23 files changed, 93 insertions(+), 60 deletions(-)
 create mode 100644 support/include/compiler.h

diff --git a/support/include/compiler.h b/support/include/compiler.h
new file mode 100644
index 00000000..950c1ecd
--- /dev/null
+++ b/support/include/compiler.h
@@ -0,0 +1,14 @@
+#ifndef COMPILER_H
+#define COMPILER_H
+
+#ifndef CONFIG_H
+#include <config.h>
+#endif
+
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+#define X_FORMAT(_x) __attribute__((__format__ _x))
+#else
+#define X_FORMAT(_x)
+#endif
+
+#endif
diff --git a/support/include/xcommon.h b/support/include/xcommon.h
index 30b0403b..0001e609 100644
--- a/support/include/xcommon.h
+++ b/support/include/xcommon.h
@@ -9,9 +9,7 @@
 #ifndef _XMALLOC_H
 #define _MALLOC_H
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
+#include "compiler.h"
 
 #include <sys/types.h>
 #include <fcntl.h>
@@ -29,12 +27,6 @@
 
 #define streq(s, t)	(strcmp ((s), (t)) == 0)
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-#define X_FORMAT(_x) __attribute__((__format__ _x))
-#else
-#define X_FORMAT(_x)
-#endif
-
 /* Functions in sundries.c that are used in mount.c and umount.c  */
 char *canonicalize (const char *path);
 void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
diff --git a/support/include/xlog.h b/support/include/xlog.h
index 32ff5a1b..b79fe641 100644
--- a/support/include/xlog.h
+++ b/support/include/xlog.h
@@ -7,9 +7,7 @@
 #ifndef XLOG_H
 #define XLOG_H
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
+#include "compiler.h"
 
 #include <stdarg.h>
 
@@ -39,12 +37,6 @@ struct xlog_debugfac {
 	int		df_fac;
 };
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
-#else
-#define XLOG_FORMAT(_x)
-#endif
-
 extern int export_errno;
 void			xlog_open(char *progname);
 void			xlog_stderr(int on);
@@ -53,10 +45,10 @@ void			xlog_config(int fac, int on);
 void			xlog_sconfig(char *, int on);
 void			xlog_from_conffile(char *);
 int			xlog_enabled(int fac);
-void			xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
-void			xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
-void			xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
-void			xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
-void			xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
+void			xlog(int fac, const char *fmt, ...) X_FORMAT((printf, 2, 3));
+void			xlog_warn(const char *fmt, ...) X_FORMAT((printf, 1, 2));
+void			xlog_err(const char *fmt, ...) X_FORMAT((printf, 1, 2));
+void			xlog_errno(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
+void			xlog_backend(int fac, const char *fmt, va_list args) X_FORMAT((printf, 2, 0));
 
 #endif /* XLOG_H */
diff --git a/support/nfs/xcommon.c b/support/nfs/xcommon.c
index 3989f0bc..5974974d 100644
--- a/support/nfs/xcommon.c
+++ b/support/nfs/xcommon.c
@@ -98,7 +98,9 @@ nfs_error (const char *fmt, ...) {
 
      fmt2 = xstrconcat2 (fmt, "\n");
      va_start (args, fmt);
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
      vfprintf (stderr, fmt2, args);
+#pragma GCC diagnostic warning "-Wformat-nonliteral"
      va_end (args);
      free (fmt2);
 }
diff --git a/support/nfsidmap/gums.c b/support/nfsidmap/gums.c
index 1d6eb318..e46f24b7 100644
--- a/support/nfsidmap/gums.c
+++ b/support/nfsidmap/gums.c
@@ -41,6 +41,8 @@
 #include <grp.h>
 #include <err.h>
 #include <syslog.h>
+
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 
diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
index bce448cf..e1b52918 100644
--- a/support/nfsidmap/libnfsidmap.c
+++ b/support/nfsidmap/libnfsidmap.c
@@ -57,6 +57,7 @@
 #include <arpa/nameser.h>
 #include <arpa/nameser_compat.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_private.h"
 #include "nfsidmap_plugin.h"
@@ -94,6 +95,7 @@ gid_t nobody_gid = (gid_t)-1;
 #endif
 
 /* Default logging fuction */
+X_FORMAT((__printf__, 1, 2))
 static void default_logger(const char *fmt, ...)
 {
 	va_list vp;
@@ -258,7 +260,7 @@ static int load_translation_plugin(char *method, struct mapping_plugin *plgn)
 	}
 	trans = init_func();
 	if (trans == NULL) {
-		IDMAP_LOG(1, ("libnfsidmap: Failed to initialize plugin %s",
+		IDMAP_LOG(1, ("libnfsidmap: %s failed to initialize plugin %s",
 			  PLUGIN_INIT_FUNC, plgname));
 		dlclose(dl);
 		return -1;
@@ -451,7 +453,7 @@ int nfs4_init_name_mapping(char *conffile)
 					nobody_user, strerror(errno)));
 			free(buf);
 		} else
-			IDMAP_LOG(0,("libnfsidmap: Nobody-User: no memory : %s", 
+			IDMAP_LOG(0,("libnfsidmap: Nobody-User (%s): no memory : %s",
 					nobody_user, strerror(errno)));
 	}
 
@@ -472,7 +474,7 @@ int nfs4_init_name_mapping(char *conffile)
 					nobody_group, strerror(errno)));
 			free(buf);
 		} else
-			IDMAP_LOG(0,("libnfsidmap: Nobody-Group: no memory : %s", 
+			IDMAP_LOG(0,("libnfsidmap: Nobody-Group (%s): no memory : %s",
 					nobody_group, strerror(errno)));
 	}
 
diff --git a/support/nfsidmap/nfsidmap.h b/support/nfsidmap/nfsidmap.h
index 10630654..dcd42eda 100644
--- a/support/nfsidmap/nfsidmap.h
+++ b/support/nfsidmap/nfsidmap.h
@@ -35,6 +35,14 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef X_FORMAT
+# ifdef __GNUC__
+#  define X_FORMAT(_x) __attribute__((__format__ _x))
+# else
+#  define X_FORMAT(_x)
+# endif
+#endif
+
 /* XXX arbitrary */
 #define NFS4_MAX_DOMAIN_LEN 512
 typedef enum {
@@ -47,7 +55,7 @@ typedef struct _extra_mapping_params {
 	int content_len;
 } extra_mapping_params;
 
-typedef void (*nfs4_idmap_log_function_t)(const char *, ...);
+typedef void (* nfs4_idmap_log_function_t)(const char *, ...) X_FORMAT((printf, 1, 2));
 
 int nfs4_init_name_mapping(char *conffile);
 int nfs4_get_default_domain(char *server, char *domain, size_t len);
diff --git a/support/nfsidmap/nfsidmap_common.c b/support/nfsidmap/nfsidmap_common.c
index f89b82ee..66390af2 100644
--- a/support/nfsidmap/nfsidmap_common.c
+++ b/support/nfsidmap/nfsidmap_common.c
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <string.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_private.h"
 #include "nfsidmap_plugin.h"
diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c
index 9d46499c..f422a9f8 100644
--- a/support/nfsidmap/nss.c
+++ b/support/nfsidmap/nss.c
@@ -47,10 +47,12 @@
 #include <grp.h>
 #include <limits.h>
 #include <ctype.h>
+#include <syslog.h>
+
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 #include "nfsidmap_private.h"
-#include <syslog.h>
 
 static char *get_default_domain(void)
 {
diff --git a/support/nfsidmap/regex.c b/support/nfsidmap/regex.c
index fdbb2e2f..a2e13148 100644
--- a/support/nfsidmap/regex.c
+++ b/support/nfsidmap/regex.c
@@ -44,6 +44,7 @@
 #include <err.h>
 #include <regex.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 
@@ -229,8 +230,9 @@ static struct group *regex_getgrnam(const char *name, const char *UNUSED(domain)
 
 		if (err)
 		{
-			IDMAP_LOG(4, ("regexp_getgrnam: removing prefix '%s' (%d long) from group '%s'", group_name_prefix, group_name_prefix_length, localgroup));
-				groupname += group_name_prefix_length;
+			IDMAP_LOG(4, ("regexp_getgrnam: removing prefix '%s' (%zd long) from group '%s'",
+				      group_name_prefix, group_name_prefix_length, localgroup));
+			groupname += group_name_prefix_length;
 		}
 		else
 		{
diff --git a/support/nfsidmap/static.c b/support/nfsidmap/static.c
index 8ac4a398..a6e1e275 100644
--- a/support/nfsidmap/static.c
+++ b/support/nfsidmap/static.c
@@ -41,6 +41,7 @@
 #include <errno.h>
 #include <err.h>
 
+#include "compiler.h"
 #include "conffile.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
diff --git a/support/nfsidmap/umich_ldap.c b/support/nfsidmap/umich_ldap.c
index c475d379..2954b61c 100644
--- a/support/nfsidmap/umich_ldap.c
+++ b/support/nfsidmap/umich_ldap.c
@@ -56,6 +56,8 @@
 /* We are using deprecated functions, get the prototypes... */
 #define LDAP_DEPRECATED 1
 #include <ldap.h>
+
+#include "compiler.h"
 #include "nfslib.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
@@ -795,8 +797,8 @@ umich_id_to_name(uid_t id, int idtype, char **name, size_t len,
 	 */
 	if (strlen(names[0]) >= len) {
 		/* not enough space to return the name */
-		IDMAP_LOG(1, ("umich_id_to_name: output buffer size (%d) "
-			  "too small to return string, '%s', of length %d",
+		IDMAP_LOG(1, ("umich_id_to_name: output buffer size (%zd) "
+			  "too small to return string, '%s', of length %zd",
 			  len, names[0], strlen(names[0])));
 		goto out_memfree;
 	}
@@ -1307,7 +1309,7 @@ get_canonical_hostname(const char *inname)
 			     sizeof(tmphost), NULL, 0, 0);
 	if (error) {
 		IDMAP_LOG(1, ("%s: getnameinfo for host '%s' failed (%d)",
-			  __FUNCTION__, inname));
+			  __FUNCTION__, inname, error));
 		goto out_free;
 	}
 	return_name = strdup (tmphost);
@@ -1356,7 +1358,7 @@ umichldap_init(void)
 			ldap_info.tls_reqcert = LDAP_OPT_X_TLS_NEVER;
 		else {
 			IDMAP_LOG(0, ("umichldap_init: Invalid value(%s) for "
-				      "LDAP_tls_reqcert."));
+				      "LDAP_tls_reqcert.", cert_req));
 			goto fail;
 		}
 	}
diff --git a/tools/locktest/testlk.c b/tools/locktest/testlk.c
index ea51f788..5dbc39f1 100644
--- a/tools/locktest/testlk.c
+++ b/tools/locktest/testlk.c
@@ -2,6 +2,7 @@
 #include <config.h>
 #endif
 
+#include <inttypes.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
@@ -81,8 +82,9 @@ main(int argc, char **argv)
 		if (fl.l_type == F_UNLCK) {
 			printf("%s: no conflicting lock\n", fname);
 		} else {
-			printf("%s: conflicting lock by %d on (%zd;%zd)\n",
-				fname, fl.l_pid, fl.l_start, fl.l_len);
+			printf("%s: conflicting lock by %d on "
+			       "(%" PRId64 ";%" PRId64 ")\n",
+			       fname, fl.l_pid, fl.l_start, fl.l_len);
 		}
 		return 0;
 	}
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index a04a7898..6f5243f2 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -38,6 +38,7 @@
 #include "exportfs.h"
 #include "xlog.h"
 #include "conffile.h"
+#include "compiler.h"
 
 static void	export_all(int verbose);
 static void	exportfs(char *arg, char *options, int verbose);
@@ -651,9 +652,7 @@ out:
 	return result;
 }
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-__attribute__((format (printf, 2, 3)))
-#endif
+X_FORMAT((printf, 2, 3))
 static char
 dumpopt(char c, char *fmt, ...)
 {
diff --git a/utils/gssd/err_util.h b/utils/gssd/err_util.h
index c4df32da..b4f5f40e 100644
--- a/utils/gssd/err_util.h
+++ b/utils/gssd/err_util.h
@@ -31,8 +31,10 @@
 #ifndef _ERR_UTIL_H_
 #define _ERR_UTIL_H_
 
+#include "compiler.h"
+
 void initerr(char *progname, int verbosity, int fg);
-void printerr(int priority, char *format, ...);
+void printerr(int priority, char *format, ...) X_FORMAT((printf, 2, 3));
 int get_verbosity(void);
 
 #endif /* _ERR_UTIL_H_ */
diff --git a/utils/gssd/gss_names.c b/utils/gssd/gss_names.c
index 2a7f3a13..5d2f2987 100644
--- a/utils/gssd/gss_names.c
+++ b/utils/gssd/gss_names.c
@@ -45,10 +45,11 @@
 #include <string.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <nfsidmap.h>
-#include <nfslib.h>
 #include <time.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
+#include "nfslib.h"
 #include "svcgssd.h"
 #include "gss_util.h"
 #include "gss_names.h"
@@ -65,7 +66,7 @@ get_krb5_hostbased_name(gss_buffer_desc *name, char **hostbased_name)
 	if (strchr(name->value, '@') && strchr(name->value, '/')) {
 		if ((sname = calloc(name->length, 1)) == NULL) {
 			printerr(0, "ERROR: get_krb5_hostbased_name failed "
-				 "to allocate %d bytes\n", name->length);
+				 "to allocate %zu bytes\n", name->length);
 			return -1;
 		}
 		/* read in name and instance and replace '/' with '@' */
@@ -102,7 +103,7 @@ get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
 	}
 	if (name.length >= 0xffff) {	    /* don't overflow */
 		printerr(0, "ERROR: get_hostbased_client_name: "
-			 "received gss_name is too long (%d bytes)\n",
+			 "received gss_name is too long (%zu bytes)\n",
 			 name.length);
 		goto out_rel_buf;
 	}
diff --git a/utils/gssd/gss_util.c b/utils/gssd/gss_util.c
index 2e6d40f0..e1714fa0 100644
--- a/utils/gssd/gss_util.c
+++ b/utils/gssd/gss_util.c
@@ -304,7 +304,7 @@ gssd_acquire_cred(char *server_name, const gss_OID oid)
 				target_name, &pbuf, NULL);
 		if (ignore_maj_stat == GSS_S_COMPLETE) {
 			printerr(1, "Unable to obtain credentials for '%.*s'\n",
-				 pbuf.length, pbuf.value);
+				 (int)pbuf.length, (char*)pbuf.value);
 			ignore_maj_stat = gss_release_buffer(&ignore_min_stat,
 							     &pbuf);
 		}
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index addac318..342bf693 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -151,7 +151,7 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 	unsigned int buf_size = 0;
 
 	printerr(2, "doing downcall: lifetime_rec=%u acceptor=%.*s\n",
-		lifetime_rec, acceptor->length, acceptor->value);
+		lifetime_rec, (int)acceptor->length, (char*)acceptor->value);
 	buf_size = sizeof(uid) + sizeof(timeout) + sizeof(pd->pd_seq_win) +
 		sizeof(pd->pd_ctx_hndl.length) + pd->pd_ctx_hndl.length +
 		sizeof(context_token->length) + context_token->length +
@@ -266,13 +266,13 @@ populate_port(struct sockaddr *sa, const socklen_t salen,
 
 	port = nfs_getport(sa, salen, program, version, protocol);
 	if (!port) {
-		printerr(0, "ERROR: unable to obtain port for prog %ld "
-			    "vers %ld\n", program, version);
+		printerr(0, "ERROR: unable to obtain port for prog %u "
+			    "vers %u\n", program, version);
 		return 0;
 	}
 
 set_port:
-	printerr(2, "DEBUG: setting port to %hu for prog %lu vers %lu\n", port,
+	printerr(2, "DEBUG: setting port to %hu for prog %u vers %u\n", port,
 		 program, version);
 
 	switch (sa->sa_family) {
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index e5b81823..cb959425 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -293,7 +293,7 @@ gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 				score++;
 
 			printerr(3, "CC '%s'(%s@%s) passed all checks and"
-				    " has mtime of %u\n",
+				    " has mtime of %ld\n",
 				 buf, princname, realm, 
 				 tmp_stat.st_mtime);
 			/*
@@ -330,8 +330,8 @@ gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 				}
 				printerr(3, "CC '%s:%s/%s' is our "
 					    "current best match "
-					    "with mtime of %u\n",
-					 cctype, dirname,
+					    "with mtime of %lu\n",
+					 *cctype, dirname,
 					 best_match_dir->d_name,
 					 best_match_stat.st_mtime);
 			}
@@ -1260,8 +1260,12 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)
 	if (err)
 		return err;
 
-	snprintf(buf, sizeof(buf), "%s:%s/%s", cctype, dirname, d->d_name);
+	err = snprintf(buf, sizeof(buf), "%s:%s/%s", cctype, dirname, d->d_name);
 	free(d);
+	if (err < 0) {
+		printerr(0, "WARNING: buffer to small for krb5 ccache");
+		return -ENOMEM;
+	}
 
 	printerr(2, "using %s as credentials cache for client with "
 		    "uid %u for server %s\n", buf, uid, servername);
diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index ec49b616..cde4ffc9 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -56,7 +56,9 @@
 #include <stdlib.h>
 #include <string.h>
 #include <signal.h>
-#include <nfsidmap.h>
+
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "nfslib.h"
 #include "svcgssd.h"
 #include "gss_util.h"
diff --git a/utils/gssd/svcgssd_proc.c b/utils/gssd/svcgssd_proc.c
index 72ec2540..5c0e46a8 100644
--- a/utils/gssd/svcgssd_proc.c
+++ b/utils/gssd/svcgssd_proc.c
@@ -48,10 +48,11 @@
 #include <string.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <nfsidmap.h>
 #include <nfslib.h>
 #include <time.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "svcgssd.h"
 #include "gss_util.h"
 #include "err_util.h"
@@ -102,7 +103,7 @@ do_svc_downcall(gss_buffer_desc *out_handle, struct svc_cred *cred,
 	qword_addint(&bp, &blen, cred->cr_uid);
 	qword_addint(&bp, &blen, cred->cr_gid);
 	qword_addint(&bp, &blen, cred->cr_ngroups);
-	printerr(2, "mech: %s, hndl len: %d, ctx len %d, timeout: %d (%d from now), "
+	printerr(2, "mech: %s, hndl len: %zd, ctx len %zd, timeout: %d (%ld from now), "
 		 "clnt: %s, uid: %d, gid: %d, num aux grps: %d:\n",
 		 fname, out_handle->length, context_token->length,
 		 endtime, endtime - time(0),
@@ -232,7 +233,7 @@ get_ids(gss_name_t client_name, gss_OID mech, struct svc_cred *cred)
 	}
 	if (name.length >= 0xffff || /* be certain name.length+1 doesn't overflow */
 	    !(sname = calloc(name.length + 1, 1))) {
-		printerr(0, "WARNING: get_ids: error allocating %d bytes "
+		printerr(0, "WARNING: get_ids: error allocating %zd bytes "
 			"for sname\n", name.length + 1);
 		gss_release_buffer(&min_stat, &name);
 		goto out;
@@ -373,7 +374,7 @@ handle_nullreq(int f) {
 	if (in_handle.length != 0) { /* CONTINUE_INIT case */
 		if (in_handle.length != sizeof(ctx)) {
 			printerr(0, "WARNING: handle_nullreq: "
-				    "input handle has unexpected length %d\n",
+				    "input handle has unexpected length %zd\n",
 				    in_handle.length);
 			goto out_err;
 		}
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 12648f67..fa6d920a 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -65,8 +65,9 @@
 #include <limits.h>
 #include <ctype.h>
 #include <libgen.h>
-#include <nfsidmap.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "xlog.h"
 #include "conffile.h"
 #include "queue.h"
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index cf7f65e9..792f832c 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -10,9 +10,10 @@
 #include <pwd.h>
 #include <grp.h>
 #include <keyutils.h>
-#include <nfsidmap.h>
 
 #include <unistd.h>
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "xlog.h"
 #include "conffile.h"
 #include "xcommon.h"
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/10] Consolidate printf format attribute handling and fix various format strings
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (9 preceding siblings ...)
  2020-07-01 18:28 ` [PATCH 09/10] Cleanup printf format attribute handling and fix various " Doug Nazar
@ 2020-07-01 18:28 ` Doug Nazar
  2020-07-01 18:28 ` [PATCH 10/10] Fix various clang warnings Doug Nazar
  2020-07-14 18:38 ` [PATCH 00/10] Misc fixes & cleanups for nfs-utils Steve Dickson
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:28 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 support/include/compiler.h         | 14 ++++++++++++++
 support/include/xcommon.h          | 10 +---------
 support/include/xlog.h             | 20 ++++++--------------
 support/nfs/xcommon.c              |  2 ++
 support/nfsidmap/gums.c            |  2 ++
 support/nfsidmap/libnfsidmap.c     |  8 +++++---
 support/nfsidmap/nfsidmap.h        | 10 +++++++++-
 support/nfsidmap/nfsidmap_common.c |  1 +
 support/nfsidmap/nss.c             |  4 +++-
 support/nfsidmap/regex.c           |  6 ++++--
 support/nfsidmap/static.c          |  1 +
 support/nfsidmap/umich_ldap.c      | 10 ++++++----
 tools/locktest/testlk.c            |  6 ++++--
 utils/exportfs/exportfs.c          |  5 ++---
 utils/gssd/err_util.h              |  4 +++-
 utils/gssd/gss_names.c             |  9 +++++----
 utils/gssd/gss_util.c              |  2 +-
 utils/gssd/gssd_proc.c             |  8 ++++----
 utils/gssd/krb5_util.c             | 12 ++++++++----
 utils/gssd/svcgssd.c               |  4 +++-
 utils/gssd/svcgssd_proc.c          |  9 +++++----
 utils/idmapd/idmapd.c              |  3 ++-
 utils/nfsidmap/nfsidmap.c          |  3 ++-
 23 files changed, 93 insertions(+), 60 deletions(-)
 create mode 100644 support/include/compiler.h

diff --git a/support/include/compiler.h b/support/include/compiler.h
new file mode 100644
index 00000000..950c1ecd
--- /dev/null
+++ b/support/include/compiler.h
@@ -0,0 +1,14 @@
+#ifndef COMPILER_H
+#define COMPILER_H
+
+#ifndef CONFIG_H
+#include <config.h>
+#endif
+
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+#define X_FORMAT(_x) __attribute__((__format__ _x))
+#else
+#define X_FORMAT(_x)
+#endif
+
+#endif
diff --git a/support/include/xcommon.h b/support/include/xcommon.h
index 30b0403b..0001e609 100644
--- a/support/include/xcommon.h
+++ b/support/include/xcommon.h
@@ -9,9 +9,7 @@
 #ifndef _XMALLOC_H
 #define _MALLOC_H
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
+#include "compiler.h"
 
 #include <sys/types.h>
 #include <fcntl.h>
@@ -29,12 +27,6 @@
 
 #define streq(s, t)	(strcmp ((s), (t)) == 0)
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-#define X_FORMAT(_x) __attribute__((__format__ _x))
-#else
-#define X_FORMAT(_x)
-#endif
-
 /* Functions in sundries.c that are used in mount.c and umount.c  */
 char *canonicalize (const char *path);
 void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
diff --git a/support/include/xlog.h b/support/include/xlog.h
index 32ff5a1b..b79fe641 100644
--- a/support/include/xlog.h
+++ b/support/include/xlog.h
@@ -7,9 +7,7 @@
 #ifndef XLOG_H
 #define XLOG_H
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
+#include "compiler.h"
 
 #include <stdarg.h>
 
@@ -39,12 +37,6 @@ struct xlog_debugfac {
 	int		df_fac;
 };
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
-#else
-#define XLOG_FORMAT(_x)
-#endif
-
 extern int export_errno;
 void			xlog_open(char *progname);
 void			xlog_stderr(int on);
@@ -53,10 +45,10 @@ void			xlog_config(int fac, int on);
 void			xlog_sconfig(char *, int on);
 void			xlog_from_conffile(char *);
 int			xlog_enabled(int fac);
-void			xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
-void			xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
-void			xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
-void			xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
-void			xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
+void			xlog(int fac, const char *fmt, ...) X_FORMAT((printf, 2, 3));
+void			xlog_warn(const char *fmt, ...) X_FORMAT((printf, 1, 2));
+void			xlog_err(const char *fmt, ...) X_FORMAT((printf, 1, 2));
+void			xlog_errno(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
+void			xlog_backend(int fac, const char *fmt, va_list args) X_FORMAT((printf, 2, 0));
 
 #endif /* XLOG_H */
diff --git a/support/nfs/xcommon.c b/support/nfs/xcommon.c
index 3989f0bc..5974974d 100644
--- a/support/nfs/xcommon.c
+++ b/support/nfs/xcommon.c
@@ -98,7 +98,9 @@ nfs_error (const char *fmt, ...) {
 
      fmt2 = xstrconcat2 (fmt, "\n");
      va_start (args, fmt);
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
      vfprintf (stderr, fmt2, args);
+#pragma GCC diagnostic warning "-Wformat-nonliteral"
      va_end (args);
      free (fmt2);
 }
diff --git a/support/nfsidmap/gums.c b/support/nfsidmap/gums.c
index 1d6eb318..e46f24b7 100644
--- a/support/nfsidmap/gums.c
+++ b/support/nfsidmap/gums.c
@@ -41,6 +41,8 @@
 #include <grp.h>
 #include <err.h>
 #include <syslog.h>
+
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 
diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
index bce448cf..e1b52918 100644
--- a/support/nfsidmap/libnfsidmap.c
+++ b/support/nfsidmap/libnfsidmap.c
@@ -57,6 +57,7 @@
 #include <arpa/nameser.h>
 #include <arpa/nameser_compat.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_private.h"
 #include "nfsidmap_plugin.h"
@@ -94,6 +95,7 @@ gid_t nobody_gid = (gid_t)-1;
 #endif
 
 /* Default logging fuction */
+X_FORMAT((__printf__, 1, 2))
 static void default_logger(const char *fmt, ...)
 {
 	va_list vp;
@@ -258,7 +260,7 @@ static int load_translation_plugin(char *method, struct mapping_plugin *plgn)
 	}
 	trans = init_func();
 	if (trans == NULL) {
-		IDMAP_LOG(1, ("libnfsidmap: Failed to initialize plugin %s",
+		IDMAP_LOG(1, ("libnfsidmap: %s failed to initialize plugin %s",
 			  PLUGIN_INIT_FUNC, plgname));
 		dlclose(dl);
 		return -1;
@@ -451,7 +453,7 @@ int nfs4_init_name_mapping(char *conffile)
 					nobody_user, strerror(errno)));
 			free(buf);
 		} else
-			IDMAP_LOG(0,("libnfsidmap: Nobody-User: no memory : %s", 
+			IDMAP_LOG(0,("libnfsidmap: Nobody-User (%s): no memory : %s",
 					nobody_user, strerror(errno)));
 	}
 
@@ -472,7 +474,7 @@ int nfs4_init_name_mapping(char *conffile)
 					nobody_group, strerror(errno)));
 			free(buf);
 		} else
-			IDMAP_LOG(0,("libnfsidmap: Nobody-Group: no memory : %s", 
+			IDMAP_LOG(0,("libnfsidmap: Nobody-Group (%s): no memory : %s",
 					nobody_group, strerror(errno)));
 	}
 
diff --git a/support/nfsidmap/nfsidmap.h b/support/nfsidmap/nfsidmap.h
index 10630654..dcd42eda 100644
--- a/support/nfsidmap/nfsidmap.h
+++ b/support/nfsidmap/nfsidmap.h
@@ -35,6 +35,14 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef X_FORMAT
+# ifdef __GNUC__
+#  define X_FORMAT(_x) __attribute__((__format__ _x))
+# else
+#  define X_FORMAT(_x)
+# endif
+#endif
+
 /* XXX arbitrary */
 #define NFS4_MAX_DOMAIN_LEN 512
 typedef enum {
@@ -47,7 +55,7 @@ typedef struct _extra_mapping_params {
 	int content_len;
 } extra_mapping_params;
 
-typedef void (*nfs4_idmap_log_function_t)(const char *, ...);
+typedef void (* nfs4_idmap_log_function_t)(const char *, ...) X_FORMAT((printf, 1, 2));
 
 int nfs4_init_name_mapping(char *conffile);
 int nfs4_get_default_domain(char *server, char *domain, size_t len);
diff --git a/support/nfsidmap/nfsidmap_common.c b/support/nfsidmap/nfsidmap_common.c
index f89b82ee..66390af2 100644
--- a/support/nfsidmap/nfsidmap_common.c
+++ b/support/nfsidmap/nfsidmap_common.c
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <string.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_private.h"
 #include "nfsidmap_plugin.h"
diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c
index 9d46499c..f422a9f8 100644
--- a/support/nfsidmap/nss.c
+++ b/support/nfsidmap/nss.c
@@ -47,10 +47,12 @@
 #include <grp.h>
 #include <limits.h>
 #include <ctype.h>
+#include <syslog.h>
+
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 #include "nfsidmap_private.h"
-#include <syslog.h>
 
 static char *get_default_domain(void)
 {
diff --git a/support/nfsidmap/regex.c b/support/nfsidmap/regex.c
index fdbb2e2f..a2e13148 100644
--- a/support/nfsidmap/regex.c
+++ b/support/nfsidmap/regex.c
@@ -44,6 +44,7 @@
 #include <err.h>
 #include <regex.h>
 
+#include "compiler.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
 
@@ -229,8 +230,9 @@ static struct group *regex_getgrnam(const char *name, const char *UNUSED(domain)
 
 		if (err)
 		{
-			IDMAP_LOG(4, ("regexp_getgrnam: removing prefix '%s' (%d long) from group '%s'", group_name_prefix, group_name_prefix_length, localgroup));
-				groupname += group_name_prefix_length;
+			IDMAP_LOG(4, ("regexp_getgrnam: removing prefix '%s' (%zd long) from group '%s'",
+				      group_name_prefix, group_name_prefix_length, localgroup));
+			groupname += group_name_prefix_length;
 		}
 		else
 		{
diff --git a/support/nfsidmap/static.c b/support/nfsidmap/static.c
index 8ac4a398..a6e1e275 100644
--- a/support/nfsidmap/static.c
+++ b/support/nfsidmap/static.c
@@ -41,6 +41,7 @@
 #include <errno.h>
 #include <err.h>
 
+#include "compiler.h"
 #include "conffile.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
diff --git a/support/nfsidmap/umich_ldap.c b/support/nfsidmap/umich_ldap.c
index c475d379..2954b61c 100644
--- a/support/nfsidmap/umich_ldap.c
+++ b/support/nfsidmap/umich_ldap.c
@@ -56,6 +56,8 @@
 /* We are using deprecated functions, get the prototypes... */
 #define LDAP_DEPRECATED 1
 #include <ldap.h>
+
+#include "compiler.h"
 #include "nfslib.h"
 #include "nfsidmap.h"
 #include "nfsidmap_plugin.h"
@@ -795,8 +797,8 @@ umich_id_to_name(uid_t id, int idtype, char **name, size_t len,
 	 */
 	if (strlen(names[0]) >= len) {
 		/* not enough space to return the name */
-		IDMAP_LOG(1, ("umich_id_to_name: output buffer size (%d) "
-			  "too small to return string, '%s', of length %d",
+		IDMAP_LOG(1, ("umich_id_to_name: output buffer size (%zd) "
+			  "too small to return string, '%s', of length %zd",
 			  len, names[0], strlen(names[0])));
 		goto out_memfree;
 	}
@@ -1307,7 +1309,7 @@ get_canonical_hostname(const char *inname)
 			     sizeof(tmphost), NULL, 0, 0);
 	if (error) {
 		IDMAP_LOG(1, ("%s: getnameinfo for host '%s' failed (%d)",
-			  __FUNCTION__, inname));
+			  __FUNCTION__, inname, error));
 		goto out_free;
 	}
 	return_name = strdup (tmphost);
@@ -1356,7 +1358,7 @@ umichldap_init(void)
 			ldap_info.tls_reqcert = LDAP_OPT_X_TLS_NEVER;
 		else {
 			IDMAP_LOG(0, ("umichldap_init: Invalid value(%s) for "
-				      "LDAP_tls_reqcert."));
+				      "LDAP_tls_reqcert.", cert_req));
 			goto fail;
 		}
 	}
diff --git a/tools/locktest/testlk.c b/tools/locktest/testlk.c
index ea51f788..5dbc39f1 100644
--- a/tools/locktest/testlk.c
+++ b/tools/locktest/testlk.c
@@ -2,6 +2,7 @@
 #include <config.h>
 #endif
 
+#include <inttypes.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
@@ -81,8 +82,9 @@ main(int argc, char **argv)
 		if (fl.l_type == F_UNLCK) {
 			printf("%s: no conflicting lock\n", fname);
 		} else {
-			printf("%s: conflicting lock by %d on (%zd;%zd)\n",
-				fname, fl.l_pid, fl.l_start, fl.l_len);
+			printf("%s: conflicting lock by %d on "
+			       "(%" PRId64 ";%" PRId64 ")\n",
+			       fname, fl.l_pid, fl.l_start, fl.l_len);
 		}
 		return 0;
 	}
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index a04a7898..6f5243f2 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -38,6 +38,7 @@
 #include "exportfs.h"
 #include "xlog.h"
 #include "conffile.h"
+#include "compiler.h"
 
 static void	export_all(int verbose);
 static void	exportfs(char *arg, char *options, int verbose);
@@ -651,9 +652,7 @@ out:
 	return result;
 }
 
-#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
-__attribute__((format (printf, 2, 3)))
-#endif
+X_FORMAT((printf, 2, 3))
 static char
 dumpopt(char c, char *fmt, ...)
 {
diff --git a/utils/gssd/err_util.h b/utils/gssd/err_util.h
index c4df32da..b4f5f40e 100644
--- a/utils/gssd/err_util.h
+++ b/utils/gssd/err_util.h
@@ -31,8 +31,10 @@
 #ifndef _ERR_UTIL_H_
 #define _ERR_UTIL_H_
 
+#include "compiler.h"
+
 void initerr(char *progname, int verbosity, int fg);
-void printerr(int priority, char *format, ...);
+void printerr(int priority, char *format, ...) X_FORMAT((printf, 2, 3));
 int get_verbosity(void);
 
 #endif /* _ERR_UTIL_H_ */
diff --git a/utils/gssd/gss_names.c b/utils/gssd/gss_names.c
index 2a7f3a13..5d2f2987 100644
--- a/utils/gssd/gss_names.c
+++ b/utils/gssd/gss_names.c
@@ -45,10 +45,11 @@
 #include <string.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <nfsidmap.h>
-#include <nfslib.h>
 #include <time.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
+#include "nfslib.h"
 #include "svcgssd.h"
 #include "gss_util.h"
 #include "gss_names.h"
@@ -65,7 +66,7 @@ get_krb5_hostbased_name(gss_buffer_desc *name, char **hostbased_name)
 	if (strchr(name->value, '@') && strchr(name->value, '/')) {
 		if ((sname = calloc(name->length, 1)) == NULL) {
 			printerr(0, "ERROR: get_krb5_hostbased_name failed "
-				 "to allocate %d bytes\n", name->length);
+				 "to allocate %zu bytes\n", name->length);
 			return -1;
 		}
 		/* read in name and instance and replace '/' with '@' */
@@ -102,7 +103,7 @@ get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
 	}
 	if (name.length >= 0xffff) {	    /* don't overflow */
 		printerr(0, "ERROR: get_hostbased_client_name: "
-			 "received gss_name is too long (%d bytes)\n",
+			 "received gss_name is too long (%zu bytes)\n",
 			 name.length);
 		goto out_rel_buf;
 	}
diff --git a/utils/gssd/gss_util.c b/utils/gssd/gss_util.c
index 2e6d40f0..e1714fa0 100644
--- a/utils/gssd/gss_util.c
+++ b/utils/gssd/gss_util.c
@@ -304,7 +304,7 @@ gssd_acquire_cred(char *server_name, const gss_OID oid)
 				target_name, &pbuf, NULL);
 		if (ignore_maj_stat == GSS_S_COMPLETE) {
 			printerr(1, "Unable to obtain credentials for '%.*s'\n",
-				 pbuf.length, pbuf.value);
+				 (int)pbuf.length, (char*)pbuf.value);
 			ignore_maj_stat = gss_release_buffer(&ignore_min_stat,
 							     &pbuf);
 		}
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index addac318..342bf693 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -151,7 +151,7 @@ do_downcall(int k5_fd, uid_t uid, struct authgss_private_data *pd,
 	unsigned int buf_size = 0;
 
 	printerr(2, "doing downcall: lifetime_rec=%u acceptor=%.*s\n",
-		lifetime_rec, acceptor->length, acceptor->value);
+		lifetime_rec, (int)acceptor->length, (char*)acceptor->value);
 	buf_size = sizeof(uid) + sizeof(timeout) + sizeof(pd->pd_seq_win) +
 		sizeof(pd->pd_ctx_hndl.length) + pd->pd_ctx_hndl.length +
 		sizeof(context_token->length) + context_token->length +
@@ -266,13 +266,13 @@ populate_port(struct sockaddr *sa, const socklen_t salen,
 
 	port = nfs_getport(sa, salen, program, version, protocol);
 	if (!port) {
-		printerr(0, "ERROR: unable to obtain port for prog %ld "
-			    "vers %ld\n", program, version);
+		printerr(0, "ERROR: unable to obtain port for prog %u "
+			    "vers %u\n", program, version);
 		return 0;
 	}
 
 set_port:
-	printerr(2, "DEBUG: setting port to %hu for prog %lu vers %lu\n", port,
+	printerr(2, "DEBUG: setting port to %hu for prog %u vers %u\n", port,
 		 program, version);
 
 	switch (sa->sa_family) {
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index e5b81823..cb959425 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -293,7 +293,7 @@ gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 				score++;
 
 			printerr(3, "CC '%s'(%s@%s) passed all checks and"
-				    " has mtime of %u\n",
+				    " has mtime of %ld\n",
 				 buf, princname, realm, 
 				 tmp_stat.st_mtime);
 			/*
@@ -330,8 +330,8 @@ gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 				}
 				printerr(3, "CC '%s:%s/%s' is our "
 					    "current best match "
-					    "with mtime of %u\n",
-					 cctype, dirname,
+					    "with mtime of %lu\n",
+					 *cctype, dirname,
 					 best_match_dir->d_name,
 					 best_match_stat.st_mtime);
 			}
@@ -1260,8 +1260,12 @@ gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)
 	if (err)
 		return err;
 
-	snprintf(buf, sizeof(buf), "%s:%s/%s", cctype, dirname, d->d_name);
+	err = snprintf(buf, sizeof(buf), "%s:%s/%s", cctype, dirname, d->d_name);
 	free(d);
+	if (err < 0) {
+		printerr(0, "WARNING: buffer to small for krb5 ccache");
+		return -ENOMEM;
+	}
 
 	printerr(2, "using %s as credentials cache for client with "
 		    "uid %u for server %s\n", buf, uid, servername);
diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index ec49b616..cde4ffc9 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -56,7 +56,9 @@
 #include <stdlib.h>
 #include <string.h>
 #include <signal.h>
-#include <nfsidmap.h>
+
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "nfslib.h"
 #include "svcgssd.h"
 #include "gss_util.h"
diff --git a/utils/gssd/svcgssd_proc.c b/utils/gssd/svcgssd_proc.c
index 72ec2540..5c0e46a8 100644
--- a/utils/gssd/svcgssd_proc.c
+++ b/utils/gssd/svcgssd_proc.c
@@ -48,10 +48,11 @@
 #include <string.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <nfsidmap.h>
 #include <nfslib.h>
 #include <time.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "svcgssd.h"
 #include "gss_util.h"
 #include "err_util.h"
@@ -102,7 +103,7 @@ do_svc_downcall(gss_buffer_desc *out_handle, struct svc_cred *cred,
 	qword_addint(&bp, &blen, cred->cr_uid);
 	qword_addint(&bp, &blen, cred->cr_gid);
 	qword_addint(&bp, &blen, cred->cr_ngroups);
-	printerr(2, "mech: %s, hndl len: %d, ctx len %d, timeout: %d (%d from now), "
+	printerr(2, "mech: %s, hndl len: %zd, ctx len %zd, timeout: %d (%ld from now), "
 		 "clnt: %s, uid: %d, gid: %d, num aux grps: %d:\n",
 		 fname, out_handle->length, context_token->length,
 		 endtime, endtime - time(0),
@@ -232,7 +233,7 @@ get_ids(gss_name_t client_name, gss_OID mech, struct svc_cred *cred)
 	}
 	if (name.length >= 0xffff || /* be certain name.length+1 doesn't overflow */
 	    !(sname = calloc(name.length + 1, 1))) {
-		printerr(0, "WARNING: get_ids: error allocating %d bytes "
+		printerr(0, "WARNING: get_ids: error allocating %zd bytes "
 			"for sname\n", name.length + 1);
 		gss_release_buffer(&min_stat, &name);
 		goto out;
@@ -373,7 +374,7 @@ handle_nullreq(int f) {
 	if (in_handle.length != 0) { /* CONTINUE_INIT case */
 		if (in_handle.length != sizeof(ctx)) {
 			printerr(0, "WARNING: handle_nullreq: "
-				    "input handle has unexpected length %d\n",
+				    "input handle has unexpected length %zd\n",
 				    in_handle.length);
 			goto out_err;
 		}
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 12648f67..fa6d920a 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -65,8 +65,9 @@
 #include <limits.h>
 #include <ctype.h>
 #include <libgen.h>
-#include <nfsidmap.h>
 
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "xlog.h"
 #include "conffile.h"
 #include "queue.h"
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index cf7f65e9..792f832c 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -10,9 +10,10 @@
 #include <pwd.h>
 #include <grp.h>
 #include <keyutils.h>
-#include <nfsidmap.h>
 
 #include <unistd.h>
+#include "compiler.h"
+#include "nfsidmap.h"
 #include "xlog.h"
 #include "conffile.h"
 #include "xcommon.h"
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 10/10] Fix various clang warnings.
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (10 preceding siblings ...)
  2020-07-01 18:28 ` [PATCH 09/10] Consolidate " Doug Nazar
@ 2020-07-01 18:28 ` Doug Nazar
  2020-07-14 18:38 ` [PATCH 00/10] Misc fixes & cleanups for nfs-utils Steve Dickson
  12 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-01 18:28 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 configure.ac              | 6 +++---
 support/include/xcommon.h | 2 +-
 utils/mount/network.c     | 4 ++--
 utils/mount/stropts.c     | 2 --
 utils/mountd/cache.c      | 2 +-
 5 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 942f3c05..dbb795f0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -638,13 +638,12 @@ my_am_cflags="\
  -Werror=parentheses \
  -Werror=aggregate-return \
  -Werror=unused-result \
- -Wno-cast-function-type \
  -fno-strict-aliasing \
 "
 
 AC_DEFUN([CHECK_CCSUPPORT], [
   my_save_cflags="$CFLAGS"
-  CFLAGS=$1
+  CFLAGS="-Werror $1"
   AC_MSG_CHECKING([whether CC supports $1])
   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],
     [AC_MSG_RESULT([yes])]
@@ -658,9 +657,10 @@ CHECK_CCSUPPORT([-Werror=format-overflow=2], [flg1])
 CHECK_CCSUPPORT([-Werror=int-conversion], [flg2])
 CHECK_CCSUPPORT([-Werror=incompatible-pointer-types], [flg3])
 CHECK_CCSUPPORT([-Werror=misleading-indentation], [flg4])
+CHECK_CCSUPPORT([-Wno-cast-function-type], [flg5])
 AX_GCC_FUNC_ATTRIBUTE([format])
 
-AC_SUBST([AM_CFLAGS], ["$my_am_cflags $flg1 $flg2 $flg3 $flg4"])
+AC_SUBST([AM_CFLAGS], ["$my_am_cflags $flg1 $flg2 $flg3 $flg4 $flg5"])
 
 # Make sure that $ACLOCAL_FLAGS are used during a rebuild
 AC_SUBST([ACLOCAL_AMFLAGS], ["-I $ac_macro_dir \$(ACLOCAL_FLAGS)"])
diff --git a/support/include/xcommon.h b/support/include/xcommon.h
index 0001e609..2120a194 100644
--- a/support/include/xcommon.h
+++ b/support/include/xcommon.h
@@ -7,7 +7,7 @@
  */
 
 #ifndef _XMALLOC_H
-#define _MALLOC_H
+#define _XMALLOC_H
 
 #include "compiler.h"
 
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 6ac913d9..d9c0b513 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1268,14 +1268,14 @@ nfs_nfs_program(struct mount_options *options, unsigned long *program)
 int
 nfs_nfs_version(char *type, struct mount_options *options, struct nfs_version *version)
 {
-	char *version_key, *version_val, *cptr;
+	char *version_key, *version_val = NULL, *cptr;
 	int i, found = 0;
 
 	version->v_mode = V_DEFAULT;
 
 	for (i = 0; nfs_version_opttbl[i]; i++) {
 		if (po_contains_prefix(options, nfs_version_opttbl[i],
-								&version_key) == PO_FOUND) {
+				       &version_key) == PO_FOUND) {
 			found++;
 			break;
 		}
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 901f995a..91a976b4 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -1094,9 +1094,7 @@ static int nfsmount_fg(struct nfsmount_info *mi)
 		if (nfs_try_mount(mi))
 			return EX_SUCCESS;
 
-#pragma GCC diagnostic ignored "-Wdiscarded-qualifiers"
 		if (errno == EBUSY && is_mountpoint(mi->node)) {
-#pragma GCC diagnostic warning "-Wdiscarded-qualifiers"
 			/*
 			 * EBUSY can happen when mounting a filesystem that
 			 * is already mounted or when the context= are
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 6cba2883..ea740672 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -57,7 +57,7 @@ enum nfsd_fsid {
 };
 
 #undef is_mountpoint
-static int is_mountpoint(char *path)
+static int is_mountpoint(const char *path)
 {
 	return check_is_mountpoint(path, nfsd_path_lstat);
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 04/10] gssd: gssd_k5_err_msg() returns a ". Use free() to release.
  2020-07-01 18:27 ` [PATCH 04/10] gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release Doug Nazar
@ 2020-07-08 14:50   ` Steve Dickson
  2020-07-12 20:27     ` Doug Nazar
  0 siblings, 1 reply; 19+ messages in thread
From: Steve Dickson @ 2020-07-08 14:50 UTC (permalink / raw)
  To: Doug Nazar, linux-nfs

Hello,

On 7/1/20 2:27 PM, Doug Nazar wrote:
> Signed-off-by: Doug Nazar <nazard@nazar.ca>
> ---
>  utils/gssd/krb5_util.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index c49c6672..b1e48241 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -484,7 +484,7 @@ gssd_get_single_krb5_cred(krb5_context context,
>  	if (ccache)
>  		krb5_cc_close(context, ccache);
>  	krb5_free_cred_contents(context, &my_creds);
> -	krb5_free_string(context, k5err);
> +	free(k5err);
>  	return (code);
>  }
>  
> @@ -723,7 +723,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
>  				 "we failed to unparse principal name: %s\n",
>  				 k5err);
>  			k5_free_kt_entry(context, kte);
> -			krb5_free_string(context, k5err);
> +			free(k5err);
>  			k5err = NULL;
>  			continue;
>  		}
> @@ -770,7 +770,7 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
>  	if (retval < 0)
>  		retval = 0;
>    out:
> -	krb5_free_string(context, k5err);
> +	free(k5err);
>  	return retval;
>  }
>  
> @@ -927,7 +927,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt,
>  				k5err = gssd_k5_err_msg(context, code);
>  				printerr(1, "%s while building principal for '%s'\n",
>  					 k5err, spn);
> -				krb5_free_string(context, k5err);
> +				free(k5err);
>  				k5err = NULL;
>  				continue;
>  			}
> @@ -937,7 +937,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt,
>  				k5err = gssd_k5_err_msg(context, code);
>  				printerr(3, "%s while getting keytab entry for '%s'\n",
>  					 k5err, spn);
> -				krb5_free_string(context, k5err);
> +				free(k5err);
>  				k5err = NULL;
>  				/*
>  				 * We tried the active directory machine account
> @@ -986,7 +986,7 @@ out:
>  		k5_free_default_realm(context, default_realm);
>  	if (realmnames)
>  		krb5_free_host_realm(context, realmnames);
> -	krb5_free_string(context, k5err);
> +	free(k5err);
>  	return retval;
>  }
>  
> @@ -1355,7 +1355,7 @@ out_free_kt:
>  out_free_context:
>  	krb5_free_context(context);
>  out:
> -	krb5_free_string(context, k5err);
> +	free(k5err);
>  	return retval;
>  }
>  
> 
I'm curious about these changes... since all krb5_free_string()
does is call free()... where is the "strdup'd msg" coming from?

steved.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 04/10] gssd: gssd_k5_err_msg() returns a ". Use free() to release.
  2020-07-08 14:50   ` [PATCH 04/10] gssd: gssd_k5_err_msg() returns a ". " Steve Dickson
@ 2020-07-12 20:27     ` Doug Nazar
  2020-07-13 18:47       ` Steve Dickson
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Nazar @ 2020-07-12 20:27 UTC (permalink / raw)
  To: Steve Dickson, linux-nfs

On 2020-07-08 10:50, Steve Dickson wrote:
> I'm curious about these changes... since all krb5_free_string()
> does is call free()... where is the "strdup'd msg" coming from?

gssd_k5_err_msg() always returns a local strdup() of the error message. 
We shouldn't be using a Kerberos library method to free them. There's no 
guarantee that the library won't change, or even that the library was 
compiled with the same malloc library.

Doug


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 04/10] gssd: gssd_k5_err_msg() returns a ". Use free() to release.
  2020-07-12 20:27     ` Doug Nazar
@ 2020-07-13 18:47       ` Steve Dickson
  2020-07-13 22:22         ` Doug Nazar
  0 siblings, 1 reply; 19+ messages in thread
From: Steve Dickson @ 2020-07-13 18:47 UTC (permalink / raw)
  To: Doug Nazar, linux-nfs



On 7/12/20 4:27 PM, Doug Nazar wrote:
> On 2020-07-08 10:50, Steve Dickson wrote:
>> I'm curious about these changes... since all krb5_free_string()
>> does is call free()... where is the "strdup'd msg" coming from?
> 
> gssd_k5_err_msg() always returns a local strdup() of the error message. 
True... 

> We shouldn't be using a Kerberos library method to free them. There's no guarantee that the library won't change, 
I guess I'm not too worry about this... I would the change was for the better.
and lets face the krb5 has not changed in a 100 years ;-) 

> or even that the library was compiled with the same malloc library.
There are different malloc libraries other than glibc? 

steved.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 04/10] gssd: gssd_k5_err_msg() returns a ". Use free() to release.
  2020-07-13 18:47       ` Steve Dickson
@ 2020-07-13 22:22         ` Doug Nazar
  0 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-13 22:22 UTC (permalink / raw)
  To: Steve Dickson, linux-nfs

On 2020-07-13 14:47, Steve Dickson wrote:
> or even that the library was compiled with the same malloc library.
> There are different malloc libraries other than glibc?

Not including the various libc implementations (dietlibc, klibc, uclibc) 
there are several specialized malloc libraries (dmalloc & jemalloc are 
the most common) that you can compile or link against. Usually since 
they replace malloc and friends everything works, but you can get into 
issues depending on order of linking and how you compile.

I recently put gssd though a few rounds of mallocfail, which allows you 
to progressive test every allocation point (during normal running) to 
ensure it's handled (instead of crashing). I've an updated patchset that 
fixes the event2 conversion to check for successful allocation and a 
couple other spots that crashed if the allocation failed.

I'm taking it a bit slower this time... kinda rushed the last patchset 
out the door since I was needed on another project.  ;-)

Doug


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 00/10] Misc fixes & cleanups for nfs-utils
  2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
                   ` (11 preceding siblings ...)
  2020-07-01 18:28 ` [PATCH 10/10] Fix various clang warnings Doug Nazar
@ 2020-07-14 18:38 ` Steve Dickson
  2020-07-16  6:56   ` Doug Nazar
  12 siblings, 1 reply; 19+ messages in thread
From: Steve Dickson @ 2020-07-14 18:38 UTC (permalink / raw)
  To: Doug Nazar, linux-nfs

Hey Doug,

On 7/1/20 2:27 PM, Doug Nazar wrote:
> Most of this work centers around gssd, however a few items I did tree
> wide. It's been compile tested with both gcc & clang on x86_64 & arm32
> and runtime tested on x86_64.
> 
> 
> Doug Nazar (10):
>   gssd: Refcount struct clnt_info to protect multithread usage
>   Update to libevent 2.x apis.
>   gssd: Cleanup on exit to support valgrind.
>   gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release.
>   gssd: Fix locking for machine principal list
>   gssd: Add a few debug statements to help track client_info lifetimes.
>   gssd: Lookup local hostname when srchost is '*'
>   gssd: We never use the nocache param of gssd_check_if_cc_exists()
>   Fix various clang warnings.
I did commit all of the above... (tag: nfs-utils-2-5-2-rc1)

I did not commit the following 
   Cleanup printf format attribute handling and fix format strings

because 3 different version were posted 

Cleanup printf format attribute handling and fix various format strings
Cleanup printf format attribute handling and fix format strings
Consolidate printf format attribute handling and fix various format strings

I was not sure which one you wanted and I was wondering what exactly is
being cleaned up? What problems is this solving?

Finally, being this is a whole tree commit and I have a number
of patches in the queue.. I would like to hold off on this one.

A patch like this will cause all those patches in the queue 
not to apply... So once I drain the queue, hopefully you
would not mind rebasing... after we talk about what you 
are trying to do.

I do appreciate the hard work... esp with gssd... I did test
it every step of the way... and it seems to be fairly 
solid... nice work!

steved.
 
> 
>  aclocal/libevent.m4                |   6 +-
>  configure.ac                       |   6 +-
>  support/include/compiler.h         |  14 +
>  support/include/xcommon.h          |  12 +-
>  support/include/xlog.h             |  20 +-
>  support/nfs/xcommon.c              |   2 +
>  support/nfsidmap/gums.c            |   2 +
>  support/nfsidmap/libnfsidmap.c     |   8 +-
>  support/nfsidmap/nfsidmap.h        |  10 +-
>  support/nfsidmap/nfsidmap_common.c |   1 +
>  support/nfsidmap/nss.c             |   4 +-
>  support/nfsidmap/regex.c           |   6 +-
>  support/nfsidmap/static.c          |   1 +
>  support/nfsidmap/umich_ldap.c      |  10 +-
>  tools/locktest/testlk.c            |   6 +-
>  utils/exportfs/exportfs.c          |   5 +-
>  utils/gssd/err_util.h              |   4 +-
>  utils/gssd/gss_names.c             |   9 +-
>  utils/gssd/gss_util.c              |   2 +-
>  utils/gssd/gssd.c                  | 165 ++++++++---
>  utils/gssd/gssd.h                  |  10 +-
>  utils/gssd/gssd_proc.c             |  14 +-
>  utils/gssd/krb5_util.c             | 422 +++++++++++++++++------------
>  utils/gssd/krb5_util.h             |  16 +-
>  utils/gssd/svcgssd.c               |   4 +-
>  utils/gssd/svcgssd_proc.c          |   9 +-
>  utils/idmapd/idmapd.c              |  65 +++--
>  utils/mount/network.c              |   4 +-
>  utils/mount/stropts.c              |   2 -
>  utils/mountd/cache.c               |   2 +-
>  utils/nfsdcld/cld-internal.h       |   2 +-
>  utils/nfsdcld/nfsdcld.c            |  29 +-
>  utils/nfsdcld/sqlite.c             |   1 -
>  utils/nfsdcltrack/sqlite.c         |   2 +-
>  utils/nfsidmap/nfsidmap.c          |   3 +-
>  35 files changed, 536 insertions(+), 342 deletions(-)
>  create mode 100644 support/include/compiler.h
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 00/10] Misc fixes & cleanups for nfs-utils
  2020-07-14 18:38 ` [PATCH 00/10] Misc fixes & cleanups for nfs-utils Steve Dickson
@ 2020-07-16  6:56   ` Doug Nazar
  0 siblings, 0 replies; 19+ messages in thread
From: Doug Nazar @ 2020-07-16  6:56 UTC (permalink / raw)
  To: Steve Dickson, linux-nfs

On 2020-07-14 14:38, Steve Dickson wrote:
>
>>    gssd: Refcount struct clnt_info to protect multithread usage
>>    Update to libevent 2.x apis.
>>    gssd: Cleanup on exit to support valgrind.
>>    gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release.
>>    gssd: Fix locking for machine principal list
>>    gssd: Add a few debug statements to help track client_info lifetimes.
>>    gssd: Lookup local hostname when srchost is '*'
>>    gssd: We never use the nocache param of gssd_check_if_cc_exists()
>>    Fix various clang warnings.
> I did commit all of the above... (tag: nfs-utils-2-5-2-rc1)

Oops, I'd been working on an updated patch set. There's nothing really 
actively wrong in the above patches. I'd just gone back and added proper 
NULL checks and error messages for the libevent conversion. I'll rebase 
and send that as an update. Also "gssd: Lookup local hostname when 
srchost is '*'" I think is wrong. After the first day I couldn't get it 
to repeat, and think it was a mis-compile issue. However, the new code 
arrangement is better in that it shows the correct dns name 
transformation. Previously it would use the same buffer for input & 
output which made the log message very confusing. I'll just drop the '*' 
check.

> I did not commit the following
>     Cleanup printf format attribute handling and fix format strings
>
> because 3 different version were posted
>
> Cleanup printf format attribute handling and fix various format strings
> Cleanup printf format attribute handling and fix format strings
> Consolidate printf format attribute handling and fix various format strings
>
> I was not sure which one you wanted and I was wondering what exactly is
> being cleaned up? What problems is this solving?

They're all the same patch. The summary line was wrapping in the cover 
letter so I edited it a few times, not realizing that format-patch was 
creating another file even if I aborted.

So, it actually does a few things, all based around fixing printf style 
formats.

There were 2 different macros defined to add printf format attribute to 
functions, and several open codings. So it first consolidates them into 
one set of macros (although there is a second copy in nfsidmap.h since 
that's an installed file and can't depend on config.h.

Then, there were several functions that were not marked with the printf 
format attribute (nfsidmap plugins and gssd printerr()).

Finally, a cleanup of all the resulting gcc & clang warnings on both 32 
& 64 bit. In several cases some real errors, not enough parameters, 
passing in various types for the dynamic length which requires an int, 
passing in a char** instead of char*, etc. Of course these are mainly 
debugging messages so rarely caused an issue but were in need of 
cleaning up.

> Finally, being this is a whole tree commit and I have a number
> of patches in the queue.. I would like to hold off on this one.
>
> A patch like this will cause all those patches in the queue
> not to apply... So once I drain the queue, hopefully you
> would not mind rebasing... after we talk about what you
> are trying to do.

Not a problem. I have it rebased here and can send it at any time, or 
split it up if you prefer.

> I do appreciate the hard work... esp with gssd... I did test
> it every step of the way... and it seems to be fairly
> solid... nice work!

I've been chasing that threading bug for over a year. Trying to stress 
the number of simultaneous mounts, types, etc. never thinking the issue 
was external. I bet if I went back and correlated the crashes I saw, 
probably happened when I was upgrading or rebooting the kdc.

Doug


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-07-16  6:56 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 18:27 [PATCH 00/10] Misc fixes & cleanups for nfs-utils Doug Nazar
2020-07-01 18:27 ` [PATCH 01/10] gssd: Refcount struct clnt_info to protect multithread usage Doug Nazar
2020-07-01 18:27 ` [PATCH 02/10] Update to libevent 2.x apis Doug Nazar
2020-07-01 18:27 ` [PATCH 03/10] gssd: Cleanup on exit to support valgrind Doug Nazar
2020-07-01 18:27 ` [PATCH 04/10] gssd: gssd_k5_err_msg() returns a strdup'd msg. Use free() to release Doug Nazar
2020-07-08 14:50   ` [PATCH 04/10] gssd: gssd_k5_err_msg() returns a ". " Steve Dickson
2020-07-12 20:27     ` Doug Nazar
2020-07-13 18:47       ` Steve Dickson
2020-07-13 22:22         ` Doug Nazar
2020-07-01 18:27 ` [PATCH 05/10] gssd: Fix locking for machine principal list Doug Nazar
2020-07-01 18:27 ` [PATCH 06/10] gssd: Add a few debug statements to help track client_info lifetimes Doug Nazar
2020-07-01 18:27 ` [PATCH 07/10] gssd: Lookup local hostname when srchost is '*' Doug Nazar
2020-07-01 18:27 ` [PATCH 08/10] gssd: We never use the nocache param of gssd_check_if_cc_exists() Doug Nazar
2020-07-01 18:28 ` [PATCH 09/10] Cleanup printf format attribute handling and fix format strings Doug Nazar
2020-07-01 18:28 ` [PATCH 09/10] Cleanup printf format attribute handling and fix various " Doug Nazar
2020-07-01 18:28 ` [PATCH 09/10] Consolidate " Doug Nazar
2020-07-01 18:28 ` [PATCH 10/10] Fix various clang warnings Doug Nazar
2020-07-14 18:38 ` [PATCH 00/10] Misc fixes & cleanups for nfs-utils Steve Dickson
2020-07-16  6:56   ` Doug Nazar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.