6570027 memory leak in idmapd for idmap show -c winname:... code path

6573151 libidmap API should be more specific about RPC failures
6573159 idmap_config.c should use idmapdlog() to log messages instead of its own routines
6573415 Segmentation Fault in "idmap show unixname:unknownuser winname"
6573634 idmapd fails to start the reaper thread to close idle AD connections
6573752 idmapd still has syslog messages that should not be localized
6574136 libidmap should clear its handles after use for possible reuse
6575859 idmap add -u/g winname:...@* ... should be treated as an error -- @* rules must be directional
6576387 libidmap should provide macro for mapping direction

4 files changed, 103 insertions, 63 deletions

diff --git a/usr/src/lib/libidmap/common/idmap_api.c b/usr/src/lib/libidmap/common/idmap_api.c
index ff0d6fe64f..3b2e538dd1 100644
--- a/usr/src/lib/libidmap/common/idmap_api.c
+++ b/usr/src/lib/libidmap/common/idmap_api.c
@@ -213,9 +213,12 @@ idmap_udt_commit(idmap_udt_handle_t *udthandle) {
 		(xdrproc_t)xdr_idmap_update_batch, (caddr_t)&udthandle->batch,
 		(xdrproc_t)xdr_idmap_retcode, (caddr_t)&retcode,
 		TIMEOUT);
-	if (clntstat != RPC_SUCCESS) {
-		return (IDMAP_ERR_RPC);
-	}
+
+	/* reset handle so that it can be used again */
+	_IDMAP_RESET_UDT_HANDLE(udthandle);
+
+	if (clntstat != RPC_SUCCESS)
+		return (_idmap_rpc2stat(clnt));
 	if (retcode != IDMAP_SUCCESS)
 		errno = idmap_stat2errno(retcode);
 	return (retcode);
@@ -239,10 +242,10 @@ idmap_udt_add_namerule(idmap_udt_handle_t *udthandle, const char *windomain,
 		boolean_t is_user, const char *winname, const char *unixname,
 		boolean_t is_nt4, int direction) {
 	idmap_retcode	retcode;
-	idmap_namerule	*rule;
+	idmap_namerule	*rule = NULL;
 	idmap_utf8str	*str;
 
-	retcode = _udt_extend_batch(udthandle, OP_ADD_NAMERULE);
+	retcode = _udt_extend_batch(udthandle);
 	if (retcode != IDMAP_SUCCESS)
 		goto errout;
 
@@ -270,11 +273,16 @@ idmap_udt_add_namerule(idmap_udt_handle_t *udthandle, const char *windomain,
 		if (retcode != IDMAP_SUCCESS)
 			goto errout;
 	}
+
+	udthandle->batch.idmap_update_batch_val[udthandle->next].opnum =
+	    OP_ADD_NAMERULE;
 	udthandle->next++;
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_update_batch, (caddr_t)&udthandle->batch);
+	/* The batch should still be usable */
+	if (rule)
+		(void) xdr_free(xdr_idmap_namerule, (caddr_t)rule);
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -286,10 +294,10 @@ idmap_udt_rm_namerule(idmap_udt_handle_t *udthandle, boolean_t is_user,
 		const char *windomain, const char *winname,
 		const char *unixname, int direction) {
 	idmap_retcode	retcode;
-	idmap_namerule	*rule;
+	idmap_namerule	*rule = NULL;
 	idmap_utf8str	*str;
 
-	retcode = _udt_extend_batch(udthandle, OP_RM_NAMERULE);
+	retcode = _udt_extend_batch(udthandle);
 	if (retcode != IDMAP_SUCCESS)
 		goto errout;
 
@@ -316,11 +324,14 @@ idmap_udt_rm_namerule(idmap_udt_handle_t *udthandle, boolean_t is_user,
 		if (retcode != IDMAP_SUCCESS)
 			goto errout;
 	}
+	udthandle->batch.idmap_update_batch_val[udthandle->next].opnum =
+	    OP_RM_NAMERULE;
 	udthandle->next++;
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_update_batch, (caddr_t)&udthandle->batch);
+	if (rule)
+		(void) xdr_free(xdr_idmap_namerule, (caddr_t)rule);
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -331,18 +342,19 @@ idmap_stat
 idmap_udt_flush_namerules(idmap_udt_handle_t *udthandle, boolean_t is_user) {
 	idmap_retcode	retcode;
 
-	retcode = _udt_extend_batch(udthandle, OP_FLUSH_NAMERULES);
+	retcode = _udt_extend_batch(udthandle);
 	if (retcode != IDMAP_SUCCESS)
 		goto errout;
 
 	udthandle->batch.idmap_update_batch_val[udthandle->next].
 		idmap_update_op_u.is_user = is_user;
 
+	udthandle->batch.idmap_update_batch_val[udthandle->next].opnum =
+	    OP_FLUSH_NAMERULES;
 	udthandle->next++;
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_update_batch, (caddr_t)&udthandle->batch);
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -393,7 +405,7 @@ idmap_iter_namerules(idmap_handle_t *handle, const char *windomain,
 
 	rule = &arg->rule;
 	rule->is_user = is_user;
-	rule->direction = -1;
+	rule->direction = IDMAP_DIRECTION_UNDEF;
 	if (windomain) {
 		str = &rule->windomain;
 		retcode = idmap_str2utf8(&str, windomain, 0);
@@ -463,7 +475,7 @@ idmap_iter_next_namerule(idmap_iter_t *iter, char **windomain,
 	if (is_nt4)
 		*is_nt4 = 0;
 	if (direction)
-		*direction = -1;
+		*direction = IDMAP_DIRECTION_UNDEF;
 
 	__ITER_CHECK(iter, IDMAP_LIST_NAMERULES);
 
@@ -603,7 +615,7 @@ idmap_iter_next_mapping(idmap_iter_t *iter, char **sidprefix,
 	if (pid)
 		*pid = UINT32_MAX;
 	if (direction)
-		*direction = -1;
+		*direction = IDMAP_DIRECTION_UNDEF;
 
 	__ITER_CHECK(iter, IDMAP_LIST_MAPPINGS);
 
@@ -784,7 +796,7 @@ idmap_get_uidbysid(idmap_get_handle_t *gh, char *sidprefix, idmap_rid_t rid,
 		int flag, uid_t *uid, idmap_stat *stat) {
 
 	idmap_retcode	retcode;
-	idmap_mapping	*mapping;
+	idmap_mapping	*mapping = NULL;
 
 	/* sanity checks */
 	if (gh == NULL)
@@ -816,10 +828,9 @@ idmap_get_uidbysid(idmap_get_handle_t *gh, char *sidprefix, idmap_rid_t rid,
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_mapping_batch, (caddr_t)&gh->batch);
-	free(gh->retlist);
-	gh->retlist = NULL;
-	gh->next = 0;
+	/* Batch created so far should still be usable */
+	if (mapping)
+		(void) memset(mapping, 0, sizeof (*mapping));
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -844,7 +855,7 @@ idmap_get_gidbysid(idmap_get_handle_t *gh, char *sidprefix, idmap_rid_t rid,
 		int flag, gid_t *gid, idmap_stat *stat) {
 
 	idmap_retcode	retcode;
-	idmap_mapping	*mapping;
+	idmap_mapping	*mapping = NULL;
 
 	/* sanity checks */
 	if (gh == NULL)
@@ -876,10 +887,8 @@ idmap_get_gidbysid(idmap_get_handle_t *gh, char *sidprefix, idmap_rid_t rid,
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_mapping_batch, (caddr_t)&gh->batch);
-	free(gh->retlist);
-	gh->retlist = NULL;
-	gh->next = 0;
+	if (mapping)
+		(void) memset(mapping, 0, sizeof (*mapping));
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -905,7 +914,7 @@ idmap_stat
 idmap_get_pidbysid(idmap_get_handle_t *gh, char *sidprefix, idmap_rid_t rid,
 		int flag, uid_t *pid, int *is_user, idmap_stat *stat) {
 	idmap_retcode	retcode;
-	idmap_mapping	*mapping;
+	idmap_mapping	*mapping = NULL;
 
 	/* sanity checks */
 	if (gh == NULL)
@@ -939,10 +948,8 @@ idmap_get_pidbysid(idmap_get_handle_t *gh, char *sidprefix, idmap_rid_t rid,
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_mapping_batch, (caddr_t)&gh->batch);
-	free(gh->retlist);
-	gh->retlist = NULL;
-	gh->next = 0;
+	if (mapping)
+		(void) memset(mapping, 0, sizeof (*mapping));
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -967,7 +974,7 @@ idmap_get_sidbyuid(idmap_get_handle_t *gh, uid_t uid, int flag,
 		char **sidprefix, idmap_rid_t *rid, idmap_stat *stat) {
 
 	idmap_retcode	retcode;
-	idmap_mapping	*mapping;
+	idmap_mapping	*mapping = NULL;
 
 	/* sanity checks */
 	if (gh == NULL)
@@ -996,10 +1003,8 @@ idmap_get_sidbyuid(idmap_get_handle_t *gh, uid_t uid, int flag,
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_mapping_batch, (caddr_t)&gh->batch);
-	free(gh->retlist);
-	gh->retlist = NULL;
-	gh->next = 0;
+	if (mapping)
+		(void) memset(mapping, 0, sizeof (*mapping));
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -1024,7 +1029,7 @@ idmap_get_sidbygid(idmap_get_handle_t *gh, gid_t gid, int flag,
 		char **sidprefix, idmap_rid_t *rid, idmap_stat *stat) {
 
 	idmap_retcode	retcode;
-	idmap_mapping	*mapping;
+	idmap_mapping	*mapping = NULL;
 
 	/* sanity checks */
 	if (gh == NULL)
@@ -1053,10 +1058,8 @@ idmap_get_sidbygid(idmap_get_handle_t *gh, gid_t gid, int flag,
 	return (IDMAP_SUCCESS);
 
 errout:
-	(void) xdr_free(xdr_idmap_mapping_batch, (caddr_t)&gh->batch);
-	free(gh->retlist);
-	gh->retlist = NULL;
-	gh->next = 0;
+	if (mapping)
+		(void) memset(mapping, 0, sizeof (*mapping));
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
 }
@@ -1090,7 +1093,7 @@ idmap_get_mappings(idmap_get_handle_t *gh) {
 		(caddr_t)&res,
 		TIMEOUT);
 	if (clntstat != RPC_SUCCESS) {
-		retcode = IDMAP_ERR_RPC;
+		retcode = _idmap_rpc2stat(clnt);
 		goto out;
 	}
 	if (res.retcode != IDMAP_SUCCESS) {
@@ -1142,10 +1145,7 @@ idmap_get_mappings(idmap_get_handle_t *gh) {
 	retcode = IDMAP_SUCCESS;
 
 out:
-	(void) xdr_free(xdr_idmap_mapping_batch, (caddr_t)&gh->batch);
-	free(gh->retlist);
-	gh->retlist = NULL;
-	gh->next = 0;
+	_IDMAP_RESET_GET_HANDLE(gh);
 	(void) xdr_free(xdr_idmap_ids_res, (caddr_t)&res);
 	errno = idmap_stat2errno(retcode);
 	return (retcode);
@@ -1197,7 +1197,7 @@ idmap_get_w2u_mapping(idmap_handle_t *handle,
 	if (unixname)
 		*unixname = NULL;
 	if (direction)
-		*direction = -1;
+		*direction = IDMAP_DIRECTION_UNDEF;
 
 	request.flag = flag;
 	request.id1.idtype = IDMAP_SID;
@@ -1235,9 +1235,8 @@ idmap_get_w2u_mapping(idmap_handle_t *handle,
 		(xdrproc_t)xdr_idmap_mappings_res, (caddr_t)&result,
 		TIMEOUT);
 
-	if (clntstat != RPC_SUCCESS) {
-		return (IDMAP_ERR_RPC);
-	}
+	if (clntstat != RPC_SUCCESS)
+		return (_idmap_rpc2stat(clnt));
 
 	retcode = result.retcode;
 
@@ -1300,7 +1299,7 @@ idmap_get_u2w_mapping(idmap_handle_t *handle,
 	if (rid)
 		*rid = UINT32_MAX;
 	if (direction)
-		*direction = -1;
+		*direction = IDMAP_DIRECTION_UNDEF;
 
 	(void) memset(&request, 0, sizeof (request));
 	(void) memset(&result, 0, sizeof (result));
@@ -1328,9 +1327,8 @@ idmap_get_u2w_mapping(idmap_handle_t *handle,
 		(xdrproc_t)xdr_idmap_mappings_res, (caddr_t)&result,
 		TIMEOUT);
 
-	if (clntstat != RPC_SUCCESS) {
-		return (IDMAP_ERR_RPC);
-	}
+	if (clntstat != RPC_SUCCESS)
+		return (_idmap_rpc2stat(clnt));
 
 	retcode = result.retcode;
 
@@ -1342,7 +1340,7 @@ idmap_get_u2w_mapping(idmap_handle_t *handle,
 
 	if (direction)
 		*direction = mapping->direction;
-	if (sidprefix) {
+	if (sidprefix && mapping->id2.idmap_id_u.sid.prefix) {
 		*sidprefix = strdup(mapping->id2.idmap_id_u.sid.prefix);
 		if (*sidprefix == NULL) {
 			retcode = IDMAP_ERR_MEMORY;
@@ -1489,7 +1487,7 @@ static stat_table_t stattable[] = {
 	{IDMAP_ERR_NORESULT, gettext("No results available"), EINVAL},
 	{IDMAP_ERR_NOTUSER, gettext("Not a user"), EINVAL},
 	{IDMAP_ERR_NOTGROUP, gettext("Not a group"), EINVAL},
-	{IDMAP_ERR_NOTSUPPORTED, gettext("Operation not supported"), EINVAL},
+	{IDMAP_ERR_NOTSUPPORTED, gettext("Operation not supported"), ENOTSUP},
 	{IDMAP_ERR_W2U_NAMERULE,
 		gettext("Invalid Windows to UNIX name-based rule"), EINVAL},
 	{IDMAP_ERR_U2W_NAMERULE,
@@ -1499,11 +1497,11 @@ static stat_table_t stattable[] = {
 	{IDMAP_ERR_ARG, gettext("Invalid argument"), EINVAL},
 	{IDMAP_ERR_SID, gettext("Invalid SID"), EINVAL},
 	{IDMAP_ERR_IDTYPE, gettext("Invalid identity type"), EINVAL},
-	{IDMAP_ERR_RPC_HANDLE, gettext("Bad RPC handle"), EINVAL},
+	{IDMAP_ERR_RPC_HANDLE, gettext("Bad RPC handle"), EBADF},
 	{IDMAP_ERR_RPC, gettext("RPC error"), EINVAL},
 	{IDMAP_ERR_CLIENT_HANDLE, gettext("Bad client handle"), EINVAL},
-	{IDMAP_ERR_BUSY, gettext("Server is busy"), EINVAL},
-	{IDMAP_ERR_PERMISSION_DENIED, gettext("Permisssion denied"), EINVAL},
+	{IDMAP_ERR_BUSY, gettext("Server is busy"), EBUSY},
+	{IDMAP_ERR_PERMISSION_DENIED, gettext("Permisssion denied"), EACCES},
 	{IDMAP_ERR_NOMAPPING,
 		gettext("Mapping not found or inhibited"), EINVAL},
 	{IDMAP_ERR_NEW_ID_ALLOC_REQD,
diff --git a/usr/src/lib/libidmap/common/idmap_impl.h b/usr/src/lib/libidmap/common/idmap_impl.h
index 858654957d..584e42a993 100644
--- a/usr/src/lib/libidmap/common/idmap_impl.h
+++ b/usr/src/lib/libidmap/common/idmap_impl.h
@@ -61,9 +61,12 @@ struct idmap_udt_handle {
 	struct idmap_handle	*ih;
 	idmap_update_batch	batch;
 	uint64_t		next;
-	char			*lastmsg;
 };
 
+#define	_IDMAP_RESET_UDT_HANDLE(uh) \
+	(void) xdr_free(xdr_idmap_update_batch, (caddr_t)&uh->batch);\
+	uh->next = 0;
+
 typedef struct idmap_get_res {
 	idmap_id_type	idtype;
 	uid_t		*uid;
@@ -79,9 +82,15 @@ struct idmap_get_handle {
 	idmap_mapping_batch	batch;
 	idmap_get_res_t		*retlist;
 	uint64_t		next;
-	char			*lastmsg;
 };
 
+#define	_IDMAP_RESET_GET_HANDLE(gh) \
+	(void) xdr_free(xdr_idmap_mapping_batch, (caddr_t)&gh->batch);\
+	if (gh->retlist) \
+		free(gh->retlist);\
+	gh->retlist = NULL;\
+	gh->next = 0;
+
 struct idmap_iter {
 	struct idmap_handle	*ih;
 	int			type;
@@ -101,10 +110,11 @@ typedef struct stat_table {
 
 typedef idmap_retcode	_idmap_stat;
 
-extern idmap_retcode	_udt_extend_batch(idmap_udt_handle_t *, int);
+extern idmap_retcode	_udt_extend_batch(idmap_udt_handle_t *);
 extern idmap_retcode	_get_ids_extend_batch(idmap_get_handle_t *);
 extern idmap_stat	_iter_get_next_list(int, idmap_iter_t *, void *,
 				uchar_t **, size_t, xdrproc_t, xdrproc_t);
+extern idmap_stat	_idmap_rpc2stat(CLIENT *);
 
 #ifdef __cplusplus
 }
diff --git a/usr/src/lib/libidmap/common/idmap_priv.h b/usr/src/lib/libidmap/common/idmap_priv.h
index d5a54b5387..42de37b785 100644
--- a/usr/src/lib/libidmap/common/idmap_priv.h
+++ b/usr/src/lib/libidmap/common/idmap_priv.h
@@ -45,6 +45,12 @@ extern "C" {
 #define	IDMAP_FATAL_ERROR(rc)	rc == IDMAP_ERR_MEMORY ||\
 				rc == IDMAP_ERR_DB
 
+/* Direction in which mapping is valid */
+#define	IDMAP_DIRECTION_UNDEF	-1	/* not defined */
+#define	IDMAP_DIRECTION_BI	0	/* bi-directional */
+#define	IDMAP_DIRECTION_W2U	1	/* windows to unix only */
+#define	IDMAP_DIRECTION_U2W	2	/* unix to windows only */
+
 /* Opaque handle to batch config add/remove operations */
 typedef struct idmap_udt_handle idmap_udt_handle_t;
 
diff --git a/usr/src/lib/libidmap/common/utils.c b/usr/src/lib/libidmap/common/utils.c
index c9d9c69001..80b9141c47 100644
--- a/usr/src/lib/libidmap/common/utils.c
+++ b/usr/src/lib/libidmap/common/utils.c
@@ -42,7 +42,7 @@
 static struct timeval TIMEOUT = { 25, 0 };
 
 idmap_retcode
-_udt_extend_batch(idmap_udt_handle_t *udthandle, int opnum) {
+_udt_extend_batch(idmap_udt_handle_t *udthandle) {
 	idmap_update_op	*tmplist;
 	size_t		nsize;
 
@@ -61,7 +61,7 @@ _udt_extend_batch(idmap_udt_handle_t *udthandle, int opnum) {
 		udthandle->batch.idmap_update_batch_len += _UDT_SIZE_INCR;
 	}
 	udthandle->batch.idmap_update_batch_val[udthandle->next].opnum =
-		opnum;
+		OP_NONE;
 	return (IDMAP_SUCCESS);
 }
 
@@ -125,8 +125,34 @@ _iter_get_next_list(int type, idmap_iter_t *iter,
 		TIMEOUT);
 	if (clntstat != RPC_SUCCESS) {
 		free(*list);
-		return (IDMAP_ERR_RPC);
+		return (_idmap_rpc2stat(clnt));
 	}
 	iter->retlist = *list;
 	return (IDMAP_SUCCESS);
 }
+
+idmap_stat
+_idmap_rpc2stat(CLIENT *clnt) {
+	/*
+	 * We only deal with door_call(3C) errors here. We look at
+	 * r_err.re_errno instead of r_err.re_status because we need
+	 * to differentiate between RPC failures caused by bad door fd
+	 * and others.
+	 */
+	struct rpc_err r_err;
+	if (clnt) {
+		clnt_geterr(clnt, &r_err);
+		errno = r_err.re_errno;
+		switch (r_err.re_errno) {
+		case ENOMEM:
+			return (IDMAP_ERR_MEMORY);
+		case EBADF:
+			return (IDMAP_ERR_RPC_HANDLE);
+		default:
+			return (IDMAP_ERR_RPC);
+		}
+	}
+
+	/* null handle */
+	return (IDMAP_ERR_RPC_HANDLE);
+}