This patch derives from debian bug 407517, where Jamie is seeing smbd memory use skyrocket, when used from IIS.
I'm concerned about two things: memory leaks of server_info in closed sessions (and leaks of other attached things), and the specific behaviour of the getpwnam() cache. I've adjusted the cache to use talloc_unlink(), which I think gives more exact semantics for what is wanted here. An in the auth code, I've tried to start using the new talloc(), to ensure we can't loose memory. This passes the 'make test' of Samba3. I don't know if it solves Jamie's problem however. (I just took a pot shot at some likely suspects). Thoughts? Andrew Bartlett -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org Samba Developer, Red Hat Inc. http://redhat.com
Index: lib/util_pw.c
===================================================================
--- lib/util_pw.c (revision 20942)
+++ lib/util_pw.c (working copy)
@@ -98,15 +98,11 @@
i = rand() % PWNAMCACHE_SIZE;
if (pwnam_cache[i] != NULL) {
- TALLOC_FREE(pwnam_cache[i]);
+ 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]);
+ return (struct passwd *)talloc_reference(mem_ctx, pwnam_cache[i]);
}
struct passwd *getpwuid_alloc(TALLOC_CTX *mem_ctx, uid_t uid)
Index: smbd/password.c
===================================================================
--- smbd/password.c (revision 20942)
+++ smbd/password.c (working copy)
@@ -90,15 +90,8 @@
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);
DLIST_REMOVE(validated_users, vuser);
@@ -107,9 +100,7 @@
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--;
}
@@ -150,7 +141,7 @@
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) {
@@ -163,14 +154,12 @@
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++;
@@ -200,6 +189,10 @@
return vuser->vuid;
}
+ /* use this to keep tabs on all our info from the authentication */
+ vuser->server_info = server_info;
+ 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
*/
@@ -209,14 +202,13 @@
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;
}
}
@@ -244,24 +236,23 @@
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;
+ 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;
}
}
@@ -277,23 +268,15 @@
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));
Index: smbd/session.c
===================================================================
--- smbd/session.c (revision 20942)
+++ smbd/session.c (working copy)
@@ -160,9 +160,9 @@
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;
Index: auth/auth_util.c
===================================================================
--- auth/auth_util.c (revision 20942)
+++ auth/auth_util.c (working copy)
@@ -561,18 +561,19 @@
DOM_SID unix_group_sid;
- if ( !(pwd = getpwnam_alloc(NULL, pdb_get_username(sampass))) ) {
+ 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;
}
- if ( !(result = make_server_info(NULL)) ) {
- TALLOC_FREE(pwd);
- return NT_STATUS_NO_MEMORY;
- }
-
result->sam_account = sampass;
+ talloc_steal(result, sampass);
result->unix_name = talloc_strdup(result, pwd->pw_name);
result->gid = pwd->pw_gid;
result->uid = pwd->pw_uid;
signature.asc
Description: This is a digitally signed message part
