[PATCH] add missing talloc_free
Richard Sharpe
realrichardsharpe at gmail.com
Sat Feb 25 22:10:47 MST 2012
On Sat, Feb 25, 2012 at 12:32 PM, Julia Lawall <julia.lawall at lip6.fr> wrote:
> There seem to be a number of places in the samba code where calls to
> talloc_free are omitted. This patch fixes some cases.
Are you aware of the way that talloc works? It seems likely to me that
when a talloc_free is done somewhere further up in the call stack that
if frees all memory.
Of course, it is possible that you have found a problem.
> 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;
> }
>
--
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)
More information about the samba-technical
mailing list