linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] nfs-utils: Misc cleanups & fixes
@ 2020-07-18  9:24 Doug Nazar
  2020-07-18  9:24 ` [PATCH 01/11] Add error handling to libevent allocations Doug Nazar
                   ` (11 more replies)
  0 siblings, 12 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Again, here are various cleanups and fixes. Nothing too major, although
a couple valgrind finds.

I've left out the printf patch for now, pending further discussion.

Thanks,
Doug


Doug Nazar (11):
  Add error handling to libevent allocations.
  gssd: Fix cccache buffer size
  gssd: Fix handling of failed allocations
  gssd: srchost should never be *
  xlog: Reorganize xlog_backend() to work around -Wmaybe-uninitialized
  nfsdcld: Add graceful exit handling and resource cleanup
  nfsdcld: Don't copy more data than exists in column
  svcgssd: Convert to using libevent
  nfsidmap: Add support to cleanup resources on exit
  svcgssd: Cleanup global resources on exit
  svcgssd: Wait for nullrpc channel if not available

 support/nfs/xlog.c                  |  41 ++++----
 support/nfsidmap/libnfsidmap.c      |  13 +++
 support/nfsidmap/nfsidmap.h         |   1 +
 support/nfsidmap/nfsidmap_common.c  |  11 ++-
 support/nfsidmap/nfsidmap_private.h |   1 +
 support/nfsidmap/nss.c              |   8 ++
 utils/gssd/Makefile.am              |   2 +-
 utils/gssd/gss_names.c              |   6 +-
 utils/gssd/gss_util.c               |   6 ++
 utils/gssd/gss_util.h               |   1 +
 utils/gssd/gssd.c                   |  37 +++++--
 utils/gssd/krb5_util.c              |  12 +--
 utils/gssd/svcgssd.c                | 143 ++++++++++++++++++++++++++--
 utils/gssd/svcgssd.h                |   3 +-
 utils/gssd/svcgssd_krb5.c           |  21 ++--
 utils/gssd/svcgssd_krb5.h           |   1 +
 utils/gssd/svcgssd_main_loop.c      |  94 ------------------
 utils/gssd/svcgssd_proc.c           |  15 +--
 utils/idmapd/idmapd.c               |  32 +++++++
 utils/nfsdcld/nfsdcld.c             |  50 +++++++++-
 utils/nfsdcld/sqlite.c              |  33 +++++--
 utils/nfsdcld/sqlite.h              |   1 +
 22 files changed, 358 insertions(+), 174 deletions(-)
 delete mode 100644 utils/gssd/svcgssd_main_loop.c

-- 
2.26.2


^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH 01/11] Add error handling to libevent allocations.
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 02/11] gssd: Fix cccache buffer size Doug Nazar
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/gssd.c       | 37 +++++++++++++++++++++++++++----------
 utils/idmapd/idmapd.c   | 32 ++++++++++++++++++++++++++++++++
 utils/nfsdcld/nfsdcld.c | 18 +++++++++++++++++-
 3 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 3c7c703a..85bc4b07 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -560,14 +560,9 @@ static int
 gssd_scan_clnt(struct clnt_info *clp)
 {
 	int clntfd;
-	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;
-
 	clntfd = openat(pipefs_fd, clp->relpath, O_RDONLY);
 	if (clntfd < 0) {
 		if (errno != ENOENT)
@@ -582,16 +577,30 @@ gssd_scan_clnt(struct clnt_info *clp)
 	if (clp->gssd_fd == -1 && clp->krb5_fd == -1)
 		clp->krb5_fd = openat(clntfd, "krb5", O_RDWR | O_NONBLOCK);
 
-	if (gssd_was_closed && clp->gssd_fd >= 0) {
+	if (!clp->gssd_ev && clp->gssd_fd >= 0) {
 		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 (!clp->gssd_ev) {
+			printerr(0, "ERROR: %s: can't create gssd event for %s: %s\n",
+				 __FUNCTION__, clp->relpath, strerror(errno));
+			close(clp->gssd_fd);
+			clp->gssd_fd = -1;
+		} else {
+			event_add(clp->gssd_ev, NULL);
+		}
 	}
 
-	if (krb5_was_closed && clp->krb5_fd >= 0) {
+	if (!clp->krb5_ev && clp->krb5_fd >= 0) {
 		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_ev) {
+			printerr(0, "ERROR: %s: can't create krb5 event for %s: %s\n",
+				 __FUNCTION__, clp->relpath, strerror(errno));
+			close(clp->krb5_fd);
+			clp->krb5_fd = -1;
+		} else {
+			event_add(clp->krb5_ev, NULL);
+		}
 	}
 
 	if (clp->krb5_fd == -1 && clp->gssd_fd == -1)
@@ -1086,7 +1095,7 @@ main(int argc, char *argv[])
 
 	evbase = event_base_new();
 	if (!evbase) {
-		printerr(0, "ERROR: failed to create event base\n");
+		printerr(0, "ERROR: failed to create event base: %s\n", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
 
@@ -1111,9 +1120,17 @@ main(int argc, char *argv[])
 	signal(SIGINT, sig_die);
 	signal(SIGTERM, sig_die);
 	sighup_ev = evsignal_new(evbase, SIGHUP, gssd_scan_cb, NULL);
+	if (!sighup_ev) {
+		printerr(0, "ERROR: failed to create SIGHUP event: %s\n", strerror(errno));
+		exit(EXIT_FAILURE);
+	}
 	evsignal_add(sighup_ev, NULL);
 	inotify_ev = event_new(evbase, inotify_fd, EV_READ | EV_PERSIST,
 			       gssd_inotify_cb, NULL);
+	if (!inotify_ev) {
+		printerr(0, "ERROR: failed to create inotify event: %s\n", strerror(errno));
+		exit(EXIT_FAILURE);
+	}
 	event_add(inotify_ev, NULL);
 
 	TAILQ_INIT(&topdir_list);
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 12648f67..491ef54c 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -400,13 +400,21 @@ main(int argc, char **argv)
 
 		/* These events are persistent */
 		rootdirev = evsignal_new(evbase, SIGUSR1, dirscancb, &icq);
+		if (rootdirev == NULL)
+			errx(1, "Failed to create SIGUSR1 event.");
 		evsignal_add(rootdirev, NULL);
 		clntdirev = evsignal_new(evbase, SIGUSR2, clntscancb, &icq);
+		if (clntdirev == NULL)
+			errx(1, "Failed to create SIGUSR2 event.");
 		evsignal_add(clntdirev, NULL);
 		svrdirev = evsignal_new(evbase, SIGHUP, svrreopen, NULL);
+		if (svrdirev == NULL)
+			errx(1, "Failed to create SIGHUP event.");
 		evsignal_add(svrdirev, NULL);
 		if ( wd >= 0) {
 			inotifyev = event_new(evbase, inotify_fd, EV_READ, dirscancb, &icq);
+			if (inotifyev == NULL)
+				errx(1, "Failed to create inotify read event.");
 			event_add(inotifyev, NULL);
 		}
 
@@ -414,6 +422,8 @@ main(int argc, char **argv)
 		/* (Delay till start of event_dispatch to avoid possibly losing
 		 * a SIGUSR1 between here and the call to event_dispatch().) */
 		initialize = evtimer_new(evbase, dirscancb, &icq);
+		if (initialize == NULL)
+			errx(1, "Failed to create initialize event.");
 		evtimer_add(initialize, &now);
 	}
 
@@ -768,6 +778,13 @@ nfsdreopen_one(struct idmap_client *ic)
 
 		ic->ic_fd = fd;
 		ic->ic_event = event_new(evbase, ic->ic_fd, EV_READ, nfsdcb, ic);
+		if (ic->ic_event == NULL) {
+			xlog_warn("nfsdreopen: Failed to create event for '%s'",
+				  ic->ic_path);
+			close(ic->ic_fd);
+			ic->ic_fd = -1;
+			return;
+		}
 		event_add(ic->ic_event, NULL);
 	} else {
 		xlog_warn("nfsdreopen: Opening '%s' failed: errno %d (%s)",
@@ -802,6 +819,14 @@ nfsdopenone(struct idmap_client *ic)
 	}
 
 	ic->ic_event = event_new(evbase, ic->ic_fd, EV_READ, nfsdcb, ic);
+	if (ic->ic_event == NULL) {
+		if (verbose > 0)
+			xlog_warn("nfsdopenone: Create event for %s failed",
+				  ic->ic_path);
+		close(ic->ic_fd);
+		ic->ic_fd = -1;
+		return (-1);
+	}
 	event_add(ic->ic_event, NULL);
 
 	if (verbose > 0)
@@ -826,6 +851,13 @@ nfsopen(struct idmap_client *ic)
 		}
 	} else {
 		ic->ic_event = event_new(evbase, ic->ic_fd, EV_READ, nfscb, ic);
+		if (ic->ic_event == NULL) {
+			xlog_warn("nfsdopenone: Create event for %s failed",
+				  ic->ic_path);
+			close(ic->ic_fd);
+			ic->ic_fd = -1;
+			return -1;
+		}
 		event_add(ic->ic_event, NULL);
 		fcntl(ic->ic_dirfd, F_NOTIFY, 0);
 		fcntl(ic->ic_dirfd, F_SETSIG, 0);
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index 6cefcf24..5ad94ce2 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -142,6 +142,7 @@ static int
 cld_pipe_open(struct cld_client *clnt)
 {
 	int fd;
+	struct event *ev;
 
 	xlog(D_GENERAL, "%s: opening upcall pipe %s", __func__, pipepath);
 	fd = open(pipepath, O_RDWR, 0);
@@ -150,6 +151,13 @@ cld_pipe_open(struct cld_client *clnt)
 		return -errno;
 	}
 
+	ev = event_new(evbase, fd, EV_READ, cldcb, clnt);
+	if (ev == NULL) {
+		xlog(D_GENERAL, "%s: failed to create event for %s", __func__, pipepath);
+		close(fd);
+		return -ENOMEM;
+	}
+
 	if (clnt->cl_event && event_initialized(clnt->cl_event)) {
 		event_del(clnt->cl_event);
 		event_free(clnt->cl_event);
@@ -158,7 +166,7 @@ cld_pipe_open(struct cld_client *clnt)
 		close(clnt->cl_fd);
 
 	clnt->cl_fd = fd;
-	clnt->cl_event = event_new(evbase, clnt->cl_fd, EV_READ, cldcb, clnt);
+	clnt->cl_event = ev;
 	/* event_add is done by the caller */
 	return 0;
 }
@@ -304,6 +312,10 @@ cld_pipe_init(struct cld_client *clnt)
 
 	/* set event for inotify read */
 	pipedir_event = event_new(evbase, inotify_fd, EV_READ, cld_inotify_cb, clnt);
+	if (pipedir_event == NULL) {
+		close(inotify_fd);
+		return -ENOMEM;
+	}
 	event_add(pipedir_event, NULL);
 out:
 	return ret;
@@ -768,6 +780,10 @@ main(int argc, char **argv)
 	}
 
 	evbase = event_base_new();
+	if (evbase == NULL) {
+		fprintf(stderr, "%s: unable to allocate event base.\n", argv[0]);
+		return 1;
+	}
 	xlog_syslog(0);
 	xlog_stderr(1);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 02/11] gssd: Fix cccache buffer size
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
  2020-07-18  9:24 ` [PATCH 01/11] Add error handling to libevent allocations Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-20 14:43   ` Steve Dickson
  2020-07-18  9:24 ` [PATCH 03/11] gssd: Fix handling of failed allocations Doug Nazar
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

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

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index e5b81823..34c81daa 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -1225,7 +1225,7 @@ out:
 int
 gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)
 {
-	char			buf[PATH_MAX+2+256], dirname[PATH_MAX];
+	char			buf[PATH_MAX+4+2+256], dirname[PATH_MAX];
 	const char		*cctype;
 	struct dirent		*d;
 	int			err, i, j;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 03/11] gssd: Fix handling of failed allocations
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
  2020-07-18  9:24 ` [PATCH 01/11] Add error handling to libevent allocations Doug Nazar
  2020-07-18  9:24 ` [PATCH 02/11] gssd: Fix cccache buffer size Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 04/11] gssd: srchost should never be * Doug Nazar
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/gss_names.c | 6 ++++--
 utils/gssd/krb5_util.c | 8 ++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/utils/gssd/gss_names.c b/utils/gssd/gss_names.c
index 2a7f3a13..982b96f4 100644
--- a/utils/gssd/gss_names.c
+++ b/utils/gssd/gss_names.c
@@ -110,10 +110,12 @@ get_hostbased_client_name(gss_name_t client_name, gss_OID mech,
 	/* For Kerberos, transform the NT_KRB5_PRINCIPAL name to
 	 * an NT_HOSTBASED_SERVICE name */
 	if (g_OID_equal(&krb5oid, mech)) {
-		if (get_krb5_hostbased_name(&name, &cname) == 0)
-			*hostbased_name = cname;
+		if (get_krb5_hostbased_name(&name, &cname) != 0)
+			goto out_rel_buf;
+		*hostbased_name = cname;
 	} else {
 		printerr(1, "WARNING: unknown/unsupport mech OID\n");
+		goto out_rel_buf;
 	}
 
 	res = 0;
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 34c81daa..1d7b30c6 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -875,10 +875,10 @@ find_keytab_entry(krb5_context context, krb5_keytab kt,
 	/* Compute the active directory machine name HOST$ */
 	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",
-				adhostoverride);
+	if (adhostoverride && strcmp(adhostoverride, notsetstr) != 0) {
+		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);
 	} else {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 04/11] gssd: srchost should never be *
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (2 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 03/11] gssd: Fix handling of failed allocations Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 05/11] xlog: Reorganize xlog_backend() to work around -Wmaybe-uninitialized Doug Nazar
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Fix silly mistake on my part due to a rebase error.

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

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index 1d7b30c6..a1a9ffe9 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -854,7 +854,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt,
 		goto out;
 
 	/* Get full local hostname */
-	if (srchost && strcmp(srchost, "*") != 0) {
+	if (srchost) {
 		strcpy(myhostname, srchost);
 	        strcpy(myhostad, myhostname);
 	} else {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 05/11] xlog: Reorganize xlog_backend() to work around -Wmaybe-uninitialized
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (3 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 04/11] gssd: srchost should never be * Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 06/11] nfsdcld: Add graceful exit handling and resource cleanup Doug Nazar
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

xlog.c: In function ‘xlog_backend’:
xlog.c:202:3: warning: ‘args2’ may be used uninitialized in this function [-Wmaybe-uninitialized]
202 |   vfprintf(stderr, fmt, args2);
    |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 support/nfs/xlog.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/support/nfs/xlog.c b/support/nfs/xlog.c
index 687d862d..86acd6ac 100644
--- a/support/nfs/xlog.c
+++ b/support/nfs/xlog.c
@@ -156,13 +156,29 @@ xlog_enabled(int fac)
 void
 xlog_backend(int kind, const char *fmt, va_list args)
 {
-	va_list args2;
-
 	if (!(kind & (L_ALL)) && !(logging && (kind & logmask)))
 		return;
 
-	if (log_stderr)
+	if (log_stderr) {
+		va_list		args2;
+#ifdef VERBOSE_PRINTF
+		time_t		now;
+		struct tm	*tm;
+
+		time(&now);
+		tm = localtime(&now);
+		fprintf(stderr, "%s[%d] %04d-%02d-%02d %02d:%02d:%02d ",
+				log_name, log_pid,
+				tm->tm_year+1900, tm->tm_mon + 1, tm->tm_mday,
+				tm->tm_hour, tm->tm_min, tm->tm_sec);
+#else
+		fprintf(stderr, "%s: ", log_name);
+#endif
 		va_copy(args2, args);
+		vfprintf(stderr, fmt, args2);
+		fprintf(stderr, "\n");
+		va_end(args2);
+	}
 
 	if (log_syslog) {
 		switch (kind) {
@@ -185,25 +201,6 @@ xlog_backend(int kind, const char *fmt, va_list args)
 		}
 	}
 
-	if (log_stderr) {
-#ifdef VERBOSE_PRINTF
-		time_t		now;
-		struct tm	*tm;
-
-		time(&now);
-		tm = localtime(&now);
-		fprintf(stderr, "%s[%d] %04d-%02d-%02d %02d:%02d:%02d ",
-				log_name, log_pid,
-				tm->tm_year+1900, tm->tm_mon + 1, tm->tm_mday,
-				tm->tm_hour, tm->tm_min, tm->tm_sec);
-#else
-		fprintf(stderr, "%s: ", log_name);
-#endif
-		vfprintf(stderr, fmt, args2);
-		fprintf(stderr, "\n");
-		va_end(args2);
-	}
-
 	if (kind == L_FATAL)
 		exit(1);
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 06/11] nfsdcld: Add graceful exit handling and resource cleanup
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (4 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 05/11] xlog: Reorganize xlog_backend() to work around -Wmaybe-uninitialized Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 07/11] nfsdcld: Don't copy more data than exists in column Doug Nazar
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/nfsdcld/nfsdcld.c | 32 ++++++++++++++++++++++++++++++--
 utils/nfsdcld/sqlite.c  | 15 +++++++++++++++
 utils/nfsdcld/sqlite.h  |  1 +
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index 5ad94ce2..636c3983 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -27,6 +27,7 @@
 #include <event2/event.h>
 #include <stdbool.h>
 #include <getopt.h>
+#include <signal.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -69,6 +70,7 @@ static int 		inotify_fd = -1;
 static struct event	 *pipedir_event;
 static struct event_base *evbase;
 static bool old_kernel = false;
+static bool signal_received = false;
 
 uint64_t current_epoch;
 uint64_t recovery_epoch;
@@ -89,6 +91,19 @@ static struct option longopts[] =
 /* forward declarations */
 static void cldcb(int UNUSED(fd), short which, void *data);
 
+static void
+sig_die(int signal)
+{
+	if (signal_received) {
+		xlog(D_GENERAL, "forced exiting on signal %d\n", signal);
+		exit(0);
+	}
+
+	signal_received = true;
+	xlog(D_GENERAL, "exiting on signal %d\n", signal);
+	event_base_loopexit(evbase, NULL);
+}
+
 static void
 usage(char *progname)
 {
@@ -881,14 +896,27 @@ main(int argc, char **argv)
 	if (rc)
 		goto out;
 
+	signal(SIGINT, sig_die);
+	signal(SIGTERM, sig_die);
+
 	xlog(D_GENERAL, "%s: Starting event dispatch handler.", __func__);
 	rc = event_base_dispatch(evbase);
 	if (rc < 0)
 		xlog(L_ERROR, "%s: event_dispatch failed: %m", __func__);
 
-	close(clnt.cl_fd);
-	close(inotify_fd);
 out:
+	if (clnt.cl_event)
+		event_free(clnt.cl_event);
+	if (clnt.cl_fd != -1)
+		close(clnt.cl_fd);
+	if (pipedir_event)
+		event_free(pipedir_event);
+	if (inotify_fd != -1)
+		close(inotify_fd);
+
+	event_base_free(evbase);
+	sqlite_shutdown();
+
 	free(progname);
 	return rc;
 }
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index e2586c39..8fd1d0c2 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -1404,3 +1404,18 @@ sqlite_first_time_done(void)
 	sqlite3_free(err);
 	return ret;
 }
+
+/*
+ * Closes all sqlite3 resources and shuts down the library.
+ *
+ */
+void
+sqlite_shutdown(void)
+{
+	if (dbh != NULL) {
+		sqlite3_close(dbh);
+		dbh = NULL;
+	}
+
+	sqlite3_shutdown();
+}
diff --git a/utils/nfsdcld/sqlite.h b/utils/nfsdcld/sqlite.h
index 0a26ad67..044236cf 100644
--- a/utils/nfsdcld/sqlite.h
+++ b/utils/nfsdcld/sqlite.h
@@ -34,4 +34,5 @@ int sqlite_iterate_recovery(int (*cb)(struct cld_client *clnt), struct cld_clien
 int sqlite_delete_cltrack_records(void);
 int sqlite_first_time_done(void);
 
+void sqlite_shutdown(void);
 #endif /* _SQLITE_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 07/11] nfsdcld: Don't copy more data than exists in column
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (5 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 06/11] nfsdcld: Add graceful exit handling and resource cleanup Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 08/11] svcgssd: Convert to using libevent Doug Nazar
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Found with valgrind.

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/nfsdcld/sqlite.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index 8fd1d0c2..03016fb9 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -1330,20 +1330,26 @@ sqlite_iterate_recovery(int (*cb)(struct cld_client *clnt), struct cld_client *c
 	}
 
 	while ((ret = sqlite3_step(stmt)) == SQLITE_ROW) {
+		const void *id;
+		int id_len;
+
+		id = sqlite3_column_blob(stmt, 0);
+		id_len = sqlite3_column_bytes(stmt, 0);
+		if (id_len > NFS4_OPAQUE_LIMIT)
+			id_len = NFS4_OPAQUE_LIMIT;
+
 		memset(&cmsg->cm_u, 0, sizeof(cmsg->cm_u));
 #if UPCALL_VERSION >= 2
-		memcpy(&cmsg->cm_u.cm_clntinfo.cc_name.cn_id,
-			sqlite3_column_blob(stmt, 0), NFS4_OPAQUE_LIMIT);
-		cmsg->cm_u.cm_clntinfo.cc_name.cn_len = sqlite3_column_bytes(stmt, 0);
+		memcpy(&cmsg->cm_u.cm_clntinfo.cc_name.cn_id, id, id_len);
+		cmsg->cm_u.cm_clntinfo.cc_name.cn_len = id_len;
 		if (sqlite3_column_bytes(stmt, 1) > 0) {
 			memcpy(&cmsg->cm_u.cm_clntinfo.cc_princhash.cp_data,
 				sqlite3_column_blob(stmt, 1), SHA256_DIGEST_SIZE);
 			cmsg->cm_u.cm_clntinfo.cc_princhash.cp_len = sqlite3_column_bytes(stmt, 1);
 		}
 #else
-		memcpy(&cmsg->cm_u.cm_name.cn_id, sqlite3_column_blob(stmt, 0),
-			NFS4_OPAQUE_LIMIT);
-		cmsg->cm_u.cm_name.cn_len = sqlite3_column_bytes(stmt, 0);
+		memcpy(&cmsg->cm_u.cm_name.cn_id, id, id_len);
+		cmsg->cm_u.cm_name.cn_len = id_len;
 #endif
 		cb(clnt);
 	}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 08/11] svcgssd: Convert to using libevent
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (6 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 07/11] nfsdcld: Don't copy more data than exists in column Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit Doug Nazar
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/Makefile.am         |  2 +-
 utils/gssd/svcgssd.c           | 72 ++++++++++++++++++++++++--
 utils/gssd/svcgssd.h           |  3 +-
 utils/gssd/svcgssd_main_loop.c | 94 ----------------------------------
 utils/gssd/svcgssd_proc.c      | 15 +-----
 5 files changed, 70 insertions(+), 116 deletions(-)
 delete mode 100644 utils/gssd/svcgssd_main_loop.c

diff --git a/utils/gssd/Makefile.am b/utils/gssd/Makefile.am
index 321046b9..21d3bb88 100644
--- a/utils/gssd/Makefile.am
+++ b/utils/gssd/Makefile.am
@@ -67,7 +67,6 @@ gssd_CFLAGS = \
 svcgssd_SOURCES = \
 	$(COMMON_SRCS) \
 	svcgssd.c \
-	svcgssd_main_loop.c \
 	svcgssd_mech2file.c \
 	svcgssd_proc.c \
 	svcgssd_krb5.c \
@@ -78,6 +77,7 @@ svcgssd_SOURCES = \
 svcgssd_LDADD = \
 	../../support/nfs/libnfs.la \
 	../../support/nfsidmap/libnfsidmap.la \
+	$(LIBEVENT) \
 	$(RPCSECGSS_LIBS) \
 	$(KRBLIBS) $(GSSAPI_LIBS) $(LIBTIRPC)
 
diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index ec49b616..f538fd2a 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -57,20 +57,30 @@
 #include <string.h>
 #include <signal.h>
 #include <nfsidmap.h>
+#include <event2/event.h>
+
 #include "nfslib.h"
 #include "svcgssd.h"
 #include "gss_util.h"
 #include "err_util.h"
 #include "conffile.h"
+#include "misc.h"
 
 struct state_paths etab;
+static bool signal_received = false;
+static struct event_base *evbase = NULL;
 
 static void
 sig_die(int signal)
 {
-	/* destroy krb5 machine creds */
+	if (signal_received) {
+		/* destroy krb5 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
@@ -89,6 +99,24 @@ usage(char *progname)
 	exit(1);
 }
 
+static void
+svcgssd_nullrpc_cb(int fd, short UNUSED(which), void *UNUSED(data))
+{
+	char	lbuf[RPC_CHAN_BUF_SIZE];
+	int	lbuflen = 0;
+
+	printerr(1, "reading null request\n");
+
+	lbuflen = read(fd, lbuf, sizeof(lbuf));
+	if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') {
+		printerr(0, "WARNING: handle_nullreq: failed reading request\n");
+		return;
+	}
+	lbuf[lbuflen-1] = 0;
+
+	handle_nullreq(lbuf);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -102,6 +130,9 @@ main(int argc, char *argv[])
 	char *progname;
 	char *principal = NULL;
 	char *s;
+	int rc;
+	int nullrpc_fd = -1;
+	struct event *nullrpc_event = NULL;
 
 	conf_init_file(NFS_CONFFILE);
 
@@ -182,6 +213,12 @@ main(int argc, char *argv[])
 
 	daemon_init(fg);
 
+	evbase = event_base_new();
+	if (!evbase) {
+		printerr(0, "ERROR: failed to create event base: %s\n", strerror(errno));
+		exit(EXIT_FAILURE);
+	}
+
 	signal(SIGINT, sig_die);
 	signal(SIGTERM, sig_die);
 	signal(SIGHUP, sig_hup);
@@ -209,10 +246,35 @@ main(int argc, char *argv[])
 		}
 	}
 
+#define NULLRPC_FILE "/proc/net/rpc/auth.rpcsec.init/channel"
+
+	nullrpc_fd = open(NULLRPC_FILE, O_RDWR);
+	if (nullrpc_fd < 0) {
+		printerr(0, "failed to open %s: %s\n",
+			 NULLRPC_FILE, strerror(errno));
+		exit(1);
+	}
+	nullrpc_event = event_new(evbase, nullrpc_fd, EV_READ | EV_PERSIST,
+				  svcgssd_nullrpc_cb, NULL);
+	if (!nullrpc_event) {
+		printerr(0, "failed to create event for %s: %s\n",
+			 NULLRPC_FILE, strerror(errno));
+		exit(1);
+	}
+	event_add(nullrpc_event, NULL);
+
 	daemon_ready();
 
 	nfs4_init_name_mapping(NULL); /* XXX: should only do this once */
-	gssd_run();
-	printerr(0, "gssd_run returned!\n");
-	abort();
+
+	rc = event_base_dispatch(evbase);
+	if (rc < 0)
+		printerr(0, "event_base_dispatch() returned %i!\n", rc);
+
+	event_free(nullrpc_event);
+	close(nullrpc_fd);
+
+	event_base_free(evbase);
+
+	return EXIT_SUCCESS;
 }
diff --git a/utils/gssd/svcgssd.h b/utils/gssd/svcgssd.h
index 02b5c7ae..e229b989 100644
--- a/utils/gssd/svcgssd.h
+++ b/utils/gssd/svcgssd.h
@@ -35,8 +35,7 @@
 #include <sys/queue.h>
 #include <gssapi/gssapi.h>
 
-void handle_nullreq(int f);
-void gssd_run(void);
+void handle_nullreq(char *cp);
 
 #define GSSD_SERVICE_NAME	"nfs"
 
diff --git a/utils/gssd/svcgssd_main_loop.c b/utils/gssd/svcgssd_main_loop.c
deleted file mode 100644
index 920520d0..00000000
--- a/utils/gssd/svcgssd_main_loop.c
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
-  Copyright (c) 2004 The Regents of the University of Michigan.
-  All rights reserved.
-
-  Redistribution and use in source and binary forms, with or without
-  modification, are permitted provided that the following conditions
-  are met:
-
-  1. Redistributions of source code must retain the above copyright
-     notice, this list of conditions and the following disclaimer.
-  2. Redistributions in binary form must reproduce the above copyright
-     notice, this list of conditions and the following disclaimer in the
-     documentation and/or other materials provided with the distribution.
-  3. Neither the name of the University nor the names of its
-     contributors may be used to endorse or promote products derived
-     from this software without specific prior written permission.
-
-  THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
-  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
-  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-  DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
-  FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
-  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
-  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
-  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
-  LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
-  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-*/
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif	/* HAVE_CONFIG_H */
-
-#include <sys/param.h>
-#include <sys/socket.h>
-#include <poll.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <netinet/in.h>
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <memory.h>
-#include <fcntl.h>
-#include <errno.h>
-#include <unistd.h>
-
-#include "svcgssd.h"
-#include "err_util.h"
-
-void
-gssd_run()
-{
-	int			ret;
-	int			f;
-	struct pollfd		pollfd;
-
-#define NULLRPC_FILE "/proc/net/rpc/auth.rpcsec.init/channel"
-
-	f = open(NULLRPC_FILE, O_RDWR);
-	if (f < 0) {
-		printerr(0, "failed to open %s: %s\n",
-			 NULLRPC_FILE, strerror(errno));
-		exit(1);
-	}
-	pollfd.fd = f;
-	pollfd.events = POLLIN;
-	while (1) {
-		int save_err;
-
-		pollfd.revents = 0;
-		printerr(1, "entering poll\n");
-		ret = poll(&pollfd, 1, -1);
-		save_err = errno;
-		printerr(1, "leaving poll\n");
-		if (ret < 0) {
-			if (save_err != EINTR)
-				printerr(0, "error return from poll: %s\n",
-					 strerror(save_err));
-		} else if (ret == 0) {
-			/* timeout; shouldn't happen. */
-		} else {
-			if (ret != 1) {
-				printerr(0, "bug: unexpected poll return %d\n",
-						ret);
-				exit(1);
-			}
-			if (pollfd.revents & POLLIN)
-				handle_nullreq(f);
-		}
-	}
-}
diff --git a/utils/gssd/svcgssd_proc.c b/utils/gssd/svcgssd_proc.c
index 72ec2540..b4031432 100644
--- a/utils/gssd/svcgssd_proc.c
+++ b/utils/gssd/svcgssd_proc.c
@@ -318,7 +318,7 @@ print_hexl(const char *description, unsigned char *cp, int length)
 #endif
 
 void
-handle_nullreq(int f) {
+handle_nullreq(char *cp) {
 	/* XXX initialize to a random integer to reduce chances of unnecessary
 	 * invalidation of existing ctx's on restarting svcgssd. */
 	static u_int32_t	handle_seq = 0;
@@ -340,24 +340,11 @@ handle_nullreq(int f) {
 	u_int32_t		maj_stat = GSS_S_FAILURE, min_stat = 0;
 	u_int32_t		ignore_min_stat;
 	struct svc_cred		cred;
-	char			lbuf[RPC_CHAN_BUF_SIZE];
-	int			lbuflen = 0;
-	char			*cp;
 	int32_t			ctx_endtime;
 	char			*hostbased_name = NULL;
 
 	printerr(1, "handling null request\n");
 
-	lbuflen = read(f, lbuf, sizeof(lbuf));
-	if (lbuflen <= 0 || lbuf[lbuflen-1] != '\n') {
-		printerr(0, "WARNING: handle_nullreq: "
-			    "failed reading request\n");
-		return;
-	}
-	lbuf[lbuflen-1] = 0;
-
-	cp = lbuf;
-
 	in_handle.length = (size_t) qword_get(&cp, in_handle.value,
 					      sizeof(in_handle_buf));
 #ifdef DEBUG
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (7 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 08/11] svcgssd: Convert to using libevent Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-20 15:49   ` Steve Dickson
  2020-07-18  9:24 ` [PATCH 10/11] svcgssd: Cleanup global " Doug Nazar
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 support/nfsidmap/libnfsidmap.c      | 13 +++++++++++++
 support/nfsidmap/nfsidmap.h         |  1 +
 support/nfsidmap/nfsidmap_common.c  | 11 ++++++++++-
 support/nfsidmap/nfsidmap_private.h |  1 +
 support/nfsidmap/nss.c              |  8 ++++++++
 5 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
index bce448cf..6b5647d2 100644
--- a/support/nfsidmap/libnfsidmap.c
+++ b/support/nfsidmap/libnfsidmap.c
@@ -496,6 +496,19 @@ out:
 	return ret ? -ENOENT: 0;
 }
 
+void nfs4_term_name_mapping(void)
+{
+	if (nfs4_plugins)
+		unload_plugins(nfs4_plugins);
+	if (gss_plugins)
+		unload_plugins(gss_plugins);
+
+	nfs4_plugins = gss_plugins = NULL;
+
+	free_local_realms();
+	conf_cleanup();
+}
+
 int
 nfs4_get_default_domain(char *UNUSED(server), char *domain, size_t len)
 {
diff --git a/support/nfsidmap/nfsidmap.h b/support/nfsidmap/nfsidmap.h
index 10630654..5a795684 100644
--- a/support/nfsidmap/nfsidmap.h
+++ b/support/nfsidmap/nfsidmap.h
@@ -50,6 +50,7 @@ typedef struct _extra_mapping_params {
 typedef void (*nfs4_idmap_log_function_t)(const char *, ...);
 
 int nfs4_init_name_mapping(char *conffile);
+void nfs4_term_name_mapping(void);
 int nfs4_get_default_domain(char *server, char *domain, size_t len);
 int nfs4_uid_to_name(uid_t uid, char *domain, char *name, size_t len);
 int nfs4_gid_to_name(gid_t gid, char *domain, char *name, size_t len);
diff --git a/support/nfsidmap/nfsidmap_common.c b/support/nfsidmap/nfsidmap_common.c
index f89b82ee..4d2cb14f 100644
--- a/support/nfsidmap/nfsidmap_common.c
+++ b/support/nfsidmap/nfsidmap_common.c
@@ -34,12 +34,21 @@ static char * toupper_str(char *s)
         return s;
 }
 
+static struct conf_list *local_realms = NULL;
+
+void free_local_realms(void)
+{
+	if (local_realms) {
+		conf_free_list(local_realms);
+		local_realms = NULL;
+	}
+}
+
 /* Get list of "local equivalent" realms.  Meaning the list of realms
  * where john@REALM.A is considered the same user as john@REALM.B
  * If not specified, default to upper-case of local domain name */
 struct conf_list *get_local_realms(void)
 {
-	static struct conf_list *local_realms = NULL;
 	if (local_realms) return local_realms;
 
 	local_realms = conf_get_list("General", "Local-Realms");
diff --git a/support/nfsidmap/nfsidmap_private.h b/support/nfsidmap/nfsidmap_private.h
index f1af55fa..a5cb6dda 100644
--- a/support/nfsidmap/nfsidmap_private.h
+++ b/support/nfsidmap/nfsidmap_private.h
@@ -37,6 +37,7 @@
 #include "conffile.h"
 
 struct conf_list *get_local_realms(void);
+void free_local_realms(void);
 int get_nostrip(void);
 int get_reformat_group(void);
 
diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c
index 9d46499c..f8dbb94a 100644
--- a/support/nfsidmap/nss.c
+++ b/support/nfsidmap/nss.c
@@ -467,6 +467,14 @@ static int nss_plugin_init(void)
 	return 0;
 }
 
+__attribute__((destructor))
+static int nss_plugin_term(void)
+{
+	free_local_realms();
+	conf_cleanup();
+	return 0;
+}
+
 
 struct trans_func nss_trans = {
 	.name		= "nsswitch",
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 10/11] svcgssd: Cleanup global resources on exit
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (8 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18  9:24 ` [PATCH 11/11] svcgssd: Wait for nullrpc channel if not available Doug Nazar
  2020-07-27 14:42 ` [PATCH 00/11] nfs-utils: Misc cleanups & fixes Steve Dickson
  11 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

Signed-off-by: Doug Nazar <nazard@nazar.ca>
---
 utils/gssd/gss_util.c     |  6 ++++++
 utils/gssd/gss_util.h     |  1 +
 utils/gssd/svcgssd.c      |  8 ++++++++
 utils/gssd/svcgssd_krb5.c | 21 ++++++++++++++-------
 utils/gssd/svcgssd_krb5.h |  1 +
 5 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/utils/gssd/gss_util.c b/utils/gssd/gss_util.c
index 2e6d40f0..a4b27779 100644
--- a/utils/gssd/gss_util.c
+++ b/utils/gssd/gss_util.c
@@ -339,3 +339,9 @@ out:
 	return retval;
 }
 
+void
+gssd_cleanup(void)
+{
+	u_int32_t min_stat;
+	gss_release_cred(&min_stat, &gssd_creds);
+}
diff --git a/utils/gssd/gss_util.h b/utils/gssd/gss_util.h
index aa9f7780..4da64e38 100644
--- a/utils/gssd/gss_util.h
+++ b/utils/gssd/gss_util.h
@@ -41,6 +41,7 @@ int gssd_acquire_cred(char *server_name, const gss_OID oid);
 void pgsserr(char *msg, u_int32_t maj_stat, u_int32_t min_stat,
 	const gss_OID mech);
 int gssd_check_mechs(void);
+void gssd_cleanup(void);
 
 #ifndef HAVE_LIBGSSGLUE
 #include <gssapi/gssapi_krb5.h>
diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index f538fd2a..3155a2f9 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -65,6 +65,7 @@
 #include "err_util.h"
 #include "conffile.h"
 #include "misc.h"
+#include "svcgssd_krb5.h"
 
 struct state_paths etab;
 static bool signal_received = false;
@@ -148,6 +149,9 @@ main(int argc, char *argv[])
 	rpc_verbosity = conf_get_num("svcgssd", "RPC-Verbosity", rpc_verbosity);
 	idmap_verbosity = conf_get_num("svcgssd", "IDMAP-Verbosity", idmap_verbosity);
 
+	/* We don't need the config anymore */
+	conf_cleanup();
+
 	while ((opt = getopt(argc, argv, "fivrnp:")) != -1) {
 		switch (opt) {
 			case 'f':
@@ -276,5 +280,9 @@ main(int argc, char *argv[])
 
 	event_base_free(evbase);
 
+	nfs4_term_name_mapping();
+	svcgssd_free_enctypes();
+	gssd_cleanup();
+
 	return EXIT_SUCCESS;
 }
diff --git a/utils/gssd/svcgssd_krb5.c b/utils/gssd/svcgssd_krb5.c
index 1d44d344..305d4751 100644
--- a/utils/gssd/svcgssd_krb5.c
+++ b/utils/gssd/svcgssd_krb5.c
@@ -74,13 +74,7 @@ parse_enctypes(char *enctypes)
 		return 0;
 
 	/* Free any existing cached_enctypes */
-	free(cached_enctypes);
-
-	if (parsed_enctypes != NULL) {
-		free(parsed_enctypes);
-		parsed_enctypes = NULL;
-		parsed_num_enctypes = 0;
-	}
+	svcgssd_free_enctypes();
 
 	/* count the number of commas */
 	for (curr = enctypes; curr && *curr != '\0'; curr = ++comma) {
@@ -162,6 +156,19 @@ out_clean_parsed:
 /*===  External routines ===*/
 /*==========================*/
 
+void
+svcgssd_free_enctypes(void)
+{
+	free(cached_enctypes);
+	cached_enctypes = NULL;
+
+	if (parsed_enctypes != NULL) {
+		free(parsed_enctypes);
+		parsed_enctypes = NULL;
+		parsed_num_enctypes = 0;
+	}
+}
+
 /*
  * Get encryption types supported by the kernel, and then
  * call gss_krb5_set_allowable_enctypes() to limit the
diff --git a/utils/gssd/svcgssd_krb5.h b/utils/gssd/svcgssd_krb5.h
index 07d5eb9b..78a90e9a 100644
--- a/utils/gssd/svcgssd_krb5.h
+++ b/utils/gssd/svcgssd_krb5.h
@@ -32,5 +32,6 @@
 #define SVCGSSD_KRB5_H
 
 int svcgssd_limit_krb5_enctypes(void);
+void svcgssd_free_enctypes(void);
 
 #endif /* SVCGSSD_KRB5_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 11/11] svcgssd: Wait for nullrpc channel if not available
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (9 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 10/11] svcgssd: Cleanup global " Doug Nazar
@ 2020-07-18  9:24 ` Doug Nazar
  2020-07-18 15:55   ` J. Bruce Fields
  2020-07-27 14:42 ` [PATCH 00/11] nfs-utils: Misc cleanups & fixes Steve Dickson
  11 siblings, 1 reply; 20+ messages in thread
From: Doug Nazar @ 2020-07-18  9:24 UTC (permalink / raw)
  To: linux-nfs

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

diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index 3155a2f9..3ab2100b 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -67,9 +67,14 @@
 #include "misc.h"
 #include "svcgssd_krb5.h"
 
-struct state_paths etab;
+struct state_paths etab; /* from cacheio.c */
 static bool signal_received = false;
 static struct event_base *evbase = NULL;
+static int nullrpc_fd = -1;
+static struct event *nullrpc_event = NULL;
+static struct event *wait_event = NULL;
+
+#define NULLRPC_FILE "/proc/net/rpc/auth.rpcsec.init/channel"
 
 static void
 sig_die(int signal)
@@ -118,6 +123,66 @@ svcgssd_nullrpc_cb(int fd, short UNUSED(which), void *UNUSED(data))
 	handle_nullreq(lbuf);
 }
 
+static void
+svcgssd_nullrpc_close(void)
+{
+	if (nullrpc_event) {
+		printerr(2, "closing nullrpc channel %s\n", NULLRPC_FILE);
+		event_free(nullrpc_event);
+		nullrpc_event = NULL;
+	}
+	if (nullrpc_fd != -1) {
+		close(nullrpc_fd);
+		nullrpc_fd = -1;
+	}
+}
+
+static void
+svcgssd_nullrpc_open(void)
+{
+	nullrpc_fd = open(NULLRPC_FILE, O_RDWR);
+	if (nullrpc_fd < 0) {
+		printerr(0, "failed to open %s: %s\n",
+			 NULLRPC_FILE, strerror(errno));
+		return;
+	}
+	nullrpc_event = event_new(evbase, nullrpc_fd, EV_READ | EV_PERSIST,
+				  svcgssd_nullrpc_cb, NULL);
+	if (!nullrpc_event) {
+		printerr(0, "failed to create event for %s: %s\n",
+			 NULLRPC_FILE, strerror(errno));
+		close(nullrpc_fd);
+		nullrpc_fd = -1;
+		return;
+	}
+	event_add(nullrpc_event, NULL);
+	printerr(2, "opened nullrpc channel %s\n", NULLRPC_FILE);
+}
+
+static void
+svcgssd_wait_cb(int UNUSED(fd), short UNUSED(which), void *UNUSED(data))
+{
+	static int times = 0;
+	int rc;
+
+	rc = access(NULLRPC_FILE, R_OK | W_OK);
+	if (rc != 0) {
+		struct timeval t = {times < 10 ? 1 : 10, 0};
+		times++;
+		if (times % 30 == 0)
+			printerr(2, "still waiting for nullrpc channel: %s\n",
+				NULLRPC_FILE);
+		evtimer_add(wait_event, &t);
+		return;
+	}
+
+	svcgssd_nullrpc_open();
+	event_free(wait_event);
+	wait_event = NULL;
+}
+
+
+
 int
 main(int argc, char *argv[])
 {
@@ -132,8 +197,6 @@ main(int argc, char *argv[])
 	char *principal = NULL;
 	char *s;
 	int rc;
-	int nullrpc_fd = -1;
-	struct event *nullrpc_event = NULL;
 
 	conf_init_file(NFS_CONFFILE);
 
@@ -250,22 +313,19 @@ main(int argc, char *argv[])
 		}
 	}
 
-#define NULLRPC_FILE "/proc/net/rpc/auth.rpcsec.init/channel"
-
-	nullrpc_fd = open(NULLRPC_FILE, O_RDWR);
-	if (nullrpc_fd < 0) {
-		printerr(0, "failed to open %s: %s\n",
-			 NULLRPC_FILE, strerror(errno));
-		exit(1);
-	}
-	nullrpc_event = event_new(evbase, nullrpc_fd, EV_READ | EV_PERSIST,
-				  svcgssd_nullrpc_cb, NULL);
+	svcgssd_nullrpc_open();
 	if (!nullrpc_event) {
-		printerr(0, "failed to create event for %s: %s\n",
-			 NULLRPC_FILE, strerror(errno));
-		exit(1);
+		struct timeval t = {1, 0};
+
+		printerr(2, "waiting for nullrpc channel to appear\n");
+		wait_event = evtimer_new(evbase, svcgssd_wait_cb, NULL);
+		if (!wait_event) {
+			printerr(0, "ERROR: failed to create wait event: %s\n",
+				 strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+		evtimer_add(wait_event, &t);
 	}
-	event_add(nullrpc_event, NULL);
 
 	daemon_ready();
 
@@ -275,8 +335,9 @@ main(int argc, char *argv[])
 	if (rc < 0)
 		printerr(0, "event_base_dispatch() returned %i!\n", rc);
 
-	event_free(nullrpc_event);
-	close(nullrpc_fd);
+	svcgssd_nullrpc_close();
+	if (wait_event)
+		event_free(wait_event);
 
 	event_base_free(evbase);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH 11/11] svcgssd: Wait for nullrpc channel if not available
  2020-07-18  9:24 ` [PATCH 11/11] svcgssd: Wait for nullrpc channel if not available Doug Nazar
@ 2020-07-18 15:55   ` J. Bruce Fields
  2020-07-18 18:07     ` Doug Nazar
  0 siblings, 1 reply; 20+ messages in thread
From: J. Bruce Fields @ 2020-07-18 15:55 UTC (permalink / raw)
  To: Doug Nazar; +Cc: linux-nfs

On Sat, Jul 18, 2020 at 05:24:21AM -0400, Doug Nazar wrote:
> Signed-off-by: Doug Nazar <nazard@nazar.ca>

So, is there a race here that could result in a hang, and has anyone
seen it in practice?

Just curious.  Thanks for doing this.--b.

> ---
>  utils/gssd/svcgssd.c | 99 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 80 insertions(+), 19 deletions(-)
> 
> diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
> index 3155a2f9..3ab2100b 100644
> --- a/utils/gssd/svcgssd.c
> +++ b/utils/gssd/svcgssd.c
> @@ -67,9 +67,14 @@
>  #include "misc.h"
>  #include "svcgssd_krb5.h"
>  
> -struct state_paths etab;
> +struct state_paths etab; /* from cacheio.c */
>  static bool signal_received = false;
>  static struct event_base *evbase = NULL;
> +static int nullrpc_fd = -1;
> +static struct event *nullrpc_event = NULL;
> +static struct event *wait_event = NULL;
> +
> +#define NULLRPC_FILE "/proc/net/rpc/auth.rpcsec.init/channel"
>  
>  static void
>  sig_die(int signal)
> @@ -118,6 +123,66 @@ svcgssd_nullrpc_cb(int fd, short UNUSED(which), void *UNUSED(data))
>  	handle_nullreq(lbuf);
>  }
>  
> +static void
> +svcgssd_nullrpc_close(void)
> +{
> +	if (nullrpc_event) {
> +		printerr(2, "closing nullrpc channel %s\n", NULLRPC_FILE);
> +		event_free(nullrpc_event);
> +		nullrpc_event = NULL;
> +	}
> +	if (nullrpc_fd != -1) {
> +		close(nullrpc_fd);
> +		nullrpc_fd = -1;
> +	}
> +}
> +
> +static void
> +svcgssd_nullrpc_open(void)
> +{
> +	nullrpc_fd = open(NULLRPC_FILE, O_RDWR);
> +	if (nullrpc_fd < 0) {
> +		printerr(0, "failed to open %s: %s\n",
> +			 NULLRPC_FILE, strerror(errno));
> +		return;
> +	}
> +	nullrpc_event = event_new(evbase, nullrpc_fd, EV_READ | EV_PERSIST,
> +				  svcgssd_nullrpc_cb, NULL);
> +	if (!nullrpc_event) {
> +		printerr(0, "failed to create event for %s: %s\n",
> +			 NULLRPC_FILE, strerror(errno));
> +		close(nullrpc_fd);
> +		nullrpc_fd = -1;
> +		return;
> +	}
> +	event_add(nullrpc_event, NULL);
> +	printerr(2, "opened nullrpc channel %s\n", NULLRPC_FILE);
> +}
> +
> +static void
> +svcgssd_wait_cb(int UNUSED(fd), short UNUSED(which), void *UNUSED(data))
> +{
> +	static int times = 0;
> +	int rc;
> +
> +	rc = access(NULLRPC_FILE, R_OK | W_OK);
> +	if (rc != 0) {
> +		struct timeval t = {times < 10 ? 1 : 10, 0};
> +		times++;
> +		if (times % 30 == 0)
> +			printerr(2, "still waiting for nullrpc channel: %s\n",
> +				NULLRPC_FILE);
> +		evtimer_add(wait_event, &t);
> +		return;
> +	}
> +
> +	svcgssd_nullrpc_open();
> +	event_free(wait_event);
> +	wait_event = NULL;
> +}
> +
> +
> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -132,8 +197,6 @@ main(int argc, char *argv[])
>  	char *principal = NULL;
>  	char *s;
>  	int rc;
> -	int nullrpc_fd = -1;
> -	struct event *nullrpc_event = NULL;
>  
>  	conf_init_file(NFS_CONFFILE);
>  
> @@ -250,22 +313,19 @@ main(int argc, char *argv[])
>  		}
>  	}
>  
> -#define NULLRPC_FILE "/proc/net/rpc/auth.rpcsec.init/channel"
> -
> -	nullrpc_fd = open(NULLRPC_FILE, O_RDWR);
> -	if (nullrpc_fd < 0) {
> -		printerr(0, "failed to open %s: %s\n",
> -			 NULLRPC_FILE, strerror(errno));
> -		exit(1);
> -	}
> -	nullrpc_event = event_new(evbase, nullrpc_fd, EV_READ | EV_PERSIST,
> -				  svcgssd_nullrpc_cb, NULL);
> +	svcgssd_nullrpc_open();
>  	if (!nullrpc_event) {
> -		printerr(0, "failed to create event for %s: %s\n",
> -			 NULLRPC_FILE, strerror(errno));
> -		exit(1);
> +		struct timeval t = {1, 0};
> +
> +		printerr(2, "waiting for nullrpc channel to appear\n");
> +		wait_event = evtimer_new(evbase, svcgssd_wait_cb, NULL);
> +		if (!wait_event) {
> +			printerr(0, "ERROR: failed to create wait event: %s\n",
> +				 strerror(errno));
> +			exit(EXIT_FAILURE);
> +		}
> +		evtimer_add(wait_event, &t);
>  	}
> -	event_add(nullrpc_event, NULL);
>  
>  	daemon_ready();
>  
> @@ -275,8 +335,9 @@ main(int argc, char *argv[])
>  	if (rc < 0)
>  		printerr(0, "event_base_dispatch() returned %i!\n", rc);
>  
> -	event_free(nullrpc_event);
> -	close(nullrpc_fd);
> +	svcgssd_nullrpc_close();
> +	if (wait_event)
> +		event_free(wait_event);
>  
>  	event_base_free(evbase);
>  
> -- 
> 2.26.2

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 11/11] svcgssd: Wait for nullrpc channel if not available
  2020-07-18 15:55   ` J. Bruce Fields
@ 2020-07-18 18:07     ` Doug Nazar
  0 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-18 18:07 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On 2020-07-18 11:55, J. Bruce Fields wrote:

> So, is there a race here that could result in a hang, and has anyone
> seen it in practice?
>
> Just curious.  Thanks for doing this.--b.

Not a hang, with the existing code, svcgssd will just exit. I'd have to 
go and restart it after boot. I'm assuming a systemd setup would just 
restart it automatically.
On my original systems I'd configured it to force load the modules, but 
I'd forgotten about that (it was about 10 years ago) when I built this 
new box last month.

As mentioned in the other email, sunrpc is loaded from an initramfs that 
doesn't have any of the gss modules. When nfsd is started (after 
svcgssd) it'll then load the gss modules but by then it's too late.

It seems to be a standard Gentoo setup. I just checked the default 
kernel config for genkernel (their kernel & initramfs builder) and it 
has all the rpc & nfs as modules, with the initramfs not supporting 
Kerberos.

Looks like Debian modprobes rpcsec_gss_krb5 before starting rpc.svcgssd.

So with this waiting for the file to appear, and I'm planning on adding 
a conditional module load to the Gentoo rpc.svcgssd init script, things 
should be as bulletproof as I can make it.

Doug


^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 02/11] gssd: Fix cccache buffer size
  2020-07-18  9:24 ` [PATCH 02/11] gssd: Fix cccache buffer size Doug Nazar
@ 2020-07-20 14:43   ` Steve Dickson
  2020-07-20 15:41     ` Doug Nazar
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Dickson @ 2020-07-20 14:43 UTC (permalink / raw)
  To: Doug Nazar, linux-nfs

Hey Doug,

On 7/18/20 5:24 AM, Doug Nazar wrote:
> Signed-off-by: Doug Nazar <nazard@nazar.ca>
> ---
>  utils/gssd/krb5_util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index e5b81823..34c81daa 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -1225,7 +1225,7 @@ out:
>  int
>  gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)
>  {
> -	char			buf[PATH_MAX+2+256], dirname[PATH_MAX];
> +	char			buf[PATH_MAX+4+2+256], dirname[PATH_MAX];
>  	const char		*cctype;
>  	struct dirent		*d;
>  	int			err, i, j;
> 
Thanks for point this out but I think I'm going to go with this:

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index e5b8182..cd5a919 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -223,7 +223,8 @@ gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
 	int found = 0;
 	struct dirent *best_match_dir = NULL;
 	struct stat best_match_stat, tmp_stat;
-	char buf[PATH_MAX+4+2+256];
+	/* dirname + cctype + d_name + NULL */
+	char buf[PATH_MAX+5+256+1];
 	char *princname = NULL;
 	char *realm = NULL;
 	int score, best_match_score = 0, err = -EACCES;
@@ -1225,7 +1226,8 @@ out:
 int
 gssd_setup_krb5_user_gss_ccache(uid_t uid, char *servername, char *dirpattern)
 {
-	char			buf[PATH_MAX+2+256], dirname[PATH_MAX];
+				/* dirname + cctype + d_name + NULL */
+	char			buf[PATH_MAX+5+256+1], dirname[PATH_MAX];
 	const char		*cctype;
 	struct dirent		*d;
 	int			err, i, j;

which explains the needed space and as well removes the warning... 

steved


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH 02/11] gssd: Fix cccache buffer size
  2020-07-20 14:43   ` Steve Dickson
@ 2020-07-20 15:41     ` Doug Nazar
  0 siblings, 0 replies; 20+ messages in thread
From: Doug Nazar @ 2020-07-20 15:41 UTC (permalink / raw)
  To: Steve Dickson, linux-nfs

On 2020-07-20 10:43, Steve Dickson wrote:
> Thanks for point this out but I think I'm going to go with this:
> -	char buf[PATH_MAX+4+2+256];
> +	/* dirname + cctype + d_name + NULL */
> +	char buf[PATH_MAX+5+256+1];
>
> which explains the needed space and as well removes the warning...

That's fine. I didn't spend much time wondering why it was written that 
way, just assumed there was a reason.

Doug


^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit
  2020-07-18  9:24 ` [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit Doug Nazar
@ 2020-07-20 15:49   ` Steve Dickson
  2020-07-20 15:58     ` Doug Nazar
  0 siblings, 1 reply; 20+ messages in thread
From: Steve Dickson @ 2020-07-20 15:49 UTC (permalink / raw)
  To: Doug Nazar, linux-nfs

Hey,

On 7/18/20 5:24 AM, Doug Nazar wrote:
> Signed-off-by: Doug Nazar <nazard@nazar.ca>
> ---
>  support/nfsidmap/libnfsidmap.c      | 13 +++++++++++++
>  support/nfsidmap/nfsidmap.h         |  1 +
>  support/nfsidmap/nfsidmap_common.c  | 11 ++++++++++-
>  support/nfsidmap/nfsidmap_private.h |  1 +
>  support/nfsidmap/nss.c              |  8 ++++++++
>  5 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/libnfsidmap.c
> index bce448cf..6b5647d2 100644
> --- a/support/nfsidmap/libnfsidmap.c
> +++ b/support/nfsidmap/libnfsidmap.c
> @@ -496,6 +496,19 @@ out:
>  	return ret ? -ENOENT: 0;
>  }
>  
> +void nfs4_term_name_mapping(void)
> +{
> +	if (nfs4_plugins)
> +		unload_plugins(nfs4_plugins);
> +	if (gss_plugins)
> +		unload_plugins(gss_plugins);
> +
> +	nfs4_plugins = gss_plugins = NULL;
> +
> +	free_local_realms();
> +	conf_cleanup();
> +}
> +
>  int
>  nfs4_get_default_domain(char *UNUSED(server), char *domain, size_t len)
>  {
> diff --git a/support/nfsidmap/nfsidmap.h b/support/nfsidmap/nfsidmap.h
> index 10630654..5a795684 100644
> --- a/support/nfsidmap/nfsidmap.h
> +++ b/support/nfsidmap/nfsidmap.h
> @@ -50,6 +50,7 @@ typedef struct _extra_mapping_params {
>  typedef void (*nfs4_idmap_log_function_t)(const char *, ...);
>  
>  int nfs4_init_name_mapping(char *conffile);
> +void nfs4_term_name_mapping(void);
>  int nfs4_get_default_domain(char *server, char *domain, size_t len);
>  int nfs4_uid_to_name(uid_t uid, char *domain, char *name, size_t len);
>  int nfs4_gid_to_name(gid_t gid, char *domain, char *name, size_t len);
> diff --git a/support/nfsidmap/nfsidmap_common.c b/support/nfsidmap/nfsidmap_common.c
> index f89b82ee..4d2cb14f 100644
> --- a/support/nfsidmap/nfsidmap_common.c
> +++ b/support/nfsidmap/nfsidmap_common.c
> @@ -34,12 +34,21 @@ static char * toupper_str(char *s)
>          return s;
>  }
>  
> +static struct conf_list *local_realms = NULL;
> +
> +void free_local_realms(void)
> +{
> +	if (local_realms) {
> +		conf_free_list(local_realms);
> +		local_realms = NULL;
> +	}
> +}
> +
>  /* Get list of "local equivalent" realms.  Meaning the list of realms
>   * where john@REALM.A is considered the same user as john@REALM.B
>   * If not specified, default to upper-case of local domain name */
>  struct conf_list *get_local_realms(void)
>  {
> -	static struct conf_list *local_realms = NULL;
>  	if (local_realms) return local_realms;
>  
>  	local_realms = conf_get_list("General", "Local-Realms");
> diff --git a/support/nfsidmap/nfsidmap_private.h b/support/nfsidmap/nfsidmap_private.h
> index f1af55fa..a5cb6dda 100644
> --- a/support/nfsidmap/nfsidmap_private.h
> +++ b/support/nfsidmap/nfsidmap_private.h
> @@ -37,6 +37,7 @@
>  #include "conffile.h"
>  
>  struct conf_list *get_local_realms(void);
> +void free_local_realms(void);
>  int get_nostrip(void);
>  int get_reformat_group(void);
>  
> diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c
> index 9d46499c..f8dbb94a 100644
> --- a/support/nfsidmap/nss.c
> +++ b/support/nfsidmap/nss.c
> @@ -467,6 +467,14 @@ static int nss_plugin_init(void)
>  	return 0;
>  }
>  
> +__attribute__((destructor))
> +static int nss_plugin_term(void)
> +{
> +	free_local_realms();
> +	conf_cleanup();
> +	return 0;
> +}
> +
Just wondering... How is nss_plugin_term() called/used?

steved.

>  
>  struct trans_func nss_trans = {
>  	.name		= "nsswitch",
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit
  2020-07-20 15:49   ` Steve Dickson
@ 2020-07-20 15:58     ` Doug Nazar
  2020-07-20 17:31       ` Steve Dickson
  0 siblings, 1 reply; 20+ messages in thread
From: Doug Nazar @ 2020-07-20 15:58 UTC (permalink / raw)
  To: Steve Dickson, linux-nfs

On 2020-07-20 11:49, Steve Dickson wrote:
>
>> +__attribute__((destructor))
>> +static int nss_plugin_term(void)
>> +{
>> +	free_local_realms();
>> +	conf_cleanup();
>> +	return 0;
>> +}
>> +
> Just wondering... How is nss_plugin_term() called/used?

Automatically during dlclose(), see the 'Initialization and finalization 
functions' section of the man page. I'd originally thought to extend 
trans_func but didn't see an easy way to extend the api (no size or 
version field) without breaking any possible out of tree plugins (do 
they exist?).

Doug


^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit
  2020-07-20 15:58     ` Doug Nazar
@ 2020-07-20 17:31       ` Steve Dickson
  0 siblings, 0 replies; 20+ messages in thread
From: Steve Dickson @ 2020-07-20 17:31 UTC (permalink / raw)
  To: Doug Nazar, linux-nfs



On 7/20/20 11:58 AM, Doug Nazar wrote:
> On 2020-07-20 11:49, Steve Dickson wrote:
>>
>>> +__attribute__((destructor))
>>> +static int nss_plugin_term(void)
>>> +{
>>> +    free_local_realms();
>>> +    conf_cleanup();
>>> +    return 0;
>>> +}
>>> +
>> Just wondering... How is nss_plugin_term() called/used?
> 
> Automatically during dlclose(), see the 'Initialization and finalization functions' section of the man page. I'd originally thought to extend trans_func but didn't see an easy way to extend the api (no size or version field) without breaking any possible out of tree plugins (do they exist?).

Interesting... I think I'll add a comment explaining it... 

No..they do not exist.. The way you are going is fine... 
Less churn is good ;-) 

steved.


^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 00/11] nfs-utils: Misc cleanups & fixes
  2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
                   ` (10 preceding siblings ...)
  2020-07-18  9:24 ` [PATCH 11/11] svcgssd: Wait for nullrpc channel if not available Doug Nazar
@ 2020-07-27 14:42 ` Steve Dickson
  11 siblings, 0 replies; 20+ messages in thread
From: Steve Dickson @ 2020-07-27 14:42 UTC (permalink / raw)
  To: Doug Nazar, linux-nfs



On 7/18/20 5:24 AM, Doug Nazar wrote:
> Again, here are various cleanups and fixes. Nothing too major, although
> a couple valgrind finds.
> 
> I've left out the printf patch for now, pending further discussion.
> 
> Thanks,
> Doug
> 
> 
> Doug Nazar (11):
>   Add error handling to libevent allocations.
>   gssd: Fix cccache buffer size
>   gssd: Fix handling of failed allocations
>   gssd: srchost should never be *
>   xlog: Reorganize xlog_backend() to work around -Wmaybe-uninitialized
>   nfsdcld: Add graceful exit handling and resource cleanup
>   nfsdcld: Don't copy more data than exists in column
>   svcgssd: Convert to using libevent
>   nfsidmap: Add support to cleanup resources on exit
>   svcgssd: Cleanup global resources on exit
>   svcgssd: Wait for nullrpc channel if not available
Series committed... (tag: nfs-utils-2-5-2-rc3)

steved.
> 
>  support/nfs/xlog.c                  |  41 ++++----
>  support/nfsidmap/libnfsidmap.c      |  13 +++
>  support/nfsidmap/nfsidmap.h         |   1 +
>  support/nfsidmap/nfsidmap_common.c  |  11 ++-
>  support/nfsidmap/nfsidmap_private.h |   1 +
>  support/nfsidmap/nss.c              |   8 ++
>  utils/gssd/Makefile.am              |   2 +-
>  utils/gssd/gss_names.c              |   6 +-
>  utils/gssd/gss_util.c               |   6 ++
>  utils/gssd/gss_util.h               |   1 +
>  utils/gssd/gssd.c                   |  37 +++++--
>  utils/gssd/krb5_util.c              |  12 +--
>  utils/gssd/svcgssd.c                | 143 ++++++++++++++++++++++++++--
>  utils/gssd/svcgssd.h                |   3 +-
>  utils/gssd/svcgssd_krb5.c           |  21 ++--
>  utils/gssd/svcgssd_krb5.h           |   1 +
>  utils/gssd/svcgssd_main_loop.c      |  94 ------------------
>  utils/gssd/svcgssd_proc.c           |  15 +--
>  utils/idmapd/idmapd.c               |  32 +++++++
>  utils/nfsdcld/nfsdcld.c             |  50 +++++++++-
>  utils/nfsdcld/sqlite.c              |  33 +++++--
>  utils/nfsdcld/sqlite.h              |   1 +
>  22 files changed, 358 insertions(+), 174 deletions(-)
>  delete mode 100644 utils/gssd/svcgssd_main_loop.c
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-07-27 14:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-18  9:24 [PATCH 00/11] nfs-utils: Misc cleanups & fixes Doug Nazar
2020-07-18  9:24 ` [PATCH 01/11] Add error handling to libevent allocations Doug Nazar
2020-07-18  9:24 ` [PATCH 02/11] gssd: Fix cccache buffer size Doug Nazar
2020-07-20 14:43   ` Steve Dickson
2020-07-20 15:41     ` Doug Nazar
2020-07-18  9:24 ` [PATCH 03/11] gssd: Fix handling of failed allocations Doug Nazar
2020-07-18  9:24 ` [PATCH 04/11] gssd: srchost should never be * Doug Nazar
2020-07-18  9:24 ` [PATCH 05/11] xlog: Reorganize xlog_backend() to work around -Wmaybe-uninitialized Doug Nazar
2020-07-18  9:24 ` [PATCH 06/11] nfsdcld: Add graceful exit handling and resource cleanup Doug Nazar
2020-07-18  9:24 ` [PATCH 07/11] nfsdcld: Don't copy more data than exists in column Doug Nazar
2020-07-18  9:24 ` [PATCH 08/11] svcgssd: Convert to using libevent Doug Nazar
2020-07-18  9:24 ` [PATCH 09/11] nfsidmap: Add support to cleanup resources on exit Doug Nazar
2020-07-20 15:49   ` Steve Dickson
2020-07-20 15:58     ` Doug Nazar
2020-07-20 17:31       ` Steve Dickson
2020-07-18  9:24 ` [PATCH 10/11] svcgssd: Cleanup global " Doug Nazar
2020-07-18  9:24 ` [PATCH 11/11] svcgssd: Wait for nullrpc channel if not available Doug Nazar
2020-07-18 15:55   ` J. Bruce Fields
2020-07-18 18:07     ` Doug Nazar
2020-07-27 14:42 ` [PATCH 00/11] nfs-utils: Misc cleanups & fixes Steve Dickson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).