[PATCH] add missing talloc_free

Julia Lawall julia.lawall at lip6.fr
Sat Feb 25 13:32:59 MST 2012


There seem to be a number of places in the samba code where calls to
talloc_free are omitted.  This patch fixes some cases.

These problems were found using Coccinelle (http://coccinelle.lip6.fr/).  A
simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression r.e;
expression e1,e2;
position p1,p2;
statement S;
@@

(
if (e == NULL || ...) S
|
if (<+...e = e2...+>) S
|
if (...) { ... when != talloc_free(e);
(
   return <+...e...+>;
|
* return ...;
)
  }
)
... when != e = e1
     when != &e
     when any
talloc_free(e);
// </smpl>

Signed-off-by: Julia Lawall <julia.lawall at lip6.fr>

---

I am not very familiar with the samba code, so these could be completely
off.  Feedback is most welcome.

  dfs_server/dfs_server_ad.c           |    2 ++
  libcli/cldap/cldap.c                 |    3 ++-
  libcli/lsarpc/util_lsarpc.c          |    3 ++-
  source4/dns_server/dlz_bind9.c       |    2 ++
  source4/dns_server/dns_server.c      |    1 +
  source4/dsdb/common/util_samr.c      |    7 +++++++
  source4/libcli/dgram/netlogon.c      |    1 +
  source4/libcli/util/clilsa.c         |    4 ++++
  source4/libnet/libnet_samsync_ldb.c  |    2 ++
  source4/libnet/libnet_site.c         |    1 +
  source4/ntvfs/posix/pvfs_dirlist.c   |    4 ++++
  source4/ntvfs/posix/pvfs_resolve.c   |    7 +++++--
  source4/ntvfs/posix/pvfs_streams.c   |    2 ++
  source4/ntvfs/posix/pvfs_xattr.c     |    1 +
  source4/utils/oLschema2ldif.c        |    3 ++-
  source4/winbind/wb_samba3_protocol.c |    1 +
  16 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/dfs_server/dfs_server_ad.c b/dfs_server/dfs_server_ad.c
index 22d3263..0c46e54 100644
--- a/dfs_server/dfs_server_ad.c
+++ b/dfs_server/dfs_server_ad.c
@@ -201,6 +201,7 @@ static NTSTATUS get_dcs_insite(TALLOC_CTX *ctx, struct ldb_context *ldb,

  		dn = ldb_msg_find_attr_as_dn(ldb, ctx, r->msgs[i], "serverReference");
  		if (!dn) {
+			talloc_free(r);
  			return NT_STATUS_INTERNAL_ERROR;
  		}

@@ -208,6 +209,7 @@ static NTSTATUS get_dcs_insite(TALLOC_CTX *ctx, struct ldb_context *ldb,
  		if (ret != LDB_SUCCESS) {
  			DEBUG(2,(__location__ ": Search for computer on %s failed - %s\n",
  				 ldb_dn_get_linearized(dn), ldb_errstring(ldb)));
+			talloc_free(r);
  			return NT_STATUS_INTERNAL_ERROR;
  		}

diff --git a/libcli/cldap/cldap.c b/libcli/cldap/cldap.c
index 3322bd8..aa383c5 100644
--- a/libcli/cldap/cldap.c
+++ b/libcli/cldap/cldap.c
@@ -357,7 +357,8 @@ NTSTATUS cldap_socket_init(TALLOC_CTX *mem_ctx,
  		 * Here we know the address family of the remote address.
  		 */
  		if (fam == NULL) {
-			return NT_STATUS_INVALID_PARAMETER_MIX;
+			status = NT_STATUS_INVALID_PARAMETER_MIX;
+			goto nterror;
  		}

  		ret = tsocket_address_inet_from_strings(c, fam,
diff --git a/libcli/lsarpc/util_lsarpc.c b/libcli/lsarpc/util_lsarpc.c
index 96c9848..4f40b95 100644
--- a/libcli/lsarpc/util_lsarpc.c
+++ b/libcli/lsarpc/util_lsarpc.c
@@ -311,7 +311,8 @@ static NTSTATUS auth_info_2_trustauth_inout_blob(TALLOC_CTX *mem_ctx,
  			      iopw,
  			      (ndr_push_flags_fn_t)ndr_push_trustAuthInOutBlob);
  	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-		return NT_STATUS_INVALID_PARAMETER;
+		status = NT_STATUS_INVALID_PARAMETER;
+		goto done;
  	}

  	status = NT_STATUS_OK;
diff --git a/source4/dns_server/dlz_bind9.c b/source4/dns_server/dlz_bind9.c
index 34320e0..f97acdb 100644
--- a/source4/dns_server/dlz_bind9.c
+++ b/source4/dns_server/dlz_bind9.c
@@ -377,6 +377,7 @@ static isc_result_t b9_putrr(struct dlz_bind9_data *state,
  	TALLOC_CTX *tmp_ctx = talloc_new(state);

  	if (!b9_format(state, tmp_ctx, rec, &type, &data)) {
+		talloc_free(tmp_ctx);
  		return ISC_R_FAILURE;
  	}

@@ -417,6 +418,7 @@ static isc_result_t b9_putnamedrr(struct dlz_bind9_data *state,
  	TALLOC_CTX *tmp_ctx = talloc_new(state);

  	if (!b9_format(state, tmp_ctx, rec, &type, &data)) {
+		talloc_free(tmp_ctx);
  		return ISC_R_FAILURE;
  	}

diff --git a/source4/dns_server/dns_server.c b/source4/dns_server/dns_server.c
index 25873c2..d1f0639 100644
--- a/source4/dns_server/dns_server.c
+++ b/source4/dns_server/dns_server.c
@@ -494,6 +494,7 @@ static NTSTATUS dns_add_socket(struct dns_server *dns,
  						&dns_socket->local_address);
  	if (ret != 0) {
  		status = map_nt_error_from_unix_common(errno);
+		talloc_free(dns_socket);
  		return status;
  	}

diff --git a/source4/dsdb/common/util_samr.c b/source4/dsdb/common/util_samr.c
index 184dfd5..e8365b1 100644
--- a/source4/dsdb/common/util_samr.c
+++ b/source4/dsdb/common/util_samr.c
@@ -113,6 +113,7 @@ NTSTATUS dsdb_add_user(struct ldb_context *ldb,
  	} else if (acct_flags == ACB_WSTRUST) {
  		if (cn_name[cn_name_len - 1] != '$') {
  			ldb_transaction_cancel(ldb);
+			talloc_free(tmp_ctx);
  			return NT_STATUS_FOOBAR;
  		}
  		cn_name[cn_name_len - 1] = '\0';
@@ -122,6 +123,7 @@ NTSTATUS dsdb_add_user(struct ldb_context *ldb,
  	} else if (acct_flags == ACB_SVRTRUST) {
  		if (cn_name[cn_name_len - 1] != '$') {
  			ldb_transaction_cancel(ldb);
+			talloc_free(tmp_ctx);
  			return NT_STATUS_FOOBAR;
  		}
  		cn_name[cn_name_len - 1] = '\0';
@@ -296,11 +298,13 @@ NTSTATUS dsdb_add_domain_group(struct ldb_context *ldb,
  				   "(&(sAMAccountName=%s)(objectclass=group))",
  				   ldb_binary_encode_string(tmp_ctx, groupname));
  	if (name != NULL) {
+		talloc_free(tmp_ctx);
  		return NT_STATUS_GROUP_EXISTS;
  	}

  	msg = ldb_msg_new(tmp_ctx);
  	if (msg == NULL) {
+		talloc_free(tmp_ctx);
  		return NT_STATUS_NO_MEMORY;
  	}

@@ -368,6 +372,7 @@ NTSTATUS dsdb_add_domain_alias(struct ldb_context *ldb,

  	if (ldb_transaction_start(ldb) != LDB_SUCCESS) {
  		DEBUG(0, ("Failed to start transaction in dsdb_add_domain_alias(): %s\n", ldb_errstring(ldb)));
+		talloc_free(tmp_ctx);
  		return NT_STATUS_INTERNAL_ERROR;
  	}

@@ -469,6 +474,7 @@ NTSTATUS dsdb_enum_group_mem(struct ldb_context *ldb,
  	if (ret != LDB_SUCCESS) {
  		DEBUG(1, ("dsdb_enum_group_mem: dsdb_search for %s failed: %s\n",
  			  ldb_dn_get_linearized(dn), ldb_errstring(ldb)));
+		talloc_free(tmp_ctx);
  		return NT_STATUS_INTERNAL_DB_CORRUPTION;
  	}

@@ -482,6 +488,7 @@ NTSTATUS dsdb_enum_group_mem(struct ldb_context *ldb,

  	members = talloc_array(mem_ctx, struct dom_sid, member_el->num_values);
  	if (members == NULL) {
+		talloc_free(tmp_ctx);
  		return NT_STATUS_NO_MEMORY;
  	}

diff --git a/source4/libcli/dgram/netlogon.c b/source4/libcli/dgram/netlogon.c
index 0aa6864..0087707 100644
--- a/source4/libcli/dgram/netlogon.c
+++ b/source4/libcli/dgram/netlogon.c
@@ -74,6 +74,7 @@ NTSTATUS dgram_mailslot_netlogon_reply(struct nbt_dgram_socket *dgmsock,

  	status = push_nbt_netlogon_response(&blob, tmp_ctx, reply);
  	if (!NT_STATUS_IS_OK(status)) {
+		talloc_free(tmp_ctx);
  		return status;
  	}

diff --git a/source4/libcli/util/clilsa.c b/source4/libcli/util/clilsa.c
index 4a81457..05cc9b5 100644
--- a/source4/libcli/util/clilsa.c
+++ b/source4/libcli/util/clilsa.c
@@ -63,6 +63,7 @@ static NTSTATUS smblsa_connect(struct smbcli_state *cli)

  	lsa->ipc_tree = smbcli_tree_init(cli->session, lsa, false);
  	if (lsa->ipc_tree == NULL) {
+		talloc_free(lsa);
  		return NT_STATUS_NO_MEMORY;
  	}

@@ -219,11 +220,13 @@ NTSTATUS smblsa_lookup_sid(struct smbcli_state *cli,

  	status = smblsa_connect(cli);
  	if (!NT_STATUS_IS_OK(status)) {
+		talloc_free(mem_ctx2);
  		return status;
  	}

  	sid = dom_sid_parse_talloc(mem_ctx2, sid_str);
  	if (sid == NULL) {
+		talloc_free(mem_ctx2);
  		return NT_STATUS_INVALID_SID;
  	}

@@ -286,6 +289,7 @@ NTSTATUS smblsa_lookup_name(struct smbcli_state *cli,

  	status = smblsa_connect(cli);
  	if (!NT_STATUS_IS_OK(status)) {
+		talloc_free(mem_ctx2);
  		return status;
  	}

diff --git a/source4/libnet/libnet_samsync_ldb.c b/source4/libnet/libnet_samsync_ldb.c
index 20f0046..2590175 100644
--- a/source4/libnet/libnet_samsync_ldb.c
+++ b/source4/libnet/libnet_samsync_ldb.c
@@ -1225,12 +1225,14 @@ NTSTATUS libnet_samsync_ldb(struct libnet_context *ctx, TALLOC_CTX *mem_ctx, str
  					       r->in.session_info,
  						   0);
  	if (!state->sam_ldb) {
+		talloc_free(state);
  		return NT_STATUS_INTERNAL_DB_ERROR;
  	}

  	state->pdb             = privilege_connect(mem_ctx,
  						   ctx->lp_ctx);
  	if (!state->pdb) {
+		talloc_free(state);
  		return NT_STATUS_INTERNAL_DB_ERROR;
  	}

diff --git a/source4/libnet/libnet_site.c b/source4/libnet/libnet_site.c
index 288e13d..a218c10 100644
--- a/source4/libnet/libnet_site.c
+++ b/source4/libnet/libnet_site.c
@@ -63,6 +63,7 @@ NTSTATUS libnet_FindSite(TALLOC_CTX *ctx, struct libnet_context *lctx, struct li
  						r->in.cldap_port,
  						&dest_address);
  	if (ret != 0) {
+		talloc_free(tmp_ctx);
  		r->out.error_string = NULL;
  		status = map_nt_error_from_unix_common(errno);
  		return status;
diff --git a/source4/ntvfs/posix/pvfs_dirlist.c b/source4/ntvfs/posix/pvfs_dirlist.c
index 1bc91c1..8dfd7b3 100644
--- a/source4/ntvfs/posix/pvfs_dirlist.c
+++ b/source4/ntvfs/posix/pvfs_dirlist.c
@@ -120,6 +120,7 @@ NTSTATUS pvfs_list_start(struct pvfs_state *pvfs, struct pvfs_filename *name,
  	if (!pattern) {
  		/* this should not happen, as pvfs_unix_path is supposed to
  		   return an absolute path */
+		talloc_free(dir);
  		return NT_STATUS_UNSUCCESSFUL;
  	}

@@ -131,16 +132,19 @@ NTSTATUS pvfs_list_start(struct pvfs_state *pvfs, struct pvfs_filename *name,

  	dir->unix_path = talloc_strdup(dir, name->full_name);
  	if (!dir->unix_path) {
+		talloc_free(dir);
  		return NT_STATUS_NO_MEMORY;
  	}

  	dir->pattern = talloc_strdup(dir, pattern);
  	if (dir->pattern == NULL) {
+		talloc_free(dir);
  		return NT_STATUS_NO_MEMORY;
  	}

  	dir->dir = opendir(name->full_name);
  	if (!dir->dir) { 
+		talloc_free(dir);
  		return pvfs_map_errno(pvfs, errno);
  	}

diff --git a/source4/ntvfs/posix/pvfs_resolve.c b/source4/ntvfs/posix/pvfs_resolve.c
index 837ea17..532d7f8 100644
--- a/source4/ntvfs/posix/pvfs_resolve.c
+++ b/source4/ntvfs/posix/pvfs_resolve.c
@@ -434,7 +434,7 @@ static NTSTATUS pvfs_reduce_name(TALLOC_CTX *mem_ctx,
  	}
  	if (err_count) {
  		if (flags & PVFS_RESOLVE_WILDCARD) err_count--;
-
+		talloc_free(s);
  		if (err_count==1) {
  			return NT_STATUS_OBJECT_NAME_INVALID;
  		} else {
@@ -451,7 +451,10 @@ static NTSTATUS pvfs_reduce_name(TALLOC_CTX *mem_ctx,
  			continue;
  		}
  		if (ISDOTDOT(components[i])) {
-			if (i < 1) return NT_STATUS_OBJECT_PATH_SYNTAX_BAD;
+			if (i < 1) {
+				talloc_free(s);
+				return NT_STATUS_OBJECT_PATH_SYNTAX_BAD;
+			}
  			memmove(&components[i-1], &components[i+1],
  				sizeof(char *)*(num_components-i));
  			i -= 2;
diff --git a/source4/ntvfs/posix/pvfs_streams.c b/source4/ntvfs/posix/pvfs_streams.c
index cacd8c1..2507fe6 100644
--- a/source4/ntvfs/posix/pvfs_streams.c
+++ b/source4/ntvfs/posix/pvfs_streams.c
@@ -253,6 +253,7 @@ NTSTATUS pvfs_stream_rename(struct pvfs_state *pvfs, struct pvfs_filename *name,

  	new_name = stream_name_normalise(streams, new_name);
  	if (new_name == NULL) {
+		talloc_free(streams);
  		return NT_STATUS_NO_MEMORY;
  	}

@@ -264,6 +265,7 @@ NTSTATUS pvfs_stream_rename(struct pvfs_state *pvfs, struct pvfs_filename *name,
  	/* the default stream always exists */
  	if (strcmp(new_name, "") == 0 ||
  	    strcasecmp_m(new_name, ":$DATA") == 0) {
+		talloc_free(streams);
  		return NT_STATUS_OBJECT_NAME_COLLISION;
  	}

diff --git a/source4/ntvfs/posix/pvfs_xattr.c b/source4/ntvfs/posix/pvfs_xattr.c
index 04ad178..59715cd 100644
--- a/source4/ntvfs/posix/pvfs_xattr.c
+++ b/source4/ntvfs/posix/pvfs_xattr.c
@@ -171,6 +171,7 @@ NTSTATUS pvfs_dosattrib_load(struct pvfs_state *pvfs, struct pvfs_filename *name
  	}

  	if (!(pvfs->flags & PVFS_FLAG_XATTR_ENABLE)) {
+		talloc_free(mem_ctx);
  		return NT_STATUS_OK;
  	}

diff --git a/source4/utils/oLschema2ldif.c b/source4/utils/oLschema2ldif.c
index ae69db1..4e39fb4 100644
--- a/source4/utils/oLschema2ldif.c
+++ b/source4/utils/oLschema2ldif.c
@@ -353,7 +353,8 @@ static struct ldb_message *process_entry(TALLOC_CTX *mem_ctx, const char *entry)
  	ldb_msg_add_string(msg, "objectClass", "top");

  	c = talloc_strdup(ctx, entry);
-	if (!c) return NULL;
+	if (!c)
+		goto failed;

  	c = skip_spaces(c);

diff --git a/source4/winbind/wb_samba3_protocol.c b/source4/winbind/wb_samba3_protocol.c
index 2846e9c..2899876 100644
--- a/source4/winbind/wb_samba3_protocol.c
+++ b/source4/winbind/wb_samba3_protocol.c
@@ -352,6 +352,7 @@ NTSTATUS wbsrv_samba3_process(struct wbsrv_samba3_call *call)
  	status = wbsrv_samba3_pull_request(call);

  	if (!NT_STATUS_IS_OK(status)) {
+		talloc_free(call);
  		return status;
  	}



More information about the samba-technical mailing list