r20098: Properly fix issues with create_token_from_username()

reported by James. Ensure that this function allocates
everything on the temporary context except the return
memory. Never call this with a null mem context, and
now use conn->mem_ctx instead in smbd/service.c.
Remove separate free functions for conn->ngroups
and conn->nt_user_token as they are now always
talloc'ed off the conn->mem_ctx. Future optimization
will be to remove conn->mem_ctx and make all objects
pointed to in the conn struct talloc'ed off conn itself.
Easy to free then :-).
Jeremy.

4 files changed, 29 insertions, 49 deletions

diff --git a/source/auth/auth_util.c b/source/auth/auth_util.c
index 7e65121b2d9..870dc9b5d07 100644
--- a/source/auth/auth_util.c
+++ b/source/auth/auth_util.c
@@ -1130,7 +1130,7 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
 			goto unix_user;
 		}
 
-		result = pdb_enum_group_memberships(mem_ctx, sam_acct,
+		result = pdb_enum_group_memberships(tmp_ctx, sam_acct,
 						    &group_sids, &gids,
 						    &num_group_sids);
 		if (!NT_STATUS_IS_OK(result)) {
@@ -1144,6 +1144,8 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
 		SMB_ASSERT(num_group_sids > 0); 
 
 		*gid = gids[0];
+
+		/* Ensure we're returning the found_username on the right context. */
 		*found_username = talloc_strdup(mem_ctx,
 						pdb_get_username(sam_acct));
 
@@ -1178,8 +1180,7 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
 			goto done;
 		}
 
-		/* group_sids can be realloced - must be done on mem_ctx not tmp_ctx. */
-		group_sids = talloc_array(mem_ctx, DOM_SID, num_group_sids);
+		group_sids = talloc_array(tmp_ctx, DOM_SID, num_group_sids);
 		if (group_sids == NULL) {
 			DEBUG(1, ("talloc_array failed\n"));
 			result = NT_STATUS_NO_MEMORY;
@@ -1194,8 +1195,9 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
 		SMB_ASSERT(num_group_sids > 0); 
 
 		*gid = gids[0];
-		*found_username = talloc_strdup(mem_ctx, pass->pw_name);
 
+		/* Ensure we're returning the found_username on the right context. */
+		*found_username = talloc_strdup(mem_ctx, pass->pw_name);
 	} else {
 
 		/* This user is from winbind, force the primary gid to the
@@ -1208,7 +1210,7 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
 		uint32 dummy;
 
 		num_group_sids = 1;
-		group_sids = talloc_array(mem_ctx, DOM_SID, num_group_sids);
+		group_sids = talloc_array(tmp_ctx, DOM_SID, num_group_sids);
 		if (group_sids == NULL) {
 			DEBUG(1, ("talloc_array failed\n"));
 			result = NT_STATUS_NO_MEMORY;
@@ -1227,6 +1229,7 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
 
 		gids = gid;
 
+		/* Ensure we're returning the found_username on the right context. */
 		*found_username = talloc_strdup(mem_ctx, username);
 	}
 
@@ -1251,13 +1254,14 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
 				"for gid %d!\n", gids[i]));
 			continue;
 		}
-		if (!add_sid_to_array_unique( mem_ctx, &unix_group_sid,
+		if (!add_sid_to_array_unique(tmp_ctx, &unix_group_sid,
 				&group_sids, &num_group_sids )) {
 			result = NT_STATUS_NO_MEMORY;
 			goto done;
 		}
 	}
 
+	/* Ensure we're creating the nt_token on the right context. */
 	*token = create_local_nt_token(mem_ctx, &user_sid,
 				       is_guest, num_group_sids, group_sids);
 
@@ -1278,6 +1282,7 @@ NTSTATUS create_token_from_username(TALLOC_CTX *mem_ctx, const char *username,
  Expensive helper function to figure out whether a user given its name is
  member of a particular group.
 ***************************************************************************/
+
 BOOL user_in_group_sid(const char *username, const DOM_SID *group_sid)
 {
 	NTSTATUS status;
diff --git a/source/include/smb.h b/source/include/smb.h
index 765e153b379..aefc06548e3 100644
--- a/source/include/smb.h
+++ b/source/include/smb.h
@@ -547,7 +547,7 @@ struct share_iterator {
 
 typedef struct connection_struct {
 	struct connection_struct *next, *prev;
-	TALLOC_CTX *mem_ctx;
+	TALLOC_CTX *mem_ctx; /* long-lived memory context for things hanging off this struct. */
 	unsigned cnum; /* an index passed over the wire */
 	struct share_params *params;
 	BOOL force_user;
diff --git a/source/smbd/conn.c b/source/smbd/conn.c
index f2c04662a1b..19ed49e7bf4 100644
--- a/source/smbd/conn.c
+++ b/source/smbd/conn.c
@@ -268,15 +268,6 @@ void conn_free_internal(connection_struct *conn)
 		handle = thandle;
 	}
 
-	if (conn->ngroups && conn->groups) {
-		TALLOC_FREE(conn->groups);
-		conn->ngroups = 0;
-	}
-
-	if (conn->nt_user_token) {
-		TALLOC_FREE(conn->nt_user_token);
-	}
-
 	free_namearray(conn->veto_list);
 	free_namearray(conn->hide_list);
 	free_namearray(conn->veto_oplock_list);
diff --git a/source/smbd/service.c b/source/smbd/service.c
index 08370b1c800..9b6743f76bb 100644
--- a/source/smbd/service.c
+++ b/source/smbd/service.c
@@ -468,43 +468,28 @@ static NTSTATUS share_sanity_checks(int snum, fstring dev)
 	return NT_STATUS_OK;
 }
 
-static NTSTATUS find_forced_user(int snum, BOOL vuser_is_guest,
-				 uid_t *uid, gid_t *gid, fstring username,
-				 struct nt_user_token **token)
+static NTSTATUS find_forced_user(connection_struct *conn, BOOL vuser_is_guest, fstring username)
 {
-	TALLOC_CTX *mem_ctx;
+	int snum = conn->params->service;
 	char *fuser, *found_username;
-	struct nt_user_token *tmp_token;
 	NTSTATUS result;
 
-	if (!(mem_ctx = talloc_new(NULL))) {
-		DEBUG(0, ("talloc_new failed\n"));
-		return NT_STATUS_NO_MEMORY;
-	}
-
-	if (!(fuser = talloc_string_sub(mem_ctx, lp_force_user(snum), "%S",
+	if (!(fuser = talloc_string_sub(conn->mem_ctx, lp_force_user(snum), "%S",
 					lp_servicename(snum)))) {
-		TALLOC_FREE(mem_ctx);
 		return NT_STATUS_NO_MEMORY;
-		
 	}
 
-	result = create_token_from_username(mem_ctx, fuser, vuser_is_guest,
-					    uid, gid, &found_username,
-					    &tmp_token);
+	result = create_token_from_username(conn->mem_ctx, fuser, vuser_is_guest,
+					    &conn->uid, &conn->gid, &found_username,
+					    &conn->nt_user_token);
 	if (!NT_STATUS_IS_OK(result)) {
-		TALLOC_FREE(mem_ctx);
 		return result;
 	}
 
-	if (!(*token = dup_nt_token(NULL, tmp_token))) {
-		TALLOC_FREE(mem_ctx);
-		return NT_STATUS_NO_MEMORY;
-	}
-
 	fstrcpy(username, found_username);
 
-	TALLOC_FREE(mem_ctx);
+	TALLOC_FREE(fuser);
+	TALLOC_FREE(found_username);
 	return NT_STATUS_OK;
 }
 
@@ -638,6 +623,7 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 		return NULL;
 	}
 
+	conn->params->service = snum;
 	conn->nt_user_token = NULL;
 
 	if (lp_guest_only(snum)) {
@@ -654,12 +640,12 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 			*status = NT_STATUS_NO_SUCH_USER;
 			return NULL;
 		}
-		status2 = create_token_from_username(NULL, pass->pw_name, True,
+		status2 = create_token_from_username(conn->mem_ctx, pass->pw_name, True,
 						     &conn->uid, &conn->gid,
 						     &found_username,
 						     &conn->nt_user_token);
 		if (!NT_STATUS_IS_OK(status2)) {
-			TALLOC_FREE(found_username);
+			TALLOC_FREE(pass);
 			conn_free(conn);
 			*status = status2;
 			return NULL;
@@ -701,6 +687,7 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 	} else if (lp_security() == SEC_SHARE) {
 		NTSTATUS status2;
 		char *found_username = NULL;
+
 		/* add it as a possible user name if we 
 		   are in share mode security */
 		add_session_user(lp_servicename(snum));
@@ -713,12 +700,11 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 			return NULL;
 		}
 		pass = Get_Pwnam(user);
-		status2 = create_token_from_username(NULL, pass->pw_name, True,
+		status2 = create_token_from_username(conn->mem_ctx, pass->pw_name, True,
 						     &conn->uid, &conn->gid,
 						     &found_username,
 						     &conn->nt_user_token);
 		if (!NT_STATUS_IS_OK(status2)) {
-			TALLOC_FREE(found_username);
 			conn_free(conn);
 			*status = status2;
 			return NULL;
@@ -740,7 +726,6 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 		    sizeof(conn->client_address)-1);
 	conn->num_files_open = 0;
 	conn->lastused = conn->lastused_count = time(NULL);
-	conn->params->service = snum;
 	conn->used = True;
 	conn->printer = (strncmp(dev,"LPT",3) == 0);
 	conn->ipc = ( (strncmp(dev,"IPC",3) == 0) ||
@@ -778,10 +763,9 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 	if (*lp_force_user(snum)) {
 		NTSTATUS status2;
 
-		status2 = find_forced_user(snum,
-					   (vuser != NULL) && vuser->guest,
-					   &conn->uid, &conn->gid, user,
-					   &conn->nt_user_token);
+		status2 = find_forced_user(conn,
+				(vuser != NULL) && vuser->guest,
+				user);
 		if (!NT_STATUS_IS_OK(status2)) {
 			conn_free(conn);
 			*status = status2;
@@ -858,7 +842,7 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 					   sid_string_static(sid)));
 				continue;
 			}
-			if (!add_gid_to_array_unique(NULL, gid, &conn->groups,
+			if (!add_gid_to_array_unique(conn->mem_ctx, gid, &conn->groups,
 						&conn->ngroups)) {
 				DEBUG(0, ("add_gid_to_array_unique failed\n"));
 				conn_free(conn);