Change the passdb interface to use allocated strings.

These strings are allocated using talloc(), either using its own memory context
stored on the SAM_ACCOUNT or one supplied by the caller.

The pdb_init_sam() and pdb_free_sam() function have been modifed so that a call
to pdb_free_sam() will either clean up (remove hashes from memory) and destroy
the TALLOC_CTX or just clean up depending on who supplied it.

The pdb_init_sam and pdb_free_sam functions now also return an NTSTATUS, and I
have modified the 3 places that actually checked these returns.

The only nasty thing about this patch is the small measure needed to maintin
interface compatability - strings set to NULL are actually set to "".

This is becouse there are too many places in Samba that do strlen() on these
strings without checking if they are NULL pointers.

A supp patch will follow to set all strings to "" in pdb_default_sam().

Andrew Bartlett
(This used to be commit 144345b41d39a6f68d01f62b7aee64ca0d328085)

6 files changed, 180 insertions, 129 deletions

diff --git a/source3/include/smb.h b/source3/include/smb.h
index 4a9a6ccd3b..4c13f0e4a6 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -664,7 +664,11 @@ typedef struct {
 		
 typedef struct sam_passwd
 {
-	struct {
+	TALLOC_CTX *mem_ctx;
+	
+	void (*free_fn)(struct sam_passwd **);
+
+	struct user_data {
 		/* initiailization flags */
 		uint32 init_flag;
 		
@@ -675,18 +679,18 @@ typedef struct sam_passwd
 		time_t pass_can_change_time;  /* password can change time */
 		time_t pass_must_change_time; /* password must change time */
 		
-		pstring username;     /* UNIX username string */
-		pstring domain;       /* Windows Domain name */
-		pstring nt_username;  /* Windows username string */
-		pstring full_name;    /* user's full name string */
-		pstring home_dir;     /* home directory string */
-		pstring dir_drive;    /* home directory drive string */
-		pstring logon_script; /* logon script string */
-		pstring profile_path; /* profile path string */
-		pstring acct_desc  ;  /* user description string */
-		pstring workstations; /* login from workstations string */
-		pstring unknown_str ; /* don't know what this is, yet. */
-		pstring munged_dial ; /* munged path name and dial-back tel number */
+		char * username;     /* UNIX username string */
+		char * domain;       /* Windows Domain name */
+		char * nt_username;  /* Windows username string */
+		char * full_name;    /* user's full name string */
+		char * home_dir;     /* home directory string */
+		char * dir_drive;    /* home directory drive string */
+		char * logon_script; /* logon script string */
+		char * profile_path; /* profile path string */
+		char * acct_desc  ;  /* user description string */
+		char * workstations; /* login from workstations string */
+		char * unknown_str ; /* don't know what this is, yet. */
+		char * munged_dial ; /* munged path name and dial-back tel number */
 		
 		uid_t uid;          /* this is a unix uid_t */
 		gid_t gid;          /* this is a unix gid_t */
diff --git a/source3/passdb/passdb.c b/source3/passdb/passdb.c
index 1dd0d4141a..39a8fb3676 100644
--- a/source3/passdb/passdb.c
+++ b/source3/passdb/passdb.c
@@ -58,14 +58,9 @@ BOOL initialize_password_db(BOOL reload)
  Fill the SAM_ACCOUNT with default values.
  ***********************************************************/
 
-static BOOL pdb_fill_default_sam(SAM_ACCOUNT *user)
+static void pdb_fill_default_sam(SAM_ACCOUNT *user)
 {
-	if (user == NULL) {
-		DEBUG(0,("pdb_fill_default_sam: SAM_ACCOUNT was NULL\n"));
-		return False;
-	}
-	
-	ZERO_STRUCTP(user);
+	ZERO_STRUCT(user->private); /* Don't touch the talloc context */
 
         /* Don't change these timestamp settings without a good reason.
            They are important for NT member server compatibility. */
@@ -85,34 +80,77 @@ static BOOL pdb_fill_default_sam(SAM_ACCOUNT *user)
 	memset(user->private.hours, 0xff, user->private.hours_len); /* available at all hours */
 	user->private.unknown_5 = 0x00000000; /* don't know */
 	user->private.unknown_6 = 0x000004ec; /* don't know */
-	return True;
 }	
 
+static void destroy_pdb_talloc(SAM_ACCOUNT **user) 
+{
+	if (*user) {
+		talloc_destroy((*user)->mem_ctx);
+		*user = NULL;
+	}
+}
+
 
-/*************************************************************
- Alloc memory and initialises a struct sam_passwd.
- ************************************************************/
+/**********************************************************************
+ Alloc memory and initialises a struct sam_passwd on supplied mem_ctx.
+***********************************************************************/
 
-BOOL pdb_init_sam(SAM_ACCOUNT **user)
+NTSTATUS pdb_init_sam_talloc(TALLOC_CTX *mem_ctx, SAM_ACCOUNT **user)
 {
 	if (*user != NULL) {
 		DEBUG(0,("pdb_init_sam: SAM_ACCOUNT was non NULL\n"));
 #if 0
 		smb_panic("NULL pointer passed to pdb_init_sam\n");
 #endif
-		return False;
+		return NT_STATUS_UNSUCCESSFUL;
 	}
-	
-	*user=(SAM_ACCOUNT *)malloc(sizeof(SAM_ACCOUNT));
+
+	if (!mem_ctx) {
+		DEBUG(0,("pdb_init_sam_talloc: mem_ctx was NULL!\n"));
+		return NT_STATUS_UNSUCCESSFUL;
+	}
+
+	*user=(SAM_ACCOUNT *)talloc(mem_ctx, sizeof(SAM_ACCOUNT));
 
 	if (*user==NULL) {
 		DEBUG(0,("pdb_init_sam: error while allocating memory\n"));
-		return False;
+		return NT_STATUS_NO_MEMORY;
 	}
 
+	(*user)->mem_ctx = mem_ctx;
+
+	(*user)->free_fn = NULL;
+
 	pdb_fill_default_sam(*user);
+	
+	return NT_STATUS_OK;
+}
 
-	return True;
+
+/*************************************************************
+ Alloc memory and initialises a struct sam_passwd.
+ ************************************************************/
+
+NTSTATUS pdb_init_sam(SAM_ACCOUNT **user)
+{
+	TALLOC_CTX *mem_ctx;
+	NTSTATUS nt_status;
+	
+	mem_ctx = talloc_init_named("passdb internal SAM_ACCOUNT allocation");
+
+	if (!mem_ctx) {
+		DEBUG(0,("pdb_init_sam: error while doing talloc_init()\n"));
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	if (!NT_STATUS_IS_OK(nt_status = pdb_init_sam_talloc(mem_ctx, user))) {
+		talloc_destroy(mem_ctx);
+		return nt_status;
+	}
+	
+	(*user)->free_fn = destroy_pdb_talloc;
+
+	return NT_STATUS_OK;
 }
 
 
@@ -120,20 +158,21 @@ BOOL pdb_init_sam(SAM_ACCOUNT **user)
  Initialises a struct sam_passwd with sane values.
  ************************************************************/
 
-BOOL pdb_init_sam_pw(SAM_ACCOUNT **new_sam_acct, const struct passwd *pwd)
+NTSTATUS pdb_init_sam_pw(SAM_ACCOUNT **new_sam_acct, const struct passwd *pwd)
 {
 	pstring str;
 	GROUP_MAP map;
 	uint32 rid;
+	NTSTATUS nt_status;
 
 	if (!pwd) {
 		new_sam_acct = NULL;
-		return False;
+		return NT_STATUS_UNSUCCESSFUL;
 	}
 
-	if (!pdb_init_sam(new_sam_acct)) {
+	if (!NT_STATUS_IS_OK(nt_status = pdb_init_sam(new_sam_acct))) {
 		new_sam_acct = NULL;
-		return False;
+		return nt_status;
 	}
 
 	pdb_set_username(*new_sam_acct, pwd->pw_name);
@@ -170,7 +209,7 @@ BOOL pdb_init_sam_pw(SAM_ACCOUNT **new_sam_acct, const struct passwd *pwd)
 	standard_sub_advanced(-1, pwd->pw_name, "", pwd->pw_gid, pwd->pw_name, str);
 	pdb_set_logon_script(*new_sam_acct, str, False);
 	
-	return True;
+	return NT_STATUS_OK;
 }
 
 
@@ -182,23 +221,13 @@ BOOL pdb_init_sam_pw(SAM_ACCOUNT **new_sam_acct, const struct passwd *pwd)
  * @param user SAM_ACCOUNT to free members of.
  **/
 
-static BOOL pdb_free_sam_contents(SAM_ACCOUNT *user)
+static void pdb_free_sam_contents(SAM_ACCOUNT *user)
 {
-	if (user == NULL) {
-		DEBUG(0,("pdb_free_sam_contents: SAM_ACCOUNT was NULL\n"));
-#if 0
-		smb_panic("NULL pointer passed to pdb_free_sam_contents\n");
-#endif
-		return False;
-	}
-
 	/* As we start mallocing more strings this is where  
 	   we should free them. */
 
 	data_blob_clear_free(&(user->private.lm_pw));
 	data_blob_clear_free(&(user->private.nt_pw));
-
-	return True;	
 }
 
 
@@ -206,25 +235,21 @@ static BOOL pdb_free_sam_contents(SAM_ACCOUNT *user)
  Reset the SAM_ACCOUNT and free the NT/LM hashes.
  ***********************************************************/
 
-BOOL pdb_reset_sam(SAM_ACCOUNT *user)
+NTSTATUS pdb_reset_sam(SAM_ACCOUNT *user)
 {
 	if (user == NULL) {
 		DEBUG(0,("pdb_reset_sam: SAM_ACCOUNT was NULL\n"));
 #if 0
 		smb_panic("NULL pointer passed to pdb_free_sam\n");
 #endif
-		return False;
+		return NT_STATUS_UNSUCCESSFUL;
 	}
 	
-	if (!pdb_free_sam_contents(user)) {
-		return False;
-	}
+	pdb_free_sam_contents(user);
 
-	if (!pdb_fill_default_sam(user)) {
-		return False;
-	}
+	pdb_fill_default_sam(user);
 
-	return True;
+	return NT_STATUS_OK;
 }
 
 
@@ -232,23 +257,23 @@ BOOL pdb_reset_sam(SAM_ACCOUNT *user)
  Free the SAM_ACCOUNT and the member pointers.
  ***********************************************************/
 
-BOOL pdb_free_sam(SAM_ACCOUNT **user)
+NTSTATUS pdb_free_sam(SAM_ACCOUNT **user)
 {
 	if (*user == NULL) {
 		DEBUG(0,("pdb_free_sam: SAM_ACCOUNT was NULL\n"));
 #if 0
 		smb_panic("NULL pointer passed to pdb_free_sam\n");
 #endif
-		return False;
+		return NT_STATUS_UNSUCCESSFUL;
 	}
 
-	if (!pdb_free_sam_contents(*user)) {
-		return False;
+	pdb_free_sam_contents(*user);
+	
+	if ((*user)->free_fn) {
+		(*user)->free_fn(user);
 	}
 
-	SAFE_FREE(*user);
-	
-	return True;	
+	return NT_STATUS_OK;	
 }
 
 
@@ -974,7 +999,7 @@ account without a valid local system user.\n", user_name);
 			return False;
 		}
 
-		if (!pdb_init_sam_pw(&sam_pass, pwd)) {
+		if (!NT_STATUS_IS_OK(pdb_init_sam_pw(&sam_pass, pwd))){
 			slprintf(err_str, err_str_len-1, "Failed initialise SAM_ACCOUNT for user %s.\n", user_name);
 			return False;
 		}
diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c
index 9abc3b5a28..0dd0f21c37 100644
--- a/source3/passdb/pdb_get_set.c
+++ b/source3/passdb/pdb_get_set.c
@@ -24,6 +24,14 @@
 
 #include "includes.h"
 
+/**
+ * @todo Redefine this to NULL, but this changes the API becouse
+ *       much of samba assumes that the pdb_get...() funtions 
+ *       return pstrings.  (ie not null-pointers).
+ */
+
+#define PDB_NOT_QUITE_NULL ""
+
 /*********************************************************************
  Collection of get...() functions for SAM_ACCOUNT_INFO.
  ********************************************************************/
@@ -464,14 +472,15 @@ BOOL pdb_set_username(SAM_ACCOUNT *sampass, const char *username)
 {	
 	if (!sampass)
 		return False;
-	
-	*sampass->private.username = '\0';
+
 	DEBUG(10, ("pdb_set_username: setting username %s, was %s\n", 
 		   username, sampass->private.username));
  
-	if (!username)
-		return False;
-	StrnCpy (sampass->private.username, username, sizeof(pstring) - 1);
+	if (username) { 
+		sampass->private.username = talloc_strdup(sampass->mem_ctx, username);
+	} else {
+		sampass->private.username = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
@@ -484,11 +493,15 @@ BOOL pdb_set_domain(SAM_ACCOUNT *sampass, const char *domain)
 {	
 	if (!sampass)
 		return False;
-	*sampass->private.domain = '\0';
-	if (!domain)
-		return False;
 
-	StrnCpy (sampass->private.domain, domain, sizeof(pstring) - 1);
+	DEBUG(10, ("pdb_set_domain: setting domain %s, was %s\n", 
+		   domain, sampass->private.domain));
+ 
+	if (domain) { 
+		sampass->private.domain = talloc_strdup(sampass->mem_ctx, domain);
+	} else {
+		sampass->private.domain = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
@@ -501,11 +514,15 @@ BOOL pdb_set_nt_username(SAM_ACCOUNT *sampass, const char *nt_username)
 {
 	if (!sampass)
 		return False;
-	*sampass->private.nt_username = '\0';
-	if (!nt_username)
-		return False;
 
-	StrnCpy (sampass->private.nt_username, nt_username, sizeof(pstring) - 1);
+	DEBUG(10, ("pdb_set_nt_username: setting nt username %s, was %s\n", 
+		   nt_username, sampass->private.nt_username));
+ 
+	if (nt_username) { 
+		sampass->private.nt_username = talloc_strdup(sampass->mem_ctx, nt_username);
+	} else {
+		sampass->private.nt_username = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
@@ -514,19 +531,19 @@ BOOL pdb_set_nt_username(SAM_ACCOUNT *sampass, const char *nt_username)
  Set the user's full name.
  ********************************************************************/
 
-BOOL pdb_set_fullname(SAM_ACCOUNT *sampass, const char *fullname)
+BOOL pdb_set_fullname(SAM_ACCOUNT *sampass, const char *full_name)
 {
 	if (!sampass)
 		return False;
 
-	DEBUG(10, ("pdb_set_fullname: setting full name %s, was %s\n", 
-		   fullname, sampass->private.full_name));
- 
-	*sampass->private.full_name = '\0';
-	if (!fullname)
-		return False;
-
-	StrnCpy (sampass->private.full_name, fullname, sizeof(pstring) - 1);
+	DEBUG(10, ("pdb_set_full_name: setting full name %s, was %s\n", 
+		   full_name, sampass->private.full_name));
+	
+	if (full_name) { 
+		sampass->private.full_name = talloc_strdup(sampass->mem_ctx, full_name);
+	} else {
+		sampass->private.full_name = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
@@ -543,12 +560,12 @@ BOOL pdb_set_logon_script(SAM_ACCOUNT *sampass, const char *logon_script, BOOL s
 	DEBUG(10, ("pdb_set_logon_script: setting logon script (store:%d) %s, was %s\n", 
 		   store, logon_script, sampass->private.logon_script));
  
-	*sampass->private.logon_script = '\0';
-	if (!logon_script)
-		return False;
-
-	StrnCpy (sampass->private.logon_script, logon_script, sizeof(pstring) - 1);
-
+	if (logon_script) { 
+		sampass->private.logon_script = talloc_strdup(sampass->mem_ctx, logon_script);
+	} else {
+		sampass->private.logon_script = PDB_NOT_QUITE_NULL;
+	}
+	
 	if (store)
 		pdb_set_init_flag(sampass, FLAG_SAM_LOGONSCRIPT); 
 
@@ -567,15 +584,15 @@ BOOL pdb_set_profile_path (SAM_ACCOUNT *sampass, const char *profile_path, BOOL
 	DEBUG(10, ("pdb_set_profile_path: setting profile path (store:%d) %s, was %s\n", 
 		   store, profile_path, sampass->private.profile_path));
  
-	*sampass->private.profile_path = '\0';
-	if (!profile_path)
-		return False;
-	
-	StrnCpy (sampass->private.profile_path, profile_path, sizeof(pstring) - 1);
+	if (profile_path) { 
+		sampass->private.profile_path = talloc_strdup(sampass->mem_ctx, profile_path);
+	} else {
+		sampass->private.profile_path = PDB_NOT_QUITE_NULL;
+	}
 
 	if (store)
 		pdb_set_init_flag(sampass, FLAG_SAM_PROFILE);
-	
+
 	return True;
 }
 
@@ -587,12 +604,13 @@ BOOL pdb_set_dir_drive (SAM_ACCOUNT *sampass, const char *dir_drive, BOOL store)
 {
 	if (!sampass)
 		return False;
-	*sampass->private.dir_drive = '\0';
-	if (!dir_drive)
-		return False;
-
-	StrnCpy (sampass->private.dir_drive, dir_drive, sizeof(pstring) - 1);
 
+	if (dir_drive) { 
+		sampass->private.dir_drive = talloc_strdup(sampass->mem_ctx, dir_drive);
+	} else {
+		sampass->private.dir_drive = PDB_NOT_QUITE_NULL;
+	}
+	
 	if (store)
 		pdb_set_init_flag(sampass, FLAG_SAM_DRIVE);
 
@@ -603,15 +621,16 @@ BOOL pdb_set_dir_drive (SAM_ACCOUNT *sampass, const char *dir_drive, BOOL store)
  Set the user's home directory.
  ********************************************************************/
 
-BOOL pdb_set_homedir (SAM_ACCOUNT *sampass, const char *homedir, BOOL store)
+BOOL pdb_set_homedir (SAM_ACCOUNT *sampass, const char *home_dir, BOOL store)
 {
 	if (!sampass)
 		return False;
-	*sampass->private.home_dir = '\0';
-	if (!homedir)
-		return False;
-	
-	StrnCpy (sampass->private.home_dir, homedir, sizeof(pstring) - 1);
+
+	if (home_dir) { 
+		sampass->private.home_dir = talloc_strdup(sampass->mem_ctx, home_dir);
+	} else {
+		sampass->private.home_dir = PDB_NOT_QUITE_NULL;
+	}
 
 	if (store)
 		pdb_set_init_flag(sampass, FLAG_SAM_SMBHOME);
@@ -627,11 +646,12 @@ BOOL pdb_set_acct_desc (SAM_ACCOUNT *sampass, const char *acct_desc)
 {
 	if (!sampass)
 		return False;
-	*sampass->private.acct_desc = '\0';
-	if (!acct_desc)
-		return False;
-	
-	StrnCpy (sampass->private.acct_desc, acct_desc, sizeof(pstring) - 1);
+
+	if (acct_desc) { 
+		sampass->private.acct_desc = talloc_strdup(sampass->mem_ctx, acct_desc);
+	} else {
+		sampass->private.acct_desc = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
@@ -644,11 +664,12 @@ BOOL pdb_set_workstations (SAM_ACCOUNT *sampass, const char *workstations)
 {
 	if (!sampass)
 		return False;
-	*sampass->private.workstations = '\0';
-	if (!workstations)
-		return False;
 
-	StrnCpy (sampass->private.workstations, workstations, sizeof(pstring) - 1);
+	if (workstations) { 
+		sampass->private.workstations = talloc_strdup(sampass->mem_ctx, workstations);
+	} else {
+		sampass->private.workstations = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
@@ -661,11 +682,12 @@ BOOL pdb_set_unknown_str (SAM_ACCOUNT *sampass, const char *unknown_str)
 {
 	if (!sampass)
 		return False;
-	*sampass->private.unknown_str = '\0';
-	if (!unknown_str)
-		return False;
 
-	StrnCpy (sampass->private.unknown_str, unknown_str, sizeof(pstring) - 1);
+	if (unknown_str) { 
+		sampass->private.unknown_str = talloc_strdup(sampass->mem_ctx, unknown_str);
+	} else {
+		sampass->private.unknown_str = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
@@ -678,11 +700,11 @@ BOOL pdb_set_munged_dial (SAM_ACCOUNT *sampass, const char *munged_dial)
 {
 	if (!sampass)
 		return False;
-	*sampass->private.munged_dial = '\0';
-	if (!munged_dial)
-		return False;
-
-	StrnCpy (sampass->private.munged_dial, munged_dial, sizeof(pstring) - 1);
+	if (munged_dial) { 
+		sampass->private.munged_dial = talloc_strdup(sampass->mem_ctx, munged_dial);
+	} else {
+		sampass->private.munged_dial = PDB_NOT_QUITE_NULL;
+	}
 
 	return True;
 }
diff --git a/source3/passdb/pdb_tdb.c b/source3/passdb/pdb_tdb.c
index c1b06067b4..90976b3fef 100644
--- a/source3/passdb/pdb_tdb.c
+++ b/source3/passdb/pdb_tdb.c
@@ -681,7 +681,7 @@ BOOL pdb_delete_sam_account(const char *sname)
 	}
   
   	/* unpack the buffer */
-	if (!pdb_init_sam (&sam_pass)) {
+	if (!NT_STATUS_IS_OK(pdb_init_sam (&sam_pass))) {
 		tdb_close (pwd_tdb);
 		return False;
 	}
diff --git a/source3/rpc_server/srv_netlog_nt.c b/source3/rpc_server/srv_netlog_nt.c
index a07ebc9a3f..1ae1b65fdd 100644
--- a/source3/rpc_server/srv_netlog_nt.c
+++ b/source3/rpc_server/srv_netlog_nt.c
@@ -189,7 +189,7 @@ static BOOL get_md4pw(char *md4pw, char *mach_acct)
 	}
 #endif /* 0 */
 
-	if(!pdb_init_sam(&sampass))
+	if(!NT_STATUS_IS_OK(pdb_init_sam(&sampass)))
 		return False;
 
 	/* JRA. This is ok as it is only used for generating the challenge. */
diff --git a/source3/utils/pdbedit.c b/source3/utils/pdbedit.c
index 046d4a4ad8..ae600f6a6a 100644
--- a/source3/utils/pdbedit.c
+++ b/source3/utils/pdbedit.c
@@ -443,7 +443,7 @@ account without a valid local system user.\n", user_name);
 			return False;
 		}
 
-		if (!pdb_init_sam_pw(&sam_pwent, pwd)) {
+		if (!NT_STATUS_IS_OK(pdb_init_sam_pw(&sam_pwent, pwd))) {
 			fprintf(stderr, "Failed initialise SAM_ACCOUNT for user %s.\n", user_name);
 			return False;
 		}