Memory leaks in smbd ?
Martin Zielinski
mz at seh.de
Fri Nov 14 08:13:18 GMT 2008
Sorry.
Volker Lendecke schrieb:
> On Thu, Nov 13, 2008 at 04:54:35PM +0100, Martin Zielinski wrote:
>> Thanks for the hints, Jeremy!
>>
>> I've looked quickly through the code of 3.2.4 and I think there are some
>> places that should be fixed.
>>
>> Some are obvious, some may be wrong as the memory is linked to another
>> struct and will be destroyed later.
>> However it is not allways visible in the code. Sometimes it is freed on
>> an error case and later in the same function it is not.
>>
>> I found that the output of the smbcontrol program attached to my first
>> post is caused by a samu - allocation in pdb_getsampwnam ().
>>
>> I think, that there is a bug in the memcache_add function.
>> I allready asked Volker Lendecke in the "memory usage" thread on the
>> samba mailing list about that!
>>
>> Attached is a diff against 3.2.4 which (hopefully) points to one or the
>> other potential leak.
>
> No diff around.
>
> Volker
--
Martin Zielinski mz at seh.de
Softwareentwicklung T +49 (0)521 94226 76
SEH Computertechnik GmbH www.seh.de
-------------- next part --------------
diff -urN source/auth/auth_sam.c source.samu/auth/auth_sam.c
--- source/auth/auth_sam.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/auth/auth_sam.c 2008-11-13 09:44:07.000000000 +0100
@@ -290,6 +290,7 @@
/* Quit if the account was locked out. */
if (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) {
DEBUG(3,("check_sam_security: Account for user %s was locked out.\n", pdb_get_username(sampass)));
+ TALLOC_FREE(sampass);
return NT_STATUS_ACCOUNT_LOCKED_OUT;
}
@@ -353,6 +354,7 @@
if (!NT_STATUS_IS_OK(nt_status)) {
DEBUG(0,("check_sam_security: make_server_info_sam() failed with '%s'\n", nt_errstr(nt_status)));
+ TALLOC_FREE(sampass);
data_blob_free(&user_sess_key);
data_blob_free(&lm_sess_key);
return nt_status;
diff -urN source/auth/auth_util.c source.samu/auth/auth_util.c
--- source/auth/auth_util.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/auth/auth_util.c 2008-11-13 09:36:06.000000000 +0100
@@ -1089,6 +1089,7 @@
status = samu_set_unix( sampass, pwd );
if (!NT_STATUS_IS_OK(status)) {
+ TALLOC_FREE(sampass);
return status;
}
@@ -1217,8 +1218,10 @@
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10, ("create_local_token failed: %s\n",
nt_errstr(status)));
+ TALLOC_FREE(sampass);
return status;
}
+ TALLOC_FREE(sampass);
/* annoying, but the Guest really does have a session key, and it is
all zeros! */
diff -urN source/lib/memcache.c source.samu/lib/memcache.c
--- source/lib/memcache.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/lib/memcache.c 2008-11-13 16:39:42.000000000 +0100
@@ -278,7 +278,16 @@
if (value.length <= cache_value.length) {
/*
* We can reuse the existing record
+ * But free talloced data before reusing the record
*/
+ if (memcache_is_talloc((enum memcache_number)e->n)
+ && (e->valuelength == sizeof(void *))) {
+ DATA_BLOB temp_key, temp_value;
+ void *ptr;
+ memcache_element_parse(e, &key, &value);
+ memcpy(&ptr, value.data, sizeof(ptr));
+ TALLOC_FREE(ptr);
+ }
memcpy(cache_value.data, value.data, value.length);
e->valuelength = value.length;
return;
diff -urN source/passdb/pdb_interface.c source.samu/passdb/pdb_interface.c
--- source/passdb/pdb_interface.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/passdb/pdb_interface.c 2008-11-13 16:36:57.000000000 +0100
@@ -795,10 +795,12 @@
if (!pdb_getsampwsid(account, &member_sid) ||
!sid_to_uid(&member_sid, &uid) ||
((pwd = getpwuid_alloc(mem_ctx, uid)) == NULL)) {
+ TALLOC_FREE(account);
return NT_STATUS_NO_SUCH_USER;
}
if (pdb_user_in_group(mem_ctx, account, &group_sid)) {
+ TALLOC_FREE(account);
return NT_STATUS_MEMBER_IN_GROUP;
}
@@ -810,8 +812,10 @@
smb_add_user_group(group_name, pwd->pw_name);
if (!pdb_user_in_group(mem_ctx, account, &group_sid)) {
+ TALLOC_FREE(account);
return NT_STATUS_ACCESS_DENIED;
}
+ TALLOC_FREE(account);
return NT_STATUS_OK;
}
@@ -857,10 +861,12 @@
if (!pdb_getsampwsid(account, &member_sid) ||
!sid_to_uid(&member_sid, &uid) ||
((pwd = getpwuid_alloc(mem_ctx, uid)) == NULL)) {
+ TALLOC_FREE(account);
return NT_STATUS_NO_SUCH_USER;
}
if (!pdb_user_in_group(mem_ctx, account, &group_sid)) {
+ TALLOC_FREE(account);
return NT_STATUS_MEMBER_NOT_IN_GROUP;
}
@@ -872,9 +878,11 @@
smb_delete_user_group(group_name, pwd->pw_name);
if (pdb_user_in_group(mem_ctx, account, &group_sid)) {
+ TALLOC_FREE(account);
return NT_STATUS_ACCESS_DENIED;
}
+ TALLOC_FREE(account);
return NT_STATUS_OK;
}
diff -urN source/passdb/pdb_smbpasswd.c source.samu/passdb/pdb_smbpasswd.c
--- source/passdb/pdb_smbpasswd.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/passdb/pdb_smbpasswd.c 2008-11-13 09:23:23.000000000 +0100
@@ -1627,6 +1627,7 @@
if (!build_sam_account(smbpasswd_state, user, pwd)) {
/* Already got debug msgs... */
+ TALLOC_FREE( user );
break;
}
diff -urN source/passdb/pdb_tdb.c source.samu/passdb/pdb_tdb.c
--- source/passdb/pdb_tdb.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/passdb/pdb_tdb.c 2008-11-13 09:28:52.000000000 +0100
@@ -1506,6 +1506,7 @@
}
if (state->current == state->num_rids) {
+ TALLOC_FREE(user);
return false;
}
diff -urN source/rpc_server/srv_samr_nt.c source.samu/rpc_server/srv_samr_nt.c
--- source/rpc_server/srv_samr_nt.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/rpc_server/srv_samr_nt.c 2008-11-13 16:36:22.000000000 +0100
@@ -2153,8 +2153,14 @@
/* append the user's RID to it */
- if (!sid_append_rid(&sid, r->in.rid))
+ if (!sid_append_rid(&sid, r->in.rid)) {
+ /* Well, ... wil sampass be cleared automaticly with the
+ pipes_struct?
+ If yes, why is sampass freed some lines later?
+ */
+ TALLOC_FREE(sampass);
return NT_STATUS_NO_SUCH_USER;
+ }
/* check if access can be granted as requested by client. */
@@ -2168,8 +2174,10 @@
&se_rights, GENERIC_RIGHTS_USER_WRITE, des_access,
&acc_granted, "_samr_OpenUser");
- if ( !NT_STATUS_IS_OK(nt_status) )
+ if ( !NT_STATUS_IS_OK(nt_status) ) {
+ TALLOC_FREE(sampass);
return nt_status;
+ }
become_root();
ret=pdb_getsampwsid(sampass, &sid);
@@ -2177,9 +2185,10 @@
/* check that the SID exists in our domain. */
if (ret == False) {
+ TALLOC_FREE(sampass);
return NT_STATUS_NO_SUCH_USER;
}
-
+ /* here is the TALLOC_FREE that is not called in the error case: */
TALLOC_FREE(sampass);
/* associate the user's SID and access bits with the new handle. */
@@ -2253,6 +2262,7 @@
if ( !ret ) {
DEBUG(4,("User %s not found\n", sid_string_dbg(user_sid)));
+ TALLOC_FREE(smbpass);
return NT_STATUS_NO_SUCH_USER;
}
diff -urN source/winbindd/winbindd_passdb.c source.samu/winbindd/winbindd_passdb.c
--- source/winbindd/winbindd_passdb.c 2008-09-18 08:49:02.000000000 +0200
+++ source.samu/winbindd/winbindd_passdb.c 2008-11-13 09:21:08.000000000 +0100
@@ -267,6 +267,7 @@
}
if ( !pdb_getsampwsid( user, user_sid ) ) {
+ TALLOC_FREE( user );
return NT_STATUS_NO_SUCH_USER;
}
More information about the samba-technical
mailing list