[PATCH v2 0/6] nfsidmap enhancements

[PATCH v2 0/6] nfsidmap enhancements

[Chuck Lever]

Add "-d" and "-l" options, and clean up some error handling
problems.

I've decided to think more about the mechanism which sets the NFSv4
domain. Neither conf_ nor libnfsidmap has any support for modifying
a configuration file. The naive approach is to open-code editing the
configuration file, which is kind of ugly.

Build and basic operational testing on Fedora 22 and on RHEL 6
passes. Steve, IMO these are ready to be merged.


Changes since the first version:
- Tested
- Various small clean ups
- "else" is gone

---

Chuck Lever (6):
      nfsidmap: Display the effective NFSv4 domain name
      nfsidmap: Use find_key_by_type_and_desc() if available
      nfsidmap: List cached ID mapping results
      nfsidmap: Fix error handling in id_lookup()
      nfsidmap: Fix error handling in name_lookup()
      nfsidmap: Clean up other exit status cases


 utils/nfsidmap/nfsidmap.c   |  299 ++++++++++++++++++++++++++++++-------------
 utils/nfsidmap/nfsidmap.man |   74 +++++++++--
 2 files changed, 271 insertions(+), 102 deletions(-)

--
Chuck Lever

[PATCH v2 1/6] nfsidmap: Display the effective NFSv4 domain name

[Chuck Lever]

Sorry for the extensive man page changes. I added the description
for the new "-d" option, then realized there was no explanation
about what an "NFSv4 domain name" is.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 utils/nfsidmap/nfsidmap.c   |   26 +++++++++++++++++--
 utils/nfsidmap/nfsidmap.man |   59 +++++++++++++++++++++++++++++++++++--------
 2 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 1f5ba67..dd490aa 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -16,7 +16,7 @@
 #include "conffile.h"
 
 int verbose = 0;
-char *usage="Usage: %s [-v] [-c || [-u|-g|-r key] || [-t timeout] key desc]";
+char *usage = "Usage: %s [-v] [-c || [-u|-g|-r key] || -d || [-t timeout] key desc]";
 
 #define MAX_ID_LEN   11
 #define IDMAP_NAMESZ 128
@@ -37,6 +37,21 @@ static int keyring_clear(char *keyring);
 #define UIDKEYS 0x1
 #define GIDKEYS 0x2
 
+static int display_default_domain(void)
+{
+	char domain[NFS4_MAX_DOMAIN_LEN];
+	int rc;
+
+	rc = nfs4_get_default_domain(NULL, domain, NFS4_MAX_DOMAIN_LEN);
+	if (rc) {
+		xlog_errno(rc, "nfs4_get_default_domain failed: %m");
+		return EXIT_FAILURE;
+	}
+
+	printf("%s\n", domain);
+	return EXIT_SUCCESS;
+}
+
 /*
  * Find either a user or group id based on the name@domain string
  */
@@ -248,7 +263,7 @@ int main(int argc, char **argv)
 	int timeout = 600;
 	key_serial_t key;
 	char *progname, *keystr = NULL;
-	int clearing = 0, keymask = 0;
+	int clearing = 0, keymask = 0, display = 0;
 
 	/* Set the basename */
 	if ((progname = strrchr(argv[0], '/')) != NULL)
@@ -258,8 +273,11 @@ int main(int argc, char **argv)
 
 	xlog_open(progname);
 
-	while ((opt = getopt(argc, argv, "u:g:r:ct:v")) != -1) {
+	while ((opt = getopt(argc, argv, "du:g:r:ct:v")) != -1) {
 		switch (opt) {
+		case 'd':
+			display++;
+			break;
 		case 'u':
 			keymask = UIDKEYS;
 			keystr = strdup(optarg);
@@ -294,6 +312,8 @@ int main(int argc, char **argv)
 	if (!verbose)
 		verbose = conf_get_num("General", "Verbosity", 0);
 
+	if (display)
+		return display_default_domain();
 	if (keystr) {
 		rc = key_invalidate(keystr, keymask);
 		return rc;		
diff --git a/utils/nfsidmap/nfsidmap.man b/utils/nfsidmap/nfsidmap.man
index 3a3a523..04ddff6 100644
--- a/utils/nfsidmap/nfsidmap.man
+++ b/utils/nfsidmap/nfsidmap.man
@@ -11,27 +11,54 @@ nfsidmap \- The NFS idmapper upcall program
 .B "nfsidmap [-v] [-c]"
 .br
 .B "nfsidmap [-v] [-u|-g|-r user]"
+.br
+.B "nfsidmap -d"
 .SH DESCRIPTION
-The file
+The NFSv4 protocol represents the local system's UID and GID values
+on the wire as strings of the form
+.IR user@domain .
+The process of translating from UID to string and string to UID is
+referred to as "ID mapping."
+.PP
+The system derives the
+.I user
+part of the string by performing a password or group lookup.
+The lookup mechanism is configured in
+.IR /etc/idmapd.conf .
+.PP
+By default, the
+.I domain
+part of the string is the system's DNS domain name.
+It can also be specified in
+.I /etc/idmapd.conf
+if the system is multi-homed,
+or if the system's DNS domain name does
+not match the name of the system's Kerberos realm.
+.PP
+The
 .I /usr/sbin/nfsidmap
-is used by the NFS idmapper to translate user and group ids into names, and to
-translate user and group names into ids. Idmapper uses request-key to perform
-the upcall and cache the result.
+program performs translations on behalf of the kernel.
+The kernel uses the request-key mechanism to perform
+an upcall.
 .I /usr/sbin/nfsidmap
-is called by /sbin/request-key, and will perform the translation and
-initialize a key with the resulting information.
+is invoked by /sbin/request-key, performs the translation,
+and initializes a key with the resulting information.
+The kernel then caches the translation results in the key.
 .PP
 .I nfsidmap
-can also used to clear the keyring of all the keys or 
-revoke one particular key.  
-This is useful when the id mappings have failed to due 
-to a lookup error resulting in all the cached uids/gids to be set 
-to the user id nobody.
+can also clear cached ID map results in the kernel,
+or revoke one particular key.
+An incorrect cached key can result in file and directory ownership
+reverting to "nobody" on NFSv4 mount points.
 .SH OPTIONS
 .TP
 .B -c 
 Clear the keyring of all the keys.
 .TP
+.B -d
+Display the system's effective NFSv4 domain name on
+.IR stdout .
+.TP
 .B -g user
 Revoke the gid key of the given user.
 .TP
@@ -89,5 +116,15 @@ Notice that the new line was added above the line for the generic program.
 request-key will find the first matching line and run the corresponding program.
 In this case, /some/other/program will handle all uid lookups, and
 /usr/sbin/nfsidmap will handle gid, user, and group lookups.
+.SH FILES
+.TP
+.I /etc/idmapd.conf
+ID mapping configuration file
+.TP
+.I /etc/request-key.conf
+Request key configuration file
+.SH "SEE ALSO"
+.BR idmapd.conf (5),
+.BR request-key (8)
 .SH AUTHOR
 Bryan Schumaker, <bjschuma@netapp.com>

[PATCH v2 2/6] nfsidmap: Use find_key_by_type_and_desc() if available

[Chuck Lever]

Recent versions of libkeyutils have find_key_by_type_and_desc()
which replaces the open-coded keyring search in keyring_clear().

I don't quite understand what's going on in key_invalidate(),
so I didn't touch it.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 utils/nfsidmap/nfsidmap.c |  102 ++++++++++++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 44 deletions(-)

diff --git a/aclocal/keyutils.m4 b/aclocal/keyutils.m4
index a392c0e..16b225d 100644
--- a/aclocal/keyutils.m4
+++ b/aclocal/keyutils.m4
@@ -8,4 +8,8 @@ AC_DEFUN([AC_KEYUTILS], [
 
   AC_CHECK_HEADERS([keyutils.h])
 
+  AC_CHECK_LIB([keyutils], [find_key_by_type_and_desc],
+		[AC_DEFINE([HAVE_FIND_KEY_BY_TYPE_AND_DESC], [1],
+			[Define to 1 if you have the `find_key_by_type_and_desc' function.])],)
+
 ])dnl
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index dd490aa..6005125 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -1,3 +1,4 @@
+#include "config.h"
 
 #include <stdarg.h>
 #include <stdio.h>
@@ -32,11 +33,66 @@ char *usage = "Usage: %s [-v] [-c || [-u|-g|-r key] || -d || [-t timeout] key de
 #define PATH_IDMAPDCONF "/etc/idmapd.conf"
 #endif
 
-static int keyring_clear(char *keyring);
-
 #define UIDKEYS 0x1
 #define GIDKEYS 0x2
 
+#ifndef HAVE_FIND_KEY_BY_TYPE_AND_DESC
+static key_serial_t find_key_by_type_and_desc(const char *type,
+		const char *desc, key_serial_t destringid)
+{
+	char buf[BUFSIZ];
+	key_serial_t key;
+	FILE *fp;
+
+	if ((fp = fopen(PROCKEYS, "r")) == NULL) {
+		xlog_err("fopen(%s) failed: %m", PROCKEYS);
+		return -1;
+	}
+
+	key = -1;
+	while(fgets(buf, BUFSIZ, fp) != NULL) {
+		unsigned int id;
+
+		if (strstr(buf, type) == NULL)
+			continue;
+		if (strstr(buf, desc) == NULL)
+			continue;
+		if (sscanf(buf, "%x %*s", &id) != 1) {
+			xlog_err("Unparsable keyring entry in %s", PROCKEYS);
+			continue;
+		}
+
+		key = (key_serial_t)id;
+		break;
+	}
+
+	fclose(fp);
+	return key;
+}
+#endif
+
+/*
+ * Clear all the keys on the given keyring
+ */
+static int keyring_clear(const char *keyring)
+{
+	key_serial_t key;
+
+	key = find_key_by_type_and_desc("keyring", keyring, 0);
+	if (key == -1) {
+		xlog_err("'%s' keyring was not found.", keyring);
+		return EXIT_FAILURE;
+	}
+
+	if (keyctl_clear(key) < 0) {
+		xlog_err("keyctl_clear(0x%x) failed: %m",
+				(unsigned int)key);
+		return EXIT_FAILURE;
+	}
+
+	return EXIT_SUCCESS;
+}
+
 static int display_default_domain(void)
 {
 	char domain[NFS4_MAX_DOMAIN_LEN];
@@ -136,49 +192,7 @@ int name_lookup(char *id, key_serial_t key, int type)
 out:
 	return rc;
 }
-/*
- * Clear all the keys on the given keyring
- */
-static int keyring_clear(char *keyring)
-{
-	FILE *fp;
-	char buf[BUFSIZ];
-	key_serial_t key;
-
-	if (keyring == NULL)
-		keyring = DEFAULT_KEYRING;
-
-	if ((fp = fopen(PROCKEYS, "r")) == NULL) {
-		xlog_err("fopen(%s) failed: %m", PROCKEYS);
-		return 1;
-	}
 
-	while(fgets(buf, BUFSIZ, fp) != NULL) {
-		if (strstr(buf, "keyring") == NULL)
-			continue;
-		if (strstr(buf, keyring) == NULL)
-			continue;
-		if (verbose) {
-			*(strchr(buf, '\n')) = '\0';
-			xlog_warn("clearing '%s'", buf);
-		}
-		/*
-		 * The key is the first arugment in the string
-		 */
-		*(strchr(buf, ' ')) = '\0';
-		sscanf(buf, "%x", &key);
-		if (keyctl_clear(key) < 0) {
-			xlog_err("keyctl_clear(0x%x) failed: %m", key);
-			fclose(fp);
-			return 1;
-		}
-		fclose(fp);
-		return 0;
-	}
-	xlog_err("'%s' keyring was not found.", keyring);
-	fclose(fp);
-	return 1;
-}
 /*
  * Revoke a key 
  */

[PATCH v2 3/6] nfsidmap: List cached ID mapping results

[Chuck Lever]

User space can see the keys, but not their contents.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 utils/nfsidmap/nfsidmap.c   |   86 ++++++++++++++++++++++++++++++++++++++++++-
 utils/nfsidmap/nfsidmap.man |   15 ++++++++
 2 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 6005125..65b5511 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -17,7 +17,7 @@
 #include "conffile.h"
 
 int verbose = 0;
-char *usage = "Usage: %s [-v] [-c || [-u|-g|-r key] || -d || [-t timeout] key desc]";
+char *usage = "Usage: %s [-v] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]";
 
 #define MAX_ID_LEN   11
 #define IDMAP_NAMESZ 128
@@ -108,6 +108,81 @@ static int display_default_domain(void)
 	return EXIT_SUCCESS;
 }
 
+static void list_key(key_serial_t key)
+{
+	char *buffer, *c;
+	int rc;
+
+	rc = keyctl_describe_alloc(key, &buffer);
+	if (rc < 0) {
+		switch (errno) {
+		case EKEYEXPIRED:
+			printf("Expired key not displayed\n");
+			break;
+		default:
+			xlog_err("Failed to describe key: %m");
+		}
+		return;
+	}
+
+	c = strrchr(buffer, ';');
+	if (!c) {
+		xlog_err("Unparsable key not displayed\n");
+		goto out_free;
+	}
+	printf("  %s\n", ++c);
+
+out_free:
+	free(buffer);
+}
+
+static void list_keys(const char *ring_name, key_serial_t ring_id)
+{
+	key_serial_t *key;
+	void *keylist;
+	int count;
+
+	count = keyctl_read_alloc(ring_id, &keylist);
+	if (count < 0) {
+		xlog_err("Failed to read keyring %s: %m", ring_name);
+		return;
+	}
+	count /= (int)sizeof(*key);
+
+	switch (count) {
+	case 0:
+		printf("No %s keys found.\n", ring_name);
+		break;
+	case 1:
+		printf("1 %s key found:\n", ring_name);
+		break;
+	default:
+		printf("%u %s keys found:\n", count, ring_name);
+	}
+
+	for (key = keylist; count--; key++)
+		list_key(*key);
+
+	free(keylist);
+}
+
+/*
+ * List all keys on a keyring
+ */
+static int list_keyring(const char *keyring)
+{
+	key_serial_t key;
+
+	key = find_key_by_type_and_desc("keyring", keyring, 0);
+	if (key == -1) {
+		xlog_err("'%s' keyring was not found.", keyring);
+		return EXIT_FAILURE;
+	}
+
+	list_keys(keyring, key);
+	return EXIT_SUCCESS;
+}
+
 /*
  * Find either a user or group id based on the name@domain string
  */
@@ -277,7 +352,7 @@ int main(int argc, char **argv)
 	int timeout = 600;
 	key_serial_t key;
 	char *progname, *keystr = NULL;
-	int clearing = 0, keymask = 0, display = 0;
+	int clearing = 0, keymask = 0, display = 0, list = 0;
 
 	/* Set the basename */
 	if ((progname = strrchr(argv[0], '/')) != NULL)
@@ -287,11 +362,14 @@ int main(int argc, char **argv)
 
 	xlog_open(progname);
 
-	while ((opt = getopt(argc, argv, "du:g:r:ct:v")) != -1) {
+	while ((opt = getopt(argc, argv, "du:g:r:ct:vl")) != -1) {
 		switch (opt) {
 		case 'd':
 			display++;
 			break;
+		case 'l':
+			list++;
+			break;
 		case 'u':
 			keymask = UIDKEYS;
 			keystr = strdup(optarg);
@@ -328,6 +406,8 @@ int main(int argc, char **argv)
 
 	if (display)
 		return display_default_domain();
+	if (list)
+		return list_keyring(DEFAULT_KEYRING);
 	if (keystr) {
 		rc = key_invalidate(keystr, keymask);
 		return rc;		
diff --git a/utils/nfsidmap/nfsidmap.man b/utils/nfsidmap/nfsidmap.man
index 04ddff6..0275bdf 100644
--- a/utils/nfsidmap/nfsidmap.man
+++ b/utils/nfsidmap/nfsidmap.man
@@ -13,6 +13,8 @@ nfsidmap \- The NFS idmapper upcall program
 .B "nfsidmap [-v] [-u|-g|-r user]"
 .br
 .B "nfsidmap -d"
+.br
+.B "nfsidmap -l"
 .SH DESCRIPTION
 The NFSv4 protocol represents the local system's UID and GID values
 on the wire as strings of the form
@@ -50,6 +52,13 @@ can also clear cached ID map results in the kernel,
 or revoke one particular key.
 An incorrect cached key can result in file and directory ownership
 reverting to "nobody" on NFSv4 mount points.
+.PP
+In addition, the
+.B -d
+and
+.B -l
+options are available to help diagnose misconfigurations.
+They have no effect on the keyring containing ID mapping results.
 .SH OPTIONS
 .TP
 .B -c 
@@ -62,6 +71,12 @@ Display the system's effective NFSv4 domain name on
 .B -g user
 Revoke the gid key of the given user.
 .TP
+.B -l
+Display on
+.I stdout
+all keys currently in the keyring used to cache ID mapping results.
+These keys are visible only to the superuser.
+.TP
 .B -r user
 Revoke both the uid and gid key of the given user.
 .TP

[PATCH v2 4/6] nfsidmap: Fix error handling in id_lookup()

[Chuck Lever]

As near as I can tell, the exit status of nfsidmap is supposed to be
zero (success) or one (failure).

The return value of id_lookup() becomes the exit status, so it
should return only zero or one.

The libnfsidmap calls return a signed integer, either 0 or negative
errno values. These have to be translated to an exit status.

libkeyutils calls return a signed long, either 0 or -1. These also
have to be translated to an exit status.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 utils/nfsidmap/nfsidmap.c |   41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 65b5511..9468c08 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -186,7 +186,7 @@ static int list_keyring(const char *keyring)
 /*
  * Find either a user or group id based on the name@domain string
  */
-int id_lookup(char *name_at_domain, key_serial_t key, int type)
+static int id_lookup(char *name_at_domain, key_serial_t key, int type)
 {
 	char id[MAX_ID_LEN];
 	uid_t uid = 0;
@@ -200,30 +200,33 @@ int id_lookup(char *name_at_domain, key_serial_t key, int type)
 		rc = nfs4_group_owner_to_gid(name_at_domain, &gid);
 		sprintf(id, "%u", gid);
 	}
-	if (rc < 0)
+	if (rc < 0) {
 		xlog_errno(rc, "id_lookup: %s: failed: %m",
 			(type == USER ? "nfs4_owner_to_uid" : "nfs4_group_owner_to_gid"));
+		return EXIT_FAILURE;
+	}
 
-	if (rc == 0) {
-		rc = keyctl_instantiate(key, id, strlen(id) + 1, 0);
-		if (rc < 0) {
-			switch(rc) {
-			case -EDQUOT:
-			case -ENFILE:
-			case -ENOMEM:
-				/*
-			 	 * The keyring is full. Clear the keyring and try again
-			 	 */
-				rc = keyring_clear(DEFAULT_KEYRING);
-				if (rc == 0)
-					rc = keyctl_instantiate(key, id, strlen(id) + 1, 0);
-				break;
-			default:
+	rc = EXIT_SUCCESS;
+	if (keyctl_instantiate(key, id, strlen(id) + 1, 0)) {
+		switch (errno) {
+		case EDQUOT:
+		case ENFILE:
+		case ENOMEM:
+			/*
+			 * The keyring is full. Clear the keyring and try again
+			 */
+			rc = keyring_clear(DEFAULT_KEYRING);
+			if (rc)
 				break;
+			if (keyctl_instantiate(key, id, strlen(id) + 1, 0)) {
+				rc = EXIT_FAILURE;
+				xlog_err("id_lookup: keyctl_instantiate failed: %m");
 			}
+			break;
+		default:
+			rc = EXIT_FAILURE;
+			break;
 		}
-		if (rc < 0)
-			xlog_err("id_lookup: keyctl_instantiate failed: %m");
 	}
 
 	return rc;

[PATCH v2 5/6] nfsidmap: Fix error handling in name_lookup()

[Chuck Lever]

As near as I can tell, the exit status of nfsidmap is supposed to be
zero (success) or one (failure).

The return value of name_lookup() becomes the exit status, so it
should return only zero or one.

The libnfsidmap calls return a signed integer, either 0 or negative
errno values. These have to be translated to an exit status.

libkeyutils calls return a signed long, either 0 or -1. These also
have to be translated to an exit status.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 utils/nfsidmap/nfsidmap.c |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 9468c08..dbfac9b 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -235,7 +235,7 @@ static int id_lookup(char *name_at_domain, key_serial_t key, int type)
 /*
  * Find the name@domain string from either a user or group id
  */
-int name_lookup(char *id, key_serial_t key, int type)
+static int name_lookup(char *id, key_serial_t key, int type)
 {
 	char name[IDMAP_NAMESZ];
 	char domain[NFS4_MAX_DOMAIN_LEN];
@@ -244,11 +244,10 @@ int name_lookup(char *id, key_serial_t key, int type)
 	int rc;
 
 	rc = nfs4_get_default_domain(NULL, domain, NFS4_MAX_DOMAIN_LEN);
-	if (rc != 0) {
+	if (rc) {
 		xlog_errno(rc,
 			"name_lookup: nfs4_get_default_domain failed: %m");
-		rc = -1;
-		goto out;
+		return EXIT_FAILURE;
 	}
 
 	if (type == USER) {
@@ -258,16 +257,18 @@ int name_lookup(char *id, key_serial_t key, int type)
 		gid = atoi(id);
 		rc = nfs4_gid_to_name(gid, domain, name, IDMAP_NAMESZ);
 	}
-	if (rc < 0)
+	if (rc) {
 		xlog_errno(rc, "name_lookup: %s: failed: %m",
 			(type == USER ? "nfs4_uid_to_name" : "nfs4_gid_to_name"));
+		return EXIT_FAILURE;
+	}
 
-	if (rc == 0) {
-		rc = keyctl_instantiate(key, &name, strlen(name), 0);
-		if (rc < 0)
-			xlog_err("name_lookup: keyctl_instantiate failed: %m");
+	rc = EXIT_SUCCESS;
+	if (keyctl_instantiate(key, &name, strlen(name), 0)) {
+		rc = EXIT_FAILURE;
+		xlog_err("name_lookup: keyctl_instantiate failed: %m");
 	}
-out:
+
 	return rc;
 }
 

[PATCH v2 6/6] nfsidmap: Clean up other exit status cases

[Chuck Lever]

Make it unambiguous where 0 or 1 represent an exit status.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 utils/nfsidmap/nfsidmap.c |   29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index dbfac9b..52e634f 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -286,7 +286,7 @@ static int key_invalidate(char *keystr, int keymask)
 
 	if ((fp = fopen(PROCKEYS, "r")) == NULL) {
 		xlog_err("fopen(%s) failed: %m", PROCKEYS);
-		return 1;
+		return EXIT_FAILURE;
 	}
 
 	while(fgets(buf, BUFSIZ, fp) != NULL) {
@@ -325,13 +325,13 @@ static int key_invalidate(char *keystr, int keymask)
 			if (errno != EOPNOTSUPP) {
 				xlog_err("keyctl_invalidate(0x%x) failed: %m", key);
 				fclose(fp);
-				return 1;
+				return EXIT_FAILURE;
 			} else {
 				/* older kernel compatibility attempt: */
 				if (keyctl_revoke(key) < 0) {
 					xlog_err("keyctl_revoke(0x%x) failed: %m", key);
 					fclose(fp);
-					return 1;
+					return EXIT_FAILURE;
 				}
 			}
 		}
@@ -339,12 +339,12 @@ static int key_invalidate(char *keystr, int keymask)
 		keymask &= ~mask;
 		if (keymask == 0) {
 			fclose(fp);
-			return 0;
+			return EXIT_SUCCESS;
 		}
 	}
 	xlog_err("'%s' key was not found.", keystr);
 	fclose(fp);
-	return 1;
+	return EXIT_FAILURE;
 }
 
 int main(int argc, char **argv)
@@ -403,7 +403,7 @@ int main(int argc, char **argv)
 
 	if ((rc = nfs4_init_name_mapping(PATH_IDMAPDCONF)))  {
 		xlog_errno(rc, "Unable to create name to user id mappings.");
-		return 1;
+		return EXIT_FAILURE;
 	}
 	if (!verbose)
 		verbose = conf_get_num("General", "Verbosity", 0);
@@ -413,20 +413,18 @@ int main(int argc, char **argv)
 	if (list)
 		return list_keyring(DEFAULT_KEYRING);
 	if (keystr) {
-		rc = key_invalidate(keystr, keymask);
-		return rc;		
+		return key_invalidate(keystr, keymask);
 	}
 	if (clearing) {
 		xlog_syslog(0);
-		rc = keyring_clear(DEFAULT_KEYRING);
-		return rc;		
+		return keyring_clear(DEFAULT_KEYRING);
 	}
 
 	xlog_stderr(0);
 	if ((argc - optind) != 2) {
 		xlog_err("Bad arg count. Check /etc/request-key.conf");
 		xlog_warn(usage, progname);
-		return 1;
+		return EXIT_FAILURE;
 	}
 
 	if (verbose)
@@ -437,13 +435,14 @@ int main(int argc, char **argv)
 	arg = strdup(argv[optind]);
 	if (arg == NULL) {
 		xlog_err("strdup failed: %m");
-		return 1;
+		return EXIT_FAILURE;
 	}
 	type = strtok(arg, ":");
 	value = strtok(NULL, ":");
-    if (value == NULL) {
+	if (value == NULL) {
+		free(arg);
 		xlog_err("Error: Null uid/gid value.");
-		return 1;
+		return EXIT_FAILURE;
 	}
 	if (verbose) {
 		xlog_warn("key: 0x%lx type: %s value: %s timeout %ld",
@@ -463,7 +462,7 @@ int main(int argc, char **argv)
 		rc = name_lookup(value, key, GROUP);
 
 	/* Set timeout to 10 (600 seconds) minutes */
-	if (rc == 0)
+	if (rc == EXIT_SUCCESS)
 		keyctl_set_timeout(key, timeout);
 
 	free(arg);

Re: [PATCH v2 0/6] nfsidmap enhancements

[Steve Dickson]

On 08/05/2015 10:45 AM, Chuck Lever wrote:
> Add "-d" and "-l" options, and clean up some error handling
> problems.
> 
> I've decided to think more about the mechanism which sets the NFSv4
> domain. Neither conf_ nor libnfsidmap has any support for modifying
> a configuration file. The naive approach is to open-code editing the
> configuration file, which is kind of ugly.
> 
> Build and basic operational testing on Fedora 22 and on RHEL 6
> passes. Steve, IMO these are ready to be merged.
> 
> 
> Changes since the first version:
> - Tested
> - Various small clean ups
> - "else" is gone
> 
> ---
> 
> Chuck Lever (6):
>       nfsidmap: Display the effective NFSv4 domain name
>       nfsidmap: Use find_key_by_type_and_desc() if available
>       nfsidmap: List cached ID mapping results
>       nfsidmap: Fix error handling in id_lookup()
>       nfsidmap: Fix error handling in name_lookup()
>       nfsidmap: Clean up other exit status cases
> 
> 
>  utils/nfsidmap/nfsidmap.c   |  299 ++++++++++++++++++++++++++++++-------------
>  utils/nfsidmap/nfsidmap.man |   74 +++++++++--
>  2 files changed, 271 insertions(+), 102 deletions(-)
All six committed... 

steved.

> 
> --
> Chuck Lever
>