1 files changed, 233 insertions, 359 deletions

diff --git a/lib/openpam_configure.c b/lib/openpam_configure.c
index d395565aefd4..778bec899961 100644
--- a/lib/openpam_configure.c
+++ b/lib/openpam_configure.c
@@ -1,6 +1,6 @@
 /*-
  * Copyright (c) 2001-2003 Networks Associates Technology, Inc.
- * Copyright (c) 2004-2011 Dag-Erling Smørgrav
+ * Copyright (c) 2004-2012 Dag-Erling Smørgrav
  * All rights reserved.
  *
  * This software was developed for the FreeBSD Project by ThinkSec AS and
@@ -32,13 +32,15 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $Id: openpam_configure.c 500 2011-11-22 12:07:03Z des $
+ * $Id: openpam_configure.c 601 2012-04-14 20:37:45Z des $
  */
 
 #ifdef HAVE_CONFIG_H
 # include "config.h"
 #endif
 
+#include <sys/param.h>
+
 #include <ctype.h>
 #include <errno.h>
 #include <stdio.h>
@@ -48,389 +50,183 @@
 #include <security/pam_appl.h>
 
 #include "openpam_impl.h"
-#include "openpam_strlcmp.h"
+#include "openpam_ctype.h"
+#include "openpam_strlcat.h"
+#include "openpam_strlcpy.h"
 
 static int openpam_load_chain(pam_handle_t *, const char *, pam_facility_t);
 
 /*
- * Evaluates to non-zero if the argument is a linear whitespace character.
- */
-#define is_lws(ch)				\
-	(ch == ' ' || ch == '\t')
-
-/*
- * Evaluates to non-zero if the argument is a printable ASCII character.
- * Assumes that the execution character set is a superset of ASCII.
- */
-#define is_p(ch) \
-	(ch >= '!' && ch <= '~')
-
-/*
- * Returns non-zero if the argument belongs to the POSIX Portable Filename
- * Character Set.  Assumes that the execution character set is a superset
- * of ASCII.
- */
-#define is_pfcs(ch)				\
-	((ch >= '0' && ch <= '9') ||		\
-	 (ch >= 'A' && ch <= 'Z') ||		\
-	 (ch >= 'a' && ch <= 'z') ||		\
-	 ch == '.' || ch == '_' || ch == '-')
-
-/*
- * Parse the service name.
- *
- * Returns the length of the service name, or 0 if the end of the string
- * was reached or a disallowed non-whitespace character was encountered.
+ * Validate a service name.
  *
- * If parse_service_name() is successful, it updates *service to point to
- * the first character of the service name and *line to point one
- * character past the end.  If it reaches the end of the string, it
- * updates *line to point to the terminating NUL character and leaves
- * *service unmodified.  In all other cases, it leaves both *line and
- * *service unmodified.
- *
- * Allowed characters are all characters in the POSIX portable filename
- * character set.
+ * Returns a non-zero value if the argument points to a NUL-terminated
+ * string consisting entirely of characters in the POSIX portable filename
+ * character set, excluding the path separator character.
  */
 static int
-parse_service_name(char **line, char **service)
+valid_service_name(const char *name)
 {
-	char *b, *e;
+	const char *p;
 
-	for (b = *line; *b && is_lws(*b); ++b)
-		/* nothing */ ;
-	if (!*b) {
-		*line = b;
-		return (0);
+	if (OPENPAM_FEATURE(RESTRICT_SERVICE_NAME)) {
+		/* path separator not allowed */
+		for (p = name; *p != '\0'; ++p)
+			if (!is_pfcs(*p))
+				return (0);
+	} else {
+		/* path separator allowed */
+		for (p = name; *p != '\0'; ++p)
+			if (!is_pfcs(*p) && *p != '/')
+				return (0);
 	}
-	for (e = b; *e && !is_lws(*e); ++e)
-		if (!is_pfcs(*e))
-			return (0);
-	if (e == b)
-		return (0);
-	*line = e;
-	*service = b;
-	return (e - b);
+	return (1);
 }
 
 /*
  * Parse the facility name.
  *
- * Returns the corresponding pam_facility_t value, or -1 if the end of the
- * string was reached, a disallowed non-whitespace character was
- * encountered, or the first word was not a recognized facility name.
- *
- * If parse_facility_name() is successful, it updates *line to point one
- * character past the end of the facility name.  If it reaches the end of
- * the string, it updates *line to point to the terminating NUL character.
- * In all other cases, it leaves *line unmodified.
+ * Returns the corresponding pam_facility_t value, or -1 if the argument
+ * is not a valid facility name.
  */
 static pam_facility_t
-parse_facility_name(char **line)
+parse_facility_name(const char *name)
 {
-	char *b, *e;
 	int i;
 
-	for (b = *line; *b && is_lws(*b); ++b)
-		/* nothing */ ;
-	if (!*b) {
-		*line = b;
-		return ((pam_facility_t)-1);
-	}
-	for (e = b; *e && !is_lws(*e); ++e)
-		/* nothing */ ;
-	if (e == b)
-		return ((pam_facility_t)-1);
 	for (i = 0; i < PAM_NUM_FACILITIES; ++i)
-		if (strlcmp(pam_facility_name[i], b, e - b) == 0)
-			break;
-	if (i == PAM_NUM_FACILITIES)
-		return ((pam_facility_t)-1);
-	*line = e;
-	return (i);
-}
-
-/*
- * Parse the word "include".
- *
- * If the next word on the line is "include", parse_include() updates
- * *line to point one character past "include" and returns 1.  Otherwise,
- * it leaves *line unmodified and returns 0.
- */
-static int
-parse_include(char **line)
-{
-	char *b, *e;
-
-	for (b = *line; *b && is_lws(*b); ++b)
-		/* nothing */ ;
-	if (!*b) {
-		*line = b;
-		return (-1);
-	}
-	for (e = b; *e && !is_lws(*e); ++e)
-		/* nothing */ ;
-	if (e == b)
-		return (0);
-	if (strlcmp("include", b, e - b) != 0)
-		return (0);
-	*line = e;
-	return (1);
+		if (strcmp(pam_facility_name[i], name) == 0)
+			return (i);
+	return ((pam_facility_t)-1);
 }
 
 /*
  * Parse the control flag.
  *
- * Returns the corresponding pam_control_t value, or -1 if the end of the
- * string was reached, a disallowed non-whitespace character was
- * encountered, or the first word was not a recognized control flag.
- *
- * If parse_control_flag() is successful, it updates *line to point one
- * character past the end of the control flag.  If it reaches the end of
- * the string, it updates *line to point to the terminating NUL character.
- * In all other cases, it leaves *line unmodified.
+ * Returns the corresponding pam_control_t value, or -1 if the argument is
+ * not a valid control flag name.
  */
 static pam_control_t
-parse_control_flag(char **line)
+parse_control_flag(const char *name)
 {
-	char *b, *e;
 	int i;
 
-	for (b = *line; *b && is_lws(*b); ++b)
-		/* nothing */ ;
-	if (!*b) {
-		*line = b;
-		return ((pam_control_t)-1);
-	}
-	for (e = b; *e && !is_lws(*e); ++e)
-		/* nothing */ ;
-	if (e == b)
-		return ((pam_control_t)-1);
 	for (i = 0; i < PAM_NUM_CONTROL_FLAGS; ++i)
-		if (strlcmp(pam_control_flag_name[i], b, e - b) == 0)
-			break;
-	if (i == PAM_NUM_CONTROL_FLAGS)
-		return ((pam_control_t)-1);
-	*line = e;
-	return (i);
+		if (strcmp(pam_control_flag_name[i], name) == 0)
+			return (i);
+	return ((pam_control_t)-1);
 }
 
 /*
- * Parse a file name.
- *
- * Returns the length of the file name, or 0 if the end of the string was
- * reached or a disallowed non-whitespace character was encountered.
+ * Validate a file name.
  *
- * If parse_filename() is successful, it updates *filename to point to the
- * first character of the filename and *line to point one character past
- * the end.  If it reaches the end of the string, it updates *line to
- * point to the terminating NUL character and leaves *filename unmodified.
- * In all other cases, it leaves both *line and *filename unmodified.
- *
- * Allowed characters are all characters in the POSIX portable filename
- * character set, plus the path separator (forward slash).
+ * Returns a non-zero value if the argument points to a NUL-terminated
+ * string consisting entirely of characters in the POSIX portable filename
+ * character set, including the path separator character.
  */
 static int
-parse_filename(char **line, char **filename)
+valid_module_name(const char *name)
 {
-	char *b, *e;
-
-	for (b = *line; *b && is_lws(*b); ++b)
-		/* nothing */ ;
-	if (!*b) {
-		*line = b;
-		return (0);
-	}
-	for (e = b; *e && !is_lws(*e); ++e)
-		if (!is_pfcs(*e) && *e != '/')
-			return (0);
-	if (e == b)
-		return (0);
-	*line = e;
-	*filename = b;
-	return (e - b);
-}
+	const char *p;
 
-/*
- * Parse an option.
- *
- * Returns a dynamically allocated string containing the next module
- * option, or NULL if the end of the string was reached or a disallowed
- * non-whitespace character was encountered.
- *
- * If parse_option() is successful, it updates *line to point one
- * character past the end of the option.  If it reaches the end of the
- * string, it updates *line to point to the terminating NUL character.  In
- * all other cases, it leaves *line unmodified.
- *
- * If parse_option() fails to allocate memory, it will return NULL and set
- * errno to a non-zero value.
- *
- * Allowed characters for option names are all characters in the POSIX
- * portable filename character set.  Allowed characters for option values
- * are any printable non-whitespace characters.  The option value may be
- * quoted in either single or double quotes, in which case space
- * characters and whichever quote character was not used are allowed.
- * Note that the entire value must be quoted, not just part of it.
- */
-static char *
-parse_option(char **line)
-{
-	char *nb, *ne, *vb, *ve;
-	unsigned char q = 0;
-	char *option;
-	size_t size;
-
-	errno = 0;
-	for (nb = *line; *nb && is_lws(*nb); ++nb)
-		/* nothing */ ;
-	if (!*nb) {
-		*line = nb;
-		return (NULL);
-	}
-	for (ne = nb; *ne && !is_lws(*ne) && *ne != '='; ++ne)
-		if (!is_pfcs(*ne))
-			return (NULL);
-	if (ne == nb)
-		return (NULL);
-	if (*ne == '=') {
-		vb = ne + 1;
-		if (*vb == '"' || *vb == '\'')
-			q = *vb++;
-		for (ve = vb;
-		     *ve && *ve != q && (is_p(*ve) || (q && is_lws(*ve)));
-		     ++ve)
-			/* nothing */ ;
-		if (q && *ve != q)
-			/* non-printable character or missing endquote */
-			return (NULL);
-		if (q && *(ve + 1) && !is_lws(*(ve + 1)))
-			/* garbage after value */
-			return (NULL);
+	if (OPENPAM_FEATURE(RESTRICT_MODULE_NAME)) {
+		/* path separator not allowed */
+		for (p = name; *p != '\0'; ++p)
+			if (!is_pfcs(*p))
+				return (0);
 	} else {
-		vb = ve = ne;
+		/* path separator allowed */
+		for (p = name; *p != '\0'; ++p)
+			if (!is_pfcs(*p) && *p != '/')
+				return (0);
 	}
-	size = (ne - nb) + 1;
-	if (ve > vb)
-		size += (ve - vb) + 1;
-	if ((option = malloc(size)) == NULL)
-		return (NULL);
-	strncpy(option, nb, ne - nb);
-	if (ve > vb) {
-		option[ne - nb] = '=';
-		strncpy(option + (ne - nb) + 1, vb, ve - vb);
-	}
-	option[size - 1] = '\0';
-	*line = q ? ve + 1 : ve;
-	return (option);
-}
-
-/*
- * Consume trailing whitespace.
- *
- * If there are no non-whitespace characters left on the line, parse_eol()
- * updates *line to point at the terminating NUL character and returns 0.
- * Otherwise, it leaves *line unmodified and returns a non-zero value.
- */
-static int
-parse_eol(char **line)
-{
-	char *p;
-
-	for (p = *line; *p && is_lws(*p); ++p)
-		/* nothing */ ;
-	if (*p)
-		return ((unsigned char)*p);
-	*line = p;
-	return (0);
+	return (1);
 }
 
 typedef enum { pam_conf_style, pam_d_style } openpam_style_t;
 
 /*
  * Extracts given chains from a policy file.
+ *
+ * Returns the number of policy entries which were found for the specified
+ * service and facility, or -1 if a system error occurred or a syntax
+ * error was encountered.
  */
 static int
 openpam_parse_chain(pam_handle_t *pamh,
 	const char *service,
 	pam_facility_t facility,
+	FILE *f,
 	const char *filename,
 	openpam_style_t style)
 {
 	pam_chain_t *this, **next;
 	pam_facility_t fclt;
 	pam_control_t ctlf;
-	char *line, *str, *name;
-	char *option, **optv;
-	int len, lineno, ret;
-	FILE *f;
+	char *name, *servicename, *modulename;
+	int count, lineno, ret, serrno;
+	char **wordv, *word;
+	int i, wordc;
 
-	if ((f = fopen(filename, "r")) == NULL) {
-		openpam_log(errno == ENOENT ? PAM_LOG_DEBUG : PAM_LOG_NOTICE,
-		    "%s: %m", filename);
-		return (PAM_SUCCESS);
-	}
-	if (openpam_check_desc_owner_perms(filename, fileno(f)) != 0) {
-		fclose(f);
-		return (PAM_SYSTEM_ERR);
-	}
+	count = 0;
 	this = NULL;
 	name = NULL;
 	lineno = 0;
-	while ((line = openpam_readline(f, &lineno, NULL)) != NULL) {
-		/* get service name if necessary */
-		if (style == pam_conf_style) {
-			if ((len = parse_service_name(&line, &str)) == 0) {
-				openpam_log(PAM_LOG_NOTICE,
-				    "%s(%d): invalid service name (ignored)",
-				    filename, lineno);
-				FREE(line);
-				continue;
-			}
-			if (strlcmp(service, str, len) != 0) {
-				FREE(line);
-				continue;
-			}
+	wordc = 0;
+	wordv = NULL;
+	while ((wordv = openpam_readlinev(f, &lineno, &wordc)) != NULL) {
+		/* blank line? */
+		if (wordc == 0) {
+			FREEV(wordc, wordv);
+			continue;
+		}
+		i = 0;
+
+		/* check service name if necessary */
+		if (style == pam_conf_style &&
+		    strcmp(wordv[i++], service) != 0) {
+			FREEV(wordc, wordv);
+			continue;
 		}
 
-		/* get facility name */
-		if ((fclt = parse_facility_name(&line)) == (pam_facility_t)-1) {
+		/* check facility name */
+		if ((word = wordv[i++]) == NULL ||
+		    (fclt = parse_facility_name(word)) == (pam_facility_t)-1) {
 			openpam_log(PAM_LOG_ERROR,
 			    "%s(%d): missing or invalid facility",
 			    filename, lineno);
 			goto fail;
 		}
 		if (facility != fclt && facility != PAM_FACILITY_ANY) {
-			FREE(line);
+			FREEV(wordc, wordv);
 			continue;
 		}
 
 		/* check for "include" */
-		if (parse_include(&line)) {
-			if ((len = parse_service_name(&line, &str)) == 0) {
+		if ((word = wordv[i++]) != NULL &&
+		    strcmp(word, "include") == 0) {
+			if ((servicename = wordv[i++]) == NULL ||
+			    !valid_service_name(servicename)) {
 				openpam_log(PAM_LOG_ERROR,
-				    "%s(%d): missing or invalid filename",
+				    "%s(%d): missing or invalid service name",
 				    filename, lineno);
 				goto fail;
 			}
-			if ((name = strndup(str, len)) == NULL)
-				goto syserr;
-			if (parse_eol(&line) != 0) {
+			if (wordv[i] != NULL) {
 				openpam_log(PAM_LOG_ERROR,
 				    "%s(%d): garbage at end of line",
 				    filename, lineno);
 				goto fail;
 			}
-			ret = openpam_load_chain(pamh, name, fclt);
-			FREE(name);
-			if (ret != PAM_SUCCESS)
+			ret = openpam_load_chain(pamh, servicename, fclt);
+			FREEV(wordc, wordv);
+			if (ret < 0)
 				goto fail;
-			FREE(line);
 			continue;
 		}
 
 		/* get control flag */
-		if ((ctlf = parse_control_flag(&line)) == (pam_control_t)-1) {
+		if (word == NULL || /* same word we compared to "include" */
+		    (ctlf = parse_control_flag(word)) == (pam_control_t)-1) {
 			openpam_log(PAM_LOG_ERROR,
 			    "%s(%d): missing or invalid control flag",
 			    filename, lineno);
@@ -438,73 +234,76 @@ openpam_parse_chain(pam_handle_t *pamh,
 		}
 
 		/* get module name */
-		if ((len = parse_filename(&line, &str)) == 0) {
+		if ((modulename = wordv[i++]) == NULL ||
+		    !valid_module_name(modulename)) {
 			openpam_log(PAM_LOG_ERROR,
 			    "%s(%d): missing or invalid module name",
 			    filename, lineno);
 			goto fail;
 		}
-		if ((name = strndup(str, len)) == NULL)
-			goto syserr;
 
 		/* allocate new entry */
 		if ((this = calloc(1, sizeof *this)) == NULL)
 			goto syserr;
 		this->flag = ctlf;
 
-		/* get module options */
-		if ((this->optv = malloc(sizeof *optv)) == NULL)
-			goto syserr;
-		this->optc = 0;
-		while ((option = parse_option(&line)) != NULL) {
-			optv = realloc(this->optv,
-			    (this->optc + 2) * sizeof *optv);
-			if (optv == NULL)
-				goto syserr;
-			this->optv = optv;
-			this->optv[this->optc++] = option;
-		}
-		this->optv[this->optc] = NULL;
-		if (*line != '\0') {
-			openpam_log(PAM_LOG_ERROR,
-			    "%s(%d): syntax error in module options",
-			    filename, lineno);
-			goto fail;
-		}
-
 		/* load module */
-		this->module = openpam_load_module(name);
-		FREE(name);
-		if (this->module == NULL)
+		if ((this->module = openpam_load_module(modulename)) == NULL)
 			goto fail;
 
+		/*
+		 * The remaining items in wordv are the module's
+		 * arguments.  We could set this->optv = wordv + i, but
+		 * then free(this->optv) wouldn't work.  Instead, we free
+		 * the words we've already consumed, shift the rest up,
+		 * and clear the tail end of the array.
+		 */
+		this->optc = wordc - i;
+		for (i = 0; i < wordc - this->optc; ++i) {
+			FREE(wordv[i]);
+			wordv[i] = wordv[wordc - this->optc + i];
+			wordv[wordc - this->optc + i] = NULL;
+		}
+		this->optv = wordv;
+		wordv = NULL;
+		wordc = 0;
+
 		/* hook it up */
 		for (next = &pamh->chains[fclt]; *next != NULL;
 		     next = &(*next)->next)
 			/* nothing */ ;
 		*next = this;
 		this = NULL;
-
-		/* next please... */
-		FREE(line);
+		++count;
 	}
-	if (!feof(f))
+	/*
+	 * The loop ended because openpam_readword() returned NULL, which
+	 * can happen for four different reasons: an I/O error (ferror(f)
+	 * is true), a memory allocation failure (ferror(f) is false,
+	 * errno is non-zero)
+	 */
+	if (ferror(f) || errno != 0)
 		goto syserr;
+	if (!feof(f))
+		goto fail;
 	fclose(f);
-	return (PAM_SUCCESS);
+	return (count);
 syserr:
+	serrno = errno;
 	openpam_log(PAM_LOG_ERROR, "%s: %m", filename);
+	errno = serrno;
+	/* fall through */
 fail:
-	if (this && this->optc) {
-		while (this->optc--)
-			FREE(this->optv[this->optc]);
-		FREE(this->optv);
-	}
+	serrno = errno;
+	if (this && this->optc && this->optv)
+		FREEV(this->optc, this->optv);
 	FREE(this);
-	FREE(line);
+	FREEV(wordc, wordv);
+	FREE(wordv);
 	FREE(name);
 	fclose(f);
-	return (PAM_SYSTEM_ERR);
+	errno = serrno;
+	return (-1);
 }
 
 static const char *openpam_policy_path[] = {
@@ -516,37 +315,110 @@ static const char *openpam_policy_path[] = {
 };
 
 /*
+ * Read the specified chains from the specified file.
+ *
+ * Returns 0 if the file exists but does not contain any matching lines.
+ *
+ * Returns -1 and sets errno to ENOENT if the file does not exist.
+ *
+ * Returns -1 and sets errno to some other non-zero value if the file
+ * exists but is unsafe or unreadable, or an I/O error occurs.
+ */
+static int
+openpam_load_file(pam_handle_t *pamh,
+	const char *service,
+	pam_facility_t facility,
+	const char *filename,
+	openpam_style_t style)
+{
+	FILE *f;
+	int ret, serrno;
+
+	/* attempt to open the file */
+	if ((f = fopen(filename, "r")) == NULL) {
+		serrno = errno;
+		openpam_log(errno == ENOENT ? PAM_LOG_DEBUG : PAM_LOG_ERROR,
+		    "%s: %m", filename);
+		errno = serrno;
+		RETURNN(-1);
+	} else {
+		openpam_log(PAM_LOG_DEBUG, "found %s", filename);
+	}
+
+	/* verify type, ownership and permissions */
+	if (OPENPAM_FEATURE(VERIFY_POLICY_FILE) &&
+	    openpam_check_desc_owner_perms(filename, fileno(f)) != 0) {
+		/* already logged the cause */
+		serrno = errno;
+		fclose(f);
+		errno = serrno;
+		RETURNN(-1);
+	}
+
+	/* parse the file */
+	ret = openpam_parse_chain(pamh, service, facility,
+	    f, filename, style);
+	RETURNN(ret);
+}
+
+/*
  * Locates the policy file for a given service and reads the given chains
  * from it.
+ *
+ * Returns the number of policy entries which were found for the specified
+ * service and facility, or -1 if a system error occurred or a syntax
+ * error was encountered.
  */
 static int
 openpam_load_chain(pam_handle_t *pamh,
 	const char *service,
 	pam_facility_t facility)
 {
-	const char **path;
-	char *filename;
+	const char *p, **path;
+	char filename[PATH_MAX];
 	size_t len;
+	openpam_style_t style;
 	int ret;
 
+	ENTERS(facility < 0 ? "any" : pam_facility_name[facility]);
+
+	/* either absolute or relative to cwd */
+	if (strchr(service, '/') != NULL) {
+		if ((p = strrchr(service, '.')) != NULL && strcmp(p, ".conf") == 0)
+			style = pam_conf_style;
+		else
+			style = pam_d_style;
+		ret = openpam_load_file(pamh, service, facility,
+		    service, style);
+		RETURNN(ret);
+	}
+
+	/* search standard locations */
 	for (path = openpam_policy_path; *path != NULL; ++path) {
-		len = strlen(*path);
-		if ((*path)[len - 1] == '/') {
-			if (asprintf(&filename, "%s%s", *path, service) < 0) {
-				openpam_log(PAM_LOG_ERROR, "asprintf(): %m");
-				return (PAM_BUF_ERR);
+		/* construct filename */
+		len = strlcpy(filename, *path, sizeof filename);
+		if (filename[len - 1] == '/') {
+			len = strlcat(filename, service, sizeof filename);
+			if (len >= sizeof filename) {
+				errno = ENAMETOOLONG;
+				RETURNN(-1);
 			}
-			ret = openpam_parse_chain(pamh, service, facility,
-			    filename, pam_d_style);
-			FREE(filename);
+			style = pam_d_style;
 		} else {
-			ret = openpam_parse_chain(pamh, service, facility,
-			    *path, pam_conf_style);
+			style = pam_conf_style;
 		}
-		if (ret != PAM_SUCCESS)
-			return (ret);
+		ret = openpam_load_file(pamh, service, facility,
+		    filename, style);
+		/* the file exists, but an error occurred */
+		if (ret == -1 && errno != ENOENT)
+			RETURNN(ret);
+		/* in pam.d style, an empty file counts as a hit */
+		if (ret == 0 && style == pam_d_style)
+			RETURNN(ret);
 	}
-	return (PAM_SUCCESS);
+
+	/* no hit */
+	RETURNN(0);
 }
 
 /*
@@ -560,25 +432,27 @@ openpam_configure(pam_handle_t *pamh,
 	const char *service)
 {
 	pam_facility_t fclt;
-	const char *p;
+	int serrno;
 
-	for (p = service; *p; ++p)
-		if (!is_pfcs(*p))
-			return (PAM_SYSTEM_ERR);
-
-	if (openpam_load_chain(pamh, service, PAM_FACILITY_ANY) != PAM_SUCCESS)
+	ENTERS(service);
+	if (!valid_service_name(service)) {
+		openpam_log(PAM_LOG_ERROR, "invalid service name");
+		RETURNC(PAM_SYSTEM_ERR);
+	}
+	if (openpam_load_chain(pamh, service, PAM_FACILITY_ANY) < 0)
 		goto load_err;
-
 	for (fclt = 0; fclt < PAM_NUM_FACILITIES; ++fclt) {
 		if (pamh->chains[fclt] != NULL)
 			continue;
-		if (openpam_load_chain(pamh, PAM_OTHER, fclt) != PAM_SUCCESS)
+		if (openpam_load_chain(pamh, PAM_OTHER, fclt) < 0)
 			goto load_err;
 	}
-	return (PAM_SUCCESS);
+	RETURNC(PAM_SUCCESS);
 load_err:
+	serrno = errno;
 	openpam_clear_chains(pamh->chains);
-	return (PAM_SYSTEM_ERR);
+	errno = serrno;
+	RETURNC(PAM_SYSTEM_ERR);
 }
 
 /*