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
Signed-off-by: Doug Nazar <[email protected]>
---
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
Fix silly mistake on my part due to a rebase error.
Signed-off-by: Doug Nazar <[email protected]>
---
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
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 <[email protected]>
---
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
Signed-off-by: Doug Nazar <[email protected]>
---
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
Signed-off-by: Doug Nazar <[email protected]>
---
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
Signed-off-by: Doug Nazar <[email protected]>
---
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 [email protected] is considered the same user as [email protected]
* 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
Found with valgrind.
Signed-off-by: Doug Nazar <[email protected]>
---
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
Signed-off-by: Doug Nazar <[email protected]>
---
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
Signed-off-by: Doug Nazar <[email protected]>
---
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
Signed-off-by: Doug Nazar <[email protected]>
---
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
On Sat, Jul 18, 2020 at 05:24:21AM -0400, Doug Nazar wrote:
> Signed-off-by: Doug Nazar <[email protected]>
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
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
Hey,
On 7/18/20 5:24 AM, Doug Nazar wrote:
> Signed-off-by: Doug Nazar <[email protected]>
> ---
> 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 [email protected] is considered the same user as [email protected]
> * 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",
>
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
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.
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
>