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)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 7e65121b2d944fe7e45a37bf7420e52ae10aaca1..870dc9b5d07891f0a5044d828897e41a0c6268b5 100644 (file)
--- 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 765e153b37978578f63d6aebf41b2d2bb7f3c83d..aefc06548e350deb64a21263b1d7c8df9252e74d 100644 (file)
--- 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 f2c04662a1b739c60e3f7a15ef4e7a5f72821e4d..19ed49e7bf4e86f038e0a650f7962281306c758c 100644 (file)
--- 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 08370b1c80014336708fde1015692b7e6997f240..9b6743f76bbc8e475e45740955051a4d22207c5f 100644 (file)
--- 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);