r22020: Make it more clear that both the vuser struct and it's contents are

talloc_free()'ed at the end of a session.

Rework the passwd cache code to use talloc_unlink and
talloc_reference, to more carefully manage the cache.

Andrew Bartlett
(This used to be commit e3e0ec25e67308de314aa61852905ee42aa2c8fe)

6 files changed, 48 insertions, 58 deletions

diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c
index 847315ef88..31178a761c 100644
--- a/source3/auth/auth_sam.c
+++ b/source3/auth/auth_sam.c
@@ -266,7 +266,7 @@ static NTSTATUS check_sam_security(const struct auth_context *auth_context,
 	/* Can't use the talloc version here, because the returned struct gets
 	   kept on the server_info */
 
-	if ( !(sampass = samu_new( NULL )) ) {
+	if ( !(sampass = samu_new( mem_ctx )) ) {
 		return NT_STATUS_NO_MEMORY;
 	}
 
diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 82567473b5..5c3bf7acce 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -561,19 +561,23 @@ NTSTATUS make_server_info_sam(auth_serversupplied_info **server_info,
 	DOM_SID unix_group_sid;
 	
 
-	if ( !(pwd = getpwnam_alloc(NULL, pdb_get_username(sampass))) ) {
-		DEBUG(1, ("User %s in passdb, but getpwnam() fails!\n",
-			  pdb_get_username(sampass)));
-		return NT_STATUS_NO_SUCH_USER;
-	}
-
 	if ( !(result = make_server_info(NULL)) ) {
 		TALLOC_FREE(pwd);
 		return NT_STATUS_NO_MEMORY;
 	}
 
+	if ( !(pwd = getpwnam_alloc(result, pdb_get_username(sampass))) ) {
+		DEBUG(1, ("User %s in passdb, but getpwnam() fails!\n",
+			  pdb_get_username(sampass)));
+		return NT_STATUS_NO_SUCH_USER;
+	}
+
 	result->sam_account = sampass;
-	result->unix_name = talloc_strdup(result, pwd->pw_name);
+	/* Ensure thaat the sampass will be freed with the result */
+	talloc_steal(result, sampass);
+	result->unix_name = pwd->pw_name;
+	/* Ensure that we keep pwd->pw_name, because we will free pwd below */
+	talloc_steal(result, pwd->pw_name);
 	result->gid = pwd->pw_gid;
 	result->uid = pwd->pw_uid;
 	
diff --git a/source3/include/smb.h b/source3/include/smb.h
index 2eed76eb80..722c34d9d3 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -1737,9 +1737,9 @@ typedef struct user_struct {
 	gid_t gid; /* gid of a validated user */
 
 	userdom_struct user;
-	char *homedir;
-	char *unix_homedir;
-	char *logon_script;
+	const char *homedir;
+	const char *unix_homedir;
+	const char *logon_script;
 	
 	BOOL guest;
 
diff --git a/source3/lib/util_pw.c b/source3/lib/util_pw.c
index dc184233a6..8b3c78a3f9 100644
--- a/source3/lib/util_pw.c
+++ b/source3/lib/util_pw.c
@@ -98,15 +98,15 @@ struct passwd *getpwnam_alloc(TALLOC_CTX *mem_ctx, const char *name)
 		i = rand() % PWNAMCACHE_SIZE;
 
 	if (pwnam_cache[i] != NULL) {
-		TALLOC_FREE(pwnam_cache[i]);
+		/* Remove this old cache entry, from the cache.  We
+		 * use talloc_unlink here because we want to be very
+		 * clear which referece we are removing */
+		talloc_unlink(pwnam_cache, pwnam_cache[i]);
 	}
 
 	pwnam_cache[i] = tcopy_passwd(pwnam_cache, temp);
-	if (pwnam_cache[i]!= NULL && mem_ctx != NULL) {
-		return (struct passwd *)talloc_reference(mem_ctx, pwnam_cache[i]);
-	}
-
-	return tcopy_passwd(NULL, pwnam_cache[i]);
+	/* while keeping this in the cache, reference a copy for the caller */
+	return (struct passwd *)talloc_reference(mem_ctx, pwnam_cache[i]);
 }
 
 struct passwd *getpwuid_alloc(TALLOC_CTX *mem_ctx, uid_t uid) 
diff --git a/source3/smbd/password.c b/source3/smbd/password.c
index bf4e9258ff..b7945bd7ea 100644
--- a/source3/smbd/password.c
+++ b/source3/smbd/password.c
@@ -92,14 +92,7 @@ void invalidate_vuid(uint16 vuid)
 	if (vuser == NULL)
 		return;
 	
-	SAFE_FREE(vuser->homedir);
-	SAFE_FREE(vuser->unix_homedir);
-	SAFE_FREE(vuser->logon_script);
-	
 	session_yield(vuser);
-	SAFE_FREE(vuser->session_keystr);
-
-	TALLOC_FREE(vuser->server_info);
 
 	data_blob_free(&vuser->session_key);
 
@@ -109,10 +102,7 @@ void invalidate_vuid(uint16 vuid)
 	   from the vuid 'owner' of connections */
 	conn_clear_vuid_cache(vuid);
 
-	SAFE_FREE(vuser->groups);
-	TALLOC_FREE(vuser->nt_user_token);
-
-	SAFE_FREE(vuser);
+	TALLOC_FREE(vuser);
 	num_validated_vuids--;
 }
 
@@ -153,7 +143,7 @@ int register_vuid(auth_serversupplied_info *server_info,
 		  DATA_BLOB session_key, DATA_BLOB response_blob,
 		  const char *smb_name)
 {
-	user_struct *vuser = NULL;
+	user_struct *vuser;
 
 	/* Paranoia check. */
 	if(lp_security() == SEC_SHARE) {
@@ -166,14 +156,12 @@ int register_vuid(auth_serversupplied_info *server_info,
 		return UID_FIELD_INVALID;
 	}
 
-	if((vuser = SMB_MALLOC_P(user_struct)) == NULL) {
-		DEBUG(0,("Failed to malloc users struct!\n"));
+	if((vuser = talloc_zero(NULL, user_struct)) == NULL) {
+		DEBUG(0,("Failed to talloc users struct!\n"));
 		data_blob_free(&session_key);
 		return UID_FIELD_INVALID;
 	}
 
-	ZERO_STRUCTP(vuser);
-
 	/* Allocate a free vuid. Yes this is a linear search... :-) */
 	while( get_valid_user_struct(next_vuid) != NULL ) {
 		next_vuid++;
@@ -203,6 +191,11 @@ int register_vuid(auth_serversupplied_info *server_info,
 		return vuser->vuid;
 	}
 
+	/* use this to keep tabs on all our info from the authentication */
+	vuser->server_info = server_info;
+	/* Ensure that the server_info will dissapear with the vuser it is now attached to */
+	talloc_steal(vuser, vuser->server_info);
+
 	/* the next functions should be done by a SID mapping system (SMS) as
 	 * the new real sam db won't have reference to unix uids or gids
 	 */
@@ -212,14 +205,13 @@ int register_vuid(auth_serversupplied_info *server_info,
 	
 	vuser->n_groups = server_info->n_groups;
 	if (vuser->n_groups) {
-		if (!(vuser->groups = (gid_t *)memdup(server_info->groups,
-						      sizeof(gid_t) *
-						      vuser->n_groups))) {
-			DEBUG(0,("register_vuid: failed to memdup "
+		if (!(vuser->groups = (gid_t *)talloc_memdup(vuser, server_info->groups,
+							     sizeof(gid_t) *
+							     vuser->n_groups))) {
+			DEBUG(0,("register_vuid: failed to talloc_memdup "
 				 "vuser->groups\n"));
 			data_blob_free(&session_key);
-			free(vuser);
-			TALLOC_FREE(server_info);
+			TALLOC_FREE(vuser);
 			return UID_FIELD_INVALID;
 		}
 	}
@@ -247,24 +239,26 @@ int register_vuid(auth_serversupplied_info *server_info,
 			const char *unix_homedir =
 				pdb_get_unix_homedir(server_info->sam_account);
 			if (unix_homedir) {
-				vuser->unix_homedir =
-					smb_xstrdup(unix_homedir);
+				vuser->unix_homedir = unix_homedir;
 			}
 		} else {
 			struct passwd *passwd =
-				getpwnam_alloc(NULL, vuser->user.unix_name);
+				getpwnam_alloc(vuser, vuser->user.unix_name);
 			if (passwd) {
-				vuser->unix_homedir =
-					smb_xstrdup(passwd->pw_dir);
+				vuser->unix_homedir = passwd->pw_dir;
+				/* Ensure that the unix_homedir now
+				 * belongs to vuser, so it goes away
+				 * with it, not with passwd below: */
+				talloc_steal(vuser, vuser->unix_homedir);
 				TALLOC_FREE(passwd);
 			}
 		}
 		
 		if (homedir) {
-			vuser->homedir = smb_xstrdup(homedir);
+			vuser->homedir = homedir;
 		}
 		if (logon_script) {
-			vuser->logon_script = smb_xstrdup(logon_script);
+			vuser->logon_script = logon_script;
 		}
 	}
 
@@ -280,23 +274,15 @@ int register_vuid(auth_serversupplied_info *server_info,
 		  vuser->user.full_name));	
 
  	if (server_info->ptok) {
-		vuser->nt_user_token = dup_nt_token(NULL, server_info->ptok);
+		vuser->nt_user_token = dup_nt_token(vuser, server_info->ptok);
 	} else {
 		DEBUG(1, ("server_info does not contain a user_token - "
 			  "cannot continue\n"));
-		TALLOC_FREE(server_info);
+		TALLOC_FREE(vuser);
 		data_blob_free(&session_key);
-		SAFE_FREE(vuser->homedir);
-		SAFE_FREE(vuser->unix_homedir);
-		SAFE_FREE(vuser->logon_script);
-
-		SAFE_FREE(vuser);
 		return UID_FIELD_INVALID;
 	}
 
-	/* use this to keep tabs on all our info from the authentication */
-	vuser->server_info = server_info;
-
 	DEBUG(3,("UNIX uid %d is UNIX user %s, and will be vuid %u\n",
 		 (int)vuser->uid,vuser->user.unix_name, vuser->vuid));
 
diff --git a/source3/smbd/session.c b/source3/smbd/session.c
index a3334a353e..05b1026f41 100644
--- a/source3/smbd/session.c
+++ b/source3/smbd/session.c
@@ -153,9 +153,9 @@ BOOL session_claim(user_struct *vuser)
 			       sessionid.id_str, sessionid.id_num);
 	}
 
-	vuser->session_keystr = SMB_STRDUP(keystr);
+	vuser->session_keystr = talloc_strdup(vuser, keystr);
 	if (!vuser->session_keystr) {
-		DEBUG(0, ("session_claim:  strdup() failed for session_keystr\n"));
+		DEBUG(0, ("session_claim:  talloc_strdup() failed for session_keystr\n"));
 		return False;
 	}
 	return True;