6483447 pam_sm_chauthtok NOT mt-safe: authtok_check/dict.c:lock_db uses alarm(2)

6548129 some pam modules can use some malloc/strdup error checking

4 files changed, 187 insertions, 180 deletions

diff --git a/usr/src/lib/pam_modules/authtok_check/dict.c b/usr/src/lib/pam_modules/authtok_check/dict.c
index ee4542aea8..fe1d6d24b5 100644
--- a/usr/src/lib/pam_modules/authtok_check/dict.c
+++ b/usr/src/lib/pam_modules/authtok_check/dict.c
@@ -2,9 +2,8 @@
  * CDDL HEADER START
  *
  * The contents of this file are subject to the terms of the
- * Common Development and Distribution License, Version 1.0 only
- * (the "License").  You may not use this file except in compliance
- * with the License.
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
  *
  * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
  * or http://www.opensolaris.org/os/licensing.
@@ -20,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2004 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -40,22 +39,15 @@ static struct flock flock = { 0, 0, 0, 0, 0, 0 };
 
 char dblock[PATH_MAX];
 
-#define	LOCK_WAIT 60
-static int timedout = 0;
-
-/*ARGSUSED*/
-void
-alarm_handler(int sig)
-{
-	timedout = 1;
-}
+#define	LOCK_WAIT	1000000
+#define	LOCK_RETRIES	60
 
 /*
  * lock_db()
  *
  * Create a lockfile to prevent simultaneous access to the database
  * creation routines. We set a timeout to LOCK_WAIT seconds. If we
- * haven't obtained a lock by that time, we bail out.
+ * haven't obtained a lock after LOCK_RETIRES attempts, we bail out.
  *
  * returns 0 on succes, -1 on (lock) failure.
  * side effect: the directory "path" will be created if it didn't exist.
@@ -63,9 +55,9 @@ alarm_handler(int sig)
 int
 lock_db(char *path)
 {
-	void (*oldhandler)(int);
 	int retval;
 	struct stat st;
+	int retries = 0;
 
 	/* create directory "path" if it doesn't exist */
 	if (stat(path, &st) == -1) {
@@ -88,17 +80,18 @@ lock_db(char *path)
 		}
 	}
 
-	flock.l_type = F_WRLCK;
-	oldhandler = sigset(SIGALRM, alarm_handler);
-	(void) alarm(LOCK_WAIT);
-	retval = fcntl(lockfd, F_SETLKW, &flock);
-	(void) alarm(0);
-	(void) sigset(SIGALRM, oldhandler);
+	do {
+		flock.l_type = F_WRLCK;
+		retval = fcntl(lockfd, F_SETLK, &flock);
+		if (retval == -1)
+			(void) usleep(LOCK_WAIT);
+	} while (retval == -1 && ++retries < LOCK_RETRIES);
 
-	if (timedout) {
+	if (retval == -1) {
+		int errno_saved = errno;
 		syslog(LOG_ERR, "pam_authtok_check::pam_sm_chauthtok: timeout "
 		    "waiting for dictionary lock.");
-		timedout = 0;
+		errno = errno_saved;
 	}
 
 	return (retval);
diff --git a/usr/src/lib/pam_modules/authtok_check/packer.c b/usr/src/lib/pam_modules/authtok_check/packer.c
index 1c34f86bca..df449a2754 100644
--- a/usr/src/lib/pam_modules/authtok_check/packer.c
+++ b/usr/src/lib/pam_modules/authtok_check/packer.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -286,8 +286,10 @@ error:
 /*
  * We merge the temporary files created by previous calls to sort_file()
  * and insert the thus sorted words into the cracklib database
+ *
+ * returns 0 on success, -1 on failure.
  */
-void
+int
 merge_files(PWDICT *pwp)
 {
 	int ti;
@@ -298,7 +300,12 @@ merge_files(PWDICT *pwp)
 	lastword[0] = '\0';
 
 	for (ti = 0; ti < tmpfp_idx; ti++)
-		words[ti] = malloc(MAXWORDLEN);
+		if ((words[ti] = malloc(MAXWORDLEN)) == NULL) {
+			while (--ti >= 0)
+				free(words[ti]);
+			return (-1);
+		}
+
 	/*
 	 * we read the first word of each of the temp-files into words[].
 	 */
@@ -337,6 +344,7 @@ merge_files(PWDICT *pwp)
 		} else
 			words[choice][MAXWORDLEN-1] = '\0';
 	}
+	return (0);
 }
 
 /*
@@ -370,7 +378,7 @@ packer(char *list, char *path)
 	free(listcopy);
 
 	if (ret == 0)
-		merge_files(pwp);
+		ret = merge_files(pwp);
 
 	(void) PWClose(pwp);
 
diff --git a/usr/src/lib/pam_modules/krb5/krb5_setcred.c b/usr/src/lib/pam_modules/krb5/krb5_setcred.c
index af251dd732..d292744a4e 100644
--- a/usr/src/lib/pam_modules/krb5/krb5_setcred.c
+++ b/usr/src/lib/pam_modules/krb5/krb5_setcred.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -100,7 +100,7 @@ pam_sm_setcred(
 	    !(flags & PAM_DELETE_CRED) &&
 	    !(flags & PAM_SILENT)) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			    "PAM-KRB5 (setcred): illegal flag %d", flags);
+		    "PAM-KRB5 (setcred): illegal flag %d", flags);
 		err = PAM_SYSTEM_ERR;
 		goto out;
 	}
@@ -123,7 +123,7 @@ pam_sm_setcred(
 		 */
 		if (flags & (PAM_REFRESH_CRED|PAM_DELETE_CRED)) {
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5 (setcred): inst kmd structure");
+			    "PAM-KRB5 (setcred): inst kmd structure");
 
 			kmd = calloc(1, sizeof (krb5_module_data_t));
 
@@ -133,7 +133,7 @@ pam_sm_setcred(
 			}
 
 			if ((err = pam_set_data(pamh, KRB5_DATA,
-				kmd, &krb5_cleanup)) != PAM_SUCCESS) {
+			    kmd, &krb5_cleanup)) != PAM_SUCCESS) {
 				free(kmd);
 				return (PAM_SYSTEM_ERR);
 			}
@@ -197,8 +197,8 @@ pam_sm_setcred(
 	 */
 	if (kmd->kcontext != NULL && kmd->debug)
 		__pam_log(LOG_AUTH | LOG_DEBUG,
-			"PAM-KRB5 (setcred): kcontext != NULL, "
-			"possible memory leak.");
+		    "PAM-KRB5 (setcred): kcontext != NULL, "
+		    "possible memory leak.");
 
 	/*
 	 * Use the authenticated and validated user, if applicable.
@@ -216,25 +216,25 @@ pam_sm_setcred(
 		if (strcmp(rep_data->type, KRB5_REPOSITORY_NAME) != 0) {
 			if (debug)
 				__pam_log(LOG_AUTH | LOG_DEBUG,
-					"PAM-KRB5 (setcred): wrong"
-					"repository found (%s), returning "
-					"PAM_IGNORE", rep_data->type);
+				    "PAM-KRB5 (setcred): wrong"
+				    "repository found (%s), returning "
+				    "PAM_IGNORE", rep_data->type);
 			return (PAM_IGNORE);
 		}
 		if (rep_data->scope_len == sizeof (krb5_repository_data_t)) {
 			krb5_data = (krb5_repository_data_t *)rep_data->scope;
 
 			if (krb5_data->flags ==
-				SUNW_PAM_KRB5_ALREADY_AUTHENTICATED &&
-				krb5_data->principal != NULL &&
-				strlen(krb5_data->principal)) {
+			    SUNW_PAM_KRB5_ALREADY_AUTHENTICATED &&
+			    krb5_data->principal != NULL &&
+			    strlen(krb5_data->principal)) {
 				if (debug)
 					__pam_log(LOG_AUTH | LOG_DEBUG,
-						"PAM-KRB5 (setcred): "
-						"Principal %s already "
-						"authenticated, "
-						"cannot setcred",
-						krb5_data->principal);
+					    "PAM-KRB5 (setcred): "
+					    "Principal %s already "
+					    "authenticated, "
+					    "cannot setcred",
+					    krb5_data->principal);
 				return (PAM_SUCCESS);
 			}
 		}
@@ -318,7 +318,7 @@ attempt_refresh_cred(
 	}
 
 	if ((code = get_kmd_kuser(kmd->kcontext, (const char *)user, kuser,
-		2*MAXHOSTNAMELEN)) != 0) {
+	    2*MAXHOSTNAMELEN)) != 0) {
 		return (code);
 	}
 
@@ -327,11 +327,11 @@ attempt_refresh_cred(
 	}
 
 	if (code = krb5_build_principal_ext(kmd->kcontext, &server,
-			    krb5_princ_realm(kmd->kcontext, me)->length,
-			    krb5_princ_realm(kmd->kcontext, me)->data,
-			    tgtname.length, tgtname.data,
-			    krb5_princ_realm(kmd->kcontext, me)->length,
-			    krb5_princ_realm(kmd->kcontext, me)->data, 0)) {
+	    krb5_princ_realm(kmd->kcontext, me)->length,
+	    krb5_princ_realm(kmd->kcontext, me)->data,
+	    tgtname.length, tgtname.data,
+	    krb5_princ_realm(kmd->kcontext, me)->length,
+	    krb5_princ_realm(kmd->kcontext, me)->data, 0)) {
 		krb5_free_principal(kmd->kcontext, me);
 		return (PAM_SYSTEM_ERR);
 	}
@@ -344,8 +344,8 @@ attempt_refresh_cred(
 	if (code) {
 		if (kmd->debug)
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5(setcred): krb5_renew_tgt() "
-				"failed: %s", error_message((errcode_t)code));
+			    "PAM-KRB5(setcred): krb5_renew_tgt() "
+			    "failed: %s", error_message((errcode_t)code));
 		return (PAM_CRED_ERR);
 	} else {
 		return (PAM_SUCCESS);
@@ -381,9 +381,9 @@ krb5_renew_tgt(
 #define	my_creds	(kmd->initcreds)
 
 	if ((flag != PAM_REFRESH_CRED) &&
-		(flag != PAM_REINITIALIZE_CRED) &&
-		(flag != PAM_ESTABLISH_CRED))
-			return (KRB5KRB_ERR_GENERIC);
+	    (flag != PAM_REINITIALIZE_CRED) &&
+	    (flag != PAM_ESTABLISH_CRED))
+		return (KRB5KRB_ERR_GENERIC);
 
 	/* this is needed only for the ktkt_warnd */
 	if ((retval = krb5_unparse_name(kmd->kcontext, me, &client_name)) != 0)
@@ -391,42 +391,42 @@ krb5_renew_tgt(
 
 	(void) memset(&creds, 0, sizeof (krb5_creds));
 	if ((retval = krb5_copy_principal(kmd->kcontext,
-				server, &creds.server))) {
+	    server, &creds.server))) {
 		if (kmd->debug)
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5 (setcred): krb5_copy_principal "
-				"failed: %s",
-				error_message((errcode_t)retval));
+			    "PAM-KRB5 (setcred): krb5_copy_principal "
+			    "failed: %s",
+			    error_message((errcode_t)retval));
 		goto cleanup_creds;
 	}
 
 	/* obtain ticket & session key */
 	retval = krb5_cc_get_principal(kmd->kcontext,
-				kmd->ccache, &creds.client);
+	    kmd->ccache, &creds.client);
 	if (retval && (kmd->debug))
 		__pam_log(LOG_AUTH | LOG_DEBUG,
-			"PAM-KRB5 (setcred): User not in cred "
-			"cache (%s)", error_message((errcode_t)retval));
+		    "PAM-KRB5 (setcred): User not in cred "
+		    "cache (%s)", error_message((errcode_t)retval));
 
 	if ((retval == KRB5_FCC_NOFILE) &&
-		(flag & (PAM_ESTABLISH_CRED|PAM_REINITIALIZE_CRED))) {
+	    (flag & (PAM_ESTABLISH_CRED|PAM_REINITIALIZE_CRED))) {
 		/*
 		 * Create a fresh ccache, and store the credentials
 		 * we got from pam_authenticate()
 		 */
 		if ((retval = krb5_cc_initialize(kmd->kcontext,
-				kmd->ccache, me)) != 0) {
+		    kmd->ccache, me)) != 0) {
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5 (setcred): krb5_cc_initialize "
-				"failed: %s",
-				error_message((errcode_t)retval));
+			    "PAM-KRB5 (setcred): krb5_cc_initialize "
+			    "failed: %s",
+			    error_message((errcode_t)retval));
 			goto cleanup_creds;
 		} else if ((retval = krb5_cc_store_cred(kmd->kcontext,
-				kmd->ccache, &my_creds)) != 0) {
+		    kmd->ccache, &my_creds)) != 0) {
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5 (setcred): krb5_cc_store_cred "
-				"failed: %s",
-				error_message((errcode_t)retval));
+			    "PAM-KRB5 (setcred): krb5_cc_store_cred "
+			    "failed: %s",
+			    error_message((errcode_t)retval));
 			goto cleanup_creds;
 		}
 	} else if (retval) {
@@ -436,10 +436,10 @@ krb5_renew_tgt(
 		 * or maybe we are looking in the wrong cache file!
 		 */
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5 (setcred): Cannot find creds"
-			" for %s (%s)",
-			client_name ? client_name : "(unknown)",
-			error_message((errcode_t)retval));
+		    "PAM-KRB5 (setcred): Cannot find creds"
+		    " for %s (%s)",
+		    client_name ? client_name : "(unknown)",
+		    error_message((errcode_t)retval));
 
 	} else if (flag & PAM_REINITIALIZE_CRED) {
 		/*
@@ -451,18 +451,18 @@ krb5_renew_tgt(
 		creds.times.endtime = my_creds.times.endtime;
 		creds.times.renew_till = my_creds.times.renew_till;
 		if ((retval = krb5_get_credentials_renew(kmd->kcontext, 0,
-				kmd->ccache, &creds, &renewed_cred))) {
+		    kmd->ccache, &creds, &renewed_cred))) {
 			if (kmd->debug)
-			    __pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5 (setcred): krb5_get_credentials",
-				"_renew(reinitialize) failed: %s",
-				error_message((errcode_t)retval));
+				__pam_log(LOG_AUTH | LOG_DEBUG,
+				    "PAM-KRB5 (setcred): krb5_get_credentials",
+				    "_renew(reinitialize) failed: %s",
+				    error_message((errcode_t)retval));
 			/* perhaps the tgt lifetime has expired */
 			if ((retval = krb5_cc_initialize(kmd->kcontext,
-					kmd->ccache, me)) != 0) {
+			    kmd->ccache, me)) != 0) {
 				goto cleanup_creds;
 			} else if ((retval = krb5_cc_store_cred(kmd->kcontext,
-					kmd->ccache, &my_creds)) != 0) {
+			    kmd->ccache, &my_creds)) != 0) {
 				goto cleanup_creds;
 			}
 		}
@@ -495,14 +495,14 @@ krb5_renew_tgt(
 		boolean_t	found = 0;
 
 		if ((retval = krb5_cc_start_seq_get(kmd->kcontext,
-				kmd->ccache, &cursor)) != 0)
+		    kmd->ccache, &cursor)) != 0)
 			goto cleanup_creds;
 
 		while ((krb5_cc_next_cred(kmd->kcontext, kmd->ccache,
-					&cursor, &nextcred) == 0)) {
+		    &cursor, &nextcred) == 0)) {
 			/* if two creds match, we just update the first */
 			if ((!found) && (creds_match(kmd->kcontext,
-						&nextcred, &creds))) {
+			    &nextcred, &creds))) {
 				/*
 				 * Mark it as found, don't store it
 				 * in the list or else it will be
@@ -516,7 +516,7 @@ krb5_renew_tgt(
 				 * in the cache later.
 				 */
 				cred_node *newnode = (cred_node *)malloc(
-						sizeof (cred_node));
+				    sizeof (cred_node));
 				if (newnode == NULL) {
 					retval = ENOMEM;
 					goto cleanup_creds;
@@ -532,14 +532,14 @@ krb5_renew_tgt(
 					fetched = fetched->next;
 				}
 				retval = krb5_copy_creds(kmd->kcontext,
-						&nextcred, &fetched->creds);
+				    &nextcred, &fetched->creds);
 				if (retval)
 					goto cleanup_creds;
 			}
 		}
 
 		if ((retval = krb5_cc_end_seq_get(kmd->kcontext,
-					kmd->ccache, &cursor)) != 0)
+		    kmd->ccache, &cursor)) != 0)
 			goto cleanup_creds;
 
 		/*
@@ -549,12 +549,12 @@ krb5_renew_tgt(
 		 */
 		if (found &&
 		    (retval = krb5_get_credentials_renew(kmd->kcontext,
-				0, kmd->ccache, &creds, &renewed_cred))) {
+		    0, kmd->ccache, &creds, &renewed_cred))) {
 			if (kmd->debug)
-			    __pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5 (setcred): krb5_get_credentials"
-				"_renew(update) failed: %s",
-				error_message((errcode_t)retval));
+				__pam_log(LOG_AUTH | LOG_DEBUG,
+				    "PAM-KRB5 (setcred): krb5_get_credentials"
+				    "_renew(update) failed: %s",
+				    error_message((errcode_t)retval));
 			/*
 			 * If we only wanted to refresh the creds but failed
 			 * due to expiration, lack of "R" flag, or other
@@ -562,10 +562,10 @@ krb5_renew_tgt(
 			 * establish new creds, add them to the cache.
 			 */
 			if ((retval = krb5_cc_initialize(kmd->kcontext,
-					kmd->ccache, me)) != 0) {
+			    kmd->ccache, me)) != 0) {
 				goto cleanup_creds;
 			} else if ((retval = krb5_cc_store_cred(kmd->kcontext,
-					kmd->ccache, &my_creds)) != 0) {
+			    kmd->ccache, &my_creds)) != 0) {
 				goto cleanup_creds;
 			}
 		}
@@ -576,7 +576,7 @@ krb5_renew_tgt(
 		 */
 		if (!found) {
 			if ((retval = krb5_cc_initialize(kmd->kcontext,
-					kmd->ccache, me)) != 0) {
+			    kmd->ccache, me)) != 0) {
 				goto cleanup_creds;
 			}
 		}
@@ -585,15 +585,16 @@ krb5_renew_tgt(
 		fetched = cred_list_head;
 		while (fetched != NULL) {
 			retval = krb5_cc_store_cred(kmd->kcontext,
-					kmd->ccache, fetched->creds);
+			    kmd->ccache, fetched->creds);
 			fetched = fetched->next;
 			if (retval) {
-			    if (kmd->debug)
-				__pam_log(LOG_AUTH | LOG_DEBUG,
-				    "PAM-KRB5(setcred): krb5_cc_store_cred() "
-				    "failed: %s",
-				    error_message((errcode_t)retval));
-			    goto cleanup_creds;
+				if (kmd->debug)
+					__pam_log(LOG_AUTH | LOG_DEBUG,
+					    "PAM-KRB5(setcred): "
+					    "krb5_cc_store_cred() "
+					    "failed: %s",
+					    error_message((errcode_t)retval));
+				goto cleanup_creds;
 			}
 		}
 	}
@@ -623,6 +624,12 @@ cleanup_creds:
 			char *filepath = NULL;
 
 			username = strdup(client_name);
+			if (username == NULL) {
+				__pam_log(LOG_AUTH | LOG_ERR,
+				    "PAM-KRB5 (setcred): Out of memory");
+				retval = KRB5KRB_ERR_GENERIC;
+				goto error;
+			}
 			if ((tmpname = strchr(username, '@')))
 				*tmpname = '\0';
 
@@ -638,9 +645,9 @@ cleanup_creds:
 			if (!(filepath = strchr(kmd->env, ':')) ||
 			    !(filepath+1)) {
 				__pam_log(LOG_AUTH | LOG_ERR,
-					"PAM-KRB5 (setcred): Invalid pathname "
-					"for credential cache of user `%s'",
-					username);
+				    "PAM-KRB5 (setcred): Invalid pathname "
+				    "for credential cache of user `%s'",
+				    username);
 				retval = KRB5KRB_ERR_GENERIC;
 				goto error;
 			}
@@ -667,13 +674,13 @@ error:
 
 		if (kmd->debug)
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5 (setcred): delete/add warning");
+			    "PAM-KRB5 (setcred): delete/add warning");
 
 		kwarn_del_warning(client_name);
 		if (kwarn_add_warning(client_name, endtime) != 0) {
 			__pam_log(LOG_AUTH | LOG_NOTICE,
-				"PAM-KRB5 (setcred): kwarn_add_warning"
-				" failed: ktkt_warnd(1M) down?");
+			    "PAM-KRB5 (setcred): kwarn_add_warning"
+			    " failed: ktkt_warnd(1M) down?");
 		}
 	}
 
@@ -699,7 +706,7 @@ creds_match(krb5_context ctx, const krb5_creds *mcreds,
 	krb5_unparse_name(ctx, creds->server, &s2);
 
 	return (krb5_principal_compare(ctx, mcreds->client, creds->client) &&
-		krb5_principal_compare(ctx, mcreds->server, creds->server));
+	    krb5_principal_compare(ctx, mcreds->server, creds->server));
 }
 
 /*
@@ -713,8 +720,8 @@ attempt_delete_initcred(krb5_module_data_t *kmd)
 
 	if (kmd->debug) {
 		__pam_log(LOG_AUTH | LOG_DEBUG,
-			"PAM-KRB5 (setcred): deleting user's "
-			"credentials (initcreds)");
+		    "PAM-KRB5 (setcred): deleting user's "
+		    "credentials (initcreds)");
 	}
 	krb5_free_cred_contents(kmd->kcontext, &kmd->initcreds);
 	(void) memset((char *)&kmd->initcreds, 0, sizeof (krb5_creds));
diff --git a/usr/src/lib/pam_modules/krb5_migrate/krb5_migrate_authenticate.c b/usr/src/lib/pam_modules/krb5_migrate/krb5_migrate_authenticate.c
index 741a02e2dc..c9d33445cb 100644
--- a/usr/src/lib/pam_modules/krb5_migrate/krb5_migrate_authenticate.c
+++ b/usr/src/lib/pam_modules/krb5_migrate/krb5_migrate_authenticate.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -52,6 +52,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 {
 	char *user = NULL;
 	char *userdata = NULL;
+	char *olduserdata = NULL;
 	char *password = NULL;
 	int err, i;
 	time_t now;
@@ -85,13 +86,12 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 		} else if (strcmp(argv[i], "expire_pw") == 0) {
 			expire_pw = 1;
 		} else if ((strstr(argv[i], "client_service=") != NULL) &&
-			    (strcmp((strstr(argv[i], "=") + 1), "") != 0)) {
-			service = (char *)strdup(strstr(argv[i], "=") + 1);
+		    (strcmp((strstr(argv[i], "=") + 1), "") != 0)) {
+			service = strdup(strstr(argv[i], "=") + 1);
 		} else {
 			__pam_log(LOG_AUTH | LOG_ERR,
-				"PAM-KRB5-AUTOMIGRATE (auth): unrecognized "
-				"option %s",
-				argv[i]);
+			    "PAM-KRB5-AUTOMIGRATE (auth): unrecognized "
+			    "option %s", argv[i]);
 		}
 	}
 
@@ -109,8 +109,17 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	if (user == NULL || (user[0] == '\0')) {
 		if (debug)
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5-AUTOMIGRATE (auth): "
-				"user empty or null");
+			    "PAM-KRB5-AUTOMIGRATE (auth): user empty or null");
+		goto cleanup;
+	}
+
+	/*
+	 * Can't tolerate memory failure later on. Get a copy
+	 * before any work is done.
+	 */
+	if ((userdata = strdup(user)) == NULL) {
+		__pam_log(LOG_AUTH | LOG_ERR,
+		    "PAM-KRB5-AUTOMIGRATE (auth): Out of memory");
 		goto cleanup;
 	}
 
@@ -125,8 +134,8 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	if (password == NULL || (password[0] == '\0')) {
 		if (debug)
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5-AUTOMIGRATE (auth): "
-				"authentication token is empty or null");
+			    "PAM-KRB5-AUTOMIGRATE (auth): "
+			    "authentication token is empty or null");
 		goto cleanup;
 	}
 
@@ -136,9 +145,8 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	 */
 	if (retval = krb5_init_context(&context)) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error initializing "
-			"krb5: %s",
-			error_message(retval));
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error initializing "
+		    "krb5: %s", error_message(retval));
 		goto cleanup;
 	}
 
@@ -147,8 +155,8 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 
 	if (def_realm == NULL && krb5_get_default_realm(context, &def_realm)) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error while obtaining "
-			"default krb5 realm");
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error while obtaining "
+		    "default krb5 realm");
 		goto cleanup;
 	}
 
@@ -156,30 +164,27 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	params.realm = def_realm;
 
 	if (kadm5_get_adm_host_srv_name(context, def_realm,
-					&kadmin_princ)) {
+	    &kadmin_princ)) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error while obtaining "
-			"host based service name for realm %s\n", def_realm);
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error while obtaining "
+		    "host based service name for realm %s\n", def_realm);
 		goto cleanup;
 	}
 
 	if (retval = krb5_sname_to_principal(context, NULL,
-				(service != NULL)?service:"host",
-				KRB5_NT_SRV_HST,
-				&svcprinc)) {
+	    (service != NULL) ? service : "host", KRB5_NT_SRV_HST, &svcprinc)) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error while creating "
-			"krb5 host service principal: %s",
-			error_message(retval));
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error while creating "
+		    "krb5 host service principal: %s",
+		    error_message(retval));
 		goto cleanup;
 	}
 
 	if (retval = krb5_unparse_name(context, svcprinc,
-				&svcprincstr)) {
+	    &svcprincstr)) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error while "
-			"unparsing principal name: %s",
-			error_message(retval));
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error while "
+		    "unparsing principal name: %s", error_message(retval));
 		krb5_free_principal(context, svcprinc);
 		goto cleanup;
 	}
@@ -190,17 +195,12 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	 * Initialize the kadm5 connection using the default keytab
 	 */
 	retval = kadm5_init_with_skey(svcprincstr, NULL,
-					kadmin_princ,
-					&params,
-					KADM5_STRUCT_VERSION,
-					KADM5_API_VERSION_2,
-					NULL,
-					&handle);
+	    kadmin_princ, &params, KADM5_STRUCT_VERSION, KADM5_API_VERSION_2,
+	    NULL, &handle);
 	if (retval) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error while "
-			"doing kadm5_init_with_skey: %s",
-			error_message(retval));
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error while "
+		    "doing kadm5_init_with_skey: %s", error_message(retval));
 		goto cleanup;
 	}
 
@@ -212,23 +212,24 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	 */
 
 	strlength = strlen(user) + strlen(def_realm) + 2;
-	userprincstr = (char *)malloc(strlength);
+	if ((userprincstr = malloc(strlength)) == NULL)
+		goto cleanup;
 	(void) strlcpy(userprincstr, user, strlength);
 	(void) strlcat(userprincstr, "@", strlength);
 	(void) strlcat(userprincstr, def_realm, strlength);
 
 
 	if (retval = krb5_parse_name(context, userprincstr,
-				&userprinc)) {
+	    &userprinc)) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error while "
-			"parsing user principal name: %s",
-			error_message(retval));
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error while "
+		    "parsing user principal name: %s",
+		    error_message(retval));
 		goto cleanup;
 	}
 
 	retval = kadm5_get_principal(handle, userprinc, &kadm5_userprinc,
-				KADM5_PRINCIPAL_NORMAL_MASK);
+	    KADM5_PRINCIPAL_NORMAL_MASK);
 
 	krb5_free_principal(context, userprinc);
 
@@ -257,20 +258,20 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 		 */
 		if (debug)
 			__pam_log(LOG_AUTH | LOG_DEBUG,
-				"PAM-KRB5-AUTOMIGRATE (auth): Principal %s "
-				"already exists in Kerberos KDC database",
-				userprincstr);
+			    "PAM-KRB5-AUTOMIGRATE (auth): Principal %s "
+			    "already exists in Kerberos KDC database",
+			    userprincstr);
 		goto cleanup;
 	}
 
 
 
 	if (retval = krb5_parse_name(context, userprincstr,
-				&(kadm5_userprinc.principal))) {
+	    &(kadm5_userprinc.principal))) {
 		__pam_log(LOG_AUTH | LOG_ERR,
-			"PAM-KRB5-AUTOMIGRATE (auth): Error while "
-			"parsing user principal name: %s",
-			error_message(retval));
+		    "PAM-KRB5-AUTOMIGRATE (auth): Error while "
+		    "parsing user principal name: %s",
+		    error_message(retval));
 		goto cleanup;
 	}
 
@@ -289,7 +290,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 
 	mask |= KADM5_PRINCIPAL;
 	retval = kadm5_create_principal(handle, &kadm5_userprinc,
-						mask, password);
+	    mask, password);
 	if (retval) {
 		switch (retval) {
 		case KADM5_AUTH_ADD:
@@ -303,9 +304,9 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 
 		default:
 			__pam_log(LOG_AUTH | LOG_ERR,
-				"PAM-KRB5-AUTOMIGRATE (auth): Generic error"
-				"while doing kadm5_create_principal: %s",
-				error_message(retval));
+			    "PAM-KRB5-AUTOMIGRATE (auth): Generic error"
+			    "while doing kadm5_create_principal: %s",
+			    error_message(retval));
 			break;
 		}
 		goto cleanup;
@@ -318,38 +319,36 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 		char messages[PAM_MAX_NUM_MSG][PAM_MAX_MSG_SIZE];
 
 		(void) snprintf(messages[0], sizeof (messages[0]),
-			dgettext(TEXT_DOMAIN, "\nUser `%s' has been "
-			"automatically migrated to the Kerberos realm %s\n"),
-			user, def_realm);
+		    dgettext(TEXT_DOMAIN, "\nUser `%s' has been "
+		    "automatically migrated to the Kerberos realm %s\n"),
+		    user, def_realm);
 		(void) __pam_display_msg(pamh, PAM_TEXT_INFO, 1,
-				messages, NULL);
+		    messages, NULL);
 	}
 	if (debug)
 		__pam_log(LOG_AUTH | LOG_DEBUG,
-			"PAM-KRB5-AUTOMIGRATE (auth): User %s "
-			"has been added to the Kerberos KDC database",
-			userprincstr);
+		    "PAM-KRB5-AUTOMIGRATE (auth): User %s "
+		    "has been added to the Kerberos KDC database",
+		    userprincstr);
 
 	/*
 	 * Since this is a new krb5 principal, do a pam_set_data()
 	 * for possible use by the acct_mgmt routine of pam_krb5(5)
 	 */
 	if (pam_get_data(pamh, KRB5_AUTOMIGRATE_DATA,
-			(const void **)&userdata) == PAM_SUCCESS) {
+	    (const void **)&olduserdata) == PAM_SUCCESS) {
 		/*
 		 * We created a princ in a previous run on the same handle and
 		 * it must have been for a different PAM_USER / princ name,
 		 * otherwise we couldn't succeed here, unless that princ
 		 * got deleted.
 		 */
-		if (userdata != NULL)
-			free(userdata);
+		if (olduserdata != NULL)
+			free(olduserdata);
 	}
-	userdata = (char *)strdup(user);
 	if (pam_set_data(pamh, KRB5_AUTOMIGRATE_DATA, userdata,
-			krb5_migrate_cleanup) != PAM_SUCCESS) {
-		if (userdata != NULL)
-			free(userdata);
+	    krb5_migrate_cleanup) != PAM_SUCCESS) {
+		free(userdata);
 	}
 
 cleanup: