From 25d6eaae8d0d885add7e64b96df7a489328c6b0f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sun, 10 Dec 2006 05:23:47 +0000 Subject: 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. (This used to be commit f83b6de44f1058811ff94ac72a8a71bd8e49e4e8) --- source3/auth/auth_util.c | 17 ++++++++++------ source3/include/smb.h | 2 +- source3/smbd/conn.c | 9 --------- source3/smbd/service.c | 50 ++++++++++++++++-------------------------------- 4 files changed, 29 insertions(+), 49 deletions(-) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 7e65121b2d..870dc9b5d0 100644 --- a/source3/auth/auth_util.c +++ b/source3/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/source3/include/smb.h b/source3/include/smb.h index 765e153b37..aefc06548e 100644 --- a/source3/include/smb.h +++ b/source3/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/source3/smbd/conn.c b/source3/smbd/conn.c index f2c04662a1..19ed49e7bf 100644 --- a/source3/smbd/conn.c +++ b/source3/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/source3/smbd/service.c b/source3/smbd/service.c index 08370b1c80..9b6743f76b 100644 --- a/source3/smbd/service.c +++ b/source3/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); -- cgit