[PATCH] Remove all uses of talloc_autofree_context() from our code (except for test)

Jeremy Allison jra at samba.org
Tue Jul 25 16:05:07 UTC 2017


Hi all,

Here is a patch to finally remove all uses of
talloc_autofree_context() from the Samba code
tree, except for the talloc tests that exercise
the talloc_autofree_context() functionality.

The final patch removes the calls to:

talloc_enable_null_tracking()

inside nmbd, smbd, winbind and samba

(Hurrah!). This will allow new code to
depend on thread-safety of the NULL context
(so long as it's isolated within a specific
thread) and will greatly ease migration to
MT-safe nirvana, as explained by this picture:

http://bholley.net/blog/2015/must-be-this-tall-to-write-multi-threaded-code.html

:-).

Please review and push if happy ! I already added
a bug report for this, as I'm hoping to
get this into 4.7.0-final.

Cheers,

Jeremy.
-------------- next part --------------
From d9d7b614bfc32fdb7d936c30278ca988f4916e8e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 10:02:08 -0700
Subject: [PATCH 01/15] s4: modules. Fix missing TALLOC_CTX in module init
 function.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/rpc_server/remote/dcesrv_remote.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/rpc_server/remote/dcesrv_remote.c b/source4/rpc_server/remote/dcesrv_remote.c
index 69ce08cd1f7..b32fe9bb8d9 100644
--- a/source4/rpc_server/remote/dcesrv_remote.c
+++ b/source4/rpc_server/remote/dcesrv_remote.c
@@ -28,7 +28,7 @@
 #include "librpc/ndr/ndr_table.h"
 #include "param/param.h"
 
-NTSTATUS dcerpc_server_remote_init(void);
+NTSTATUS dcerpc_server_remote_init(TALLOC_CTX *ctx);
 
 struct dcesrv_remote_private {
 	struct dcerpc_pipe *c_pipe;
@@ -403,7 +403,7 @@ static bool remote_op_interface_by_name(struct dcesrv_interface *iface, const ch
 	return false;	
 }
 
-NTSTATUS dcerpc_server_remote_init(void)
+NTSTATUS dcerpc_server_remote_init(TALLOC_CTX *ctx)
 {
 	NTSTATUS ret;
 	static const struct dcesrv_endpoint_server ep_server = {
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 38920e1be481539d676384b8d68e3687c67c3951 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 11:43:47 -0700
Subject: [PATCH 02/15] lib: rpc: The registered interfaces are a lists of
 singletons that are never removed.

Allocate them off the NULL context not the talloc_autofree_context().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 librpc/ndr/ndr_table.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/librpc/ndr/ndr_table.c b/librpc/ndr/ndr_table.c
index 3dc158fa7ea..f64e5ac4d20 100644
--- a/librpc/ndr/ndr_table.c
+++ b/librpc/ndr/ndr_table.c
@@ -44,7 +44,17 @@ NTSTATUS ndr_table_register(const struct ndr_interface_table *table)
 		}
 	}
 
-	l = talloc(talloc_autofree_context(), struct ndr_interface_list);
+	/*
+	 * This is a singleton instance guarenteed
+	 * by the above check to be only added once
+	 * into the list so we can allocate off the NULL
+	 * context. We never want this to be freed
+	 * until process shutdown. If needed we could
+	 * add a deregister function that walks and
+	 * frees the list.
+	 */
+
+	l = talloc(NULL, struct ndr_interface_list);
 	l->table = table;
 
 	DLIST_ADD(ndr_interfaces, l);
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 3a5c73532cd6a20f14096d1ff6fafa0a312463a2 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 12:00:21 -0700
Subject: [PATCH 03/15] s4: COM: Remove talloc_autofree_context() from (unused)
 COM code.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 pidl/lib/Parse/Pidl/Samba4/COM/Proxy.pm |  6 +++---
 source4/lib/com/classes/simple.c        |  4 ++--
 source4/lib/com/com.h                   |  3 ++-
 source4/lib/com/dcom/tables.c           | 10 ++++++----
 source4/lib/com/tables.c                |  6 ++++--
 source4/lib/wmi/wbemdata.c              |  4 ++--
 6 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/pidl/lib/Parse/Pidl/Samba4/COM/Proxy.pm b/pidl/lib/Parse/Pidl/Samba4/COM/Proxy.pm
index 27e1e5d4243..35e6e3f0d19 100644
--- a/pidl/lib/Parse/Pidl/Samba4/COM/Proxy.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/COM/Proxy.pm
@@ -44,9 +44,9 @@ sub ParseRegFunc($)
 {
 	my $interface = shift;
 
-	$res .= "static NTSTATUS dcom_proxy_$interface->{NAME}_init(void)
+	$res .= "static NTSTATUS dcom_proxy_$interface->{NAME}_init(TALLOC_CTX *ctx)
 {
-	struct $interface->{NAME}_vtable *proxy_vtable = talloc(talloc_autofree_context(), struct $interface->{NAME}_vtable);
+	struct $interface->{NAME}_vtable *proxy_vtable = talloc(ctx, struct $interface->{NAME}_vtable);
 ";
 
 	if (defined($interface->{BASE})) {
@@ -75,7 +75,7 @@ sub ParseRegFunc($)
 	$res.= "
 	proxy_vtable->iid = ndr_table_$interface->{NAME}.syntax_id.uuid;
 
-	return dcom_register_proxy((struct IUnknown_vtable *)proxy_vtable);
+	return dcom_register_proxy(ctx, (struct IUnknown_vtable *)proxy_vtable);
 }\n\n";
 }
 
diff --git a/source4/lib/com/classes/simple.c b/source4/lib/com/classes/simple.c
index 28f5d848de5..7d0573372e3 100644
--- a/source4/lib/com/classes/simple.c
+++ b/source4/lib/com/classes/simple.c
@@ -110,7 +110,7 @@ static struct IStream_vtable simple_IStream_vtable = {
 NTSTATUS com_simple_init(TALLOC_CTX *ctx)
 {
 	struct GUID clsid;
-	struct IUnknown *class_object = talloc(talloc_autofree_context(), struct IUnknown);
+	struct IUnknown *class_object = talloc(ctx, struct IUnknown);
 
 	class_object->ctx = NULL;
 	class_object->object_data = NULL;
@@ -120,5 +120,5 @@ NTSTATUS com_simple_init(TALLOC_CTX *ctx)
 	GUID_from_string(COM_ICLASSFACTORY_UUID, &simple_classobject_vtable.iid);
 	GUID_from_string(COM_ISTREAM_UUID, &simple_IStream_vtable.iid);
 
-	return com_register_running_class(&clsid, PROGID_SIMPLE, class_object);
+	return com_register_running_class(ctx, &clsid, PROGID_SIMPLE, class_object);
 }
diff --git a/source4/lib/com/com.h b/source4/lib/com/com.h
index 82d10daf4fd..e6be3114678 100644
--- a/source4/lib/com/com.h
+++ b/source4/lib/com/com.h
@@ -21,6 +21,7 @@
 #define __SAMBA_COM_H__
 
 #include "librpc/gen_ndr/misc.h"
+#include "lib/talloc/talloc.h"
 
 struct com_context;
 struct tevent_context;
@@ -38,7 +39,7 @@ struct com_context
 };
 
 struct IUnknown *com_class_by_clsid(struct com_context *ctx, const struct GUID *clsid);
-NTSTATUS com_register_running_class(struct GUID *clsid, const char *progid, struct IUnknown *p);
+NTSTATUS com_register_running_class(TALLOC_CTX *ctx, struct GUID *clsid, const char *progid, struct IUnknown *p);
 
 struct dcom_interface_p *dcom_get_local_iface_p(struct GUID *ipid);
 
diff --git a/source4/lib/com/dcom/tables.c b/source4/lib/com/dcom/tables.c
index f94aa879253..7f745c1d479 100644
--- a/source4/lib/com/dcom/tables.c
+++ b/source4/lib/com/dcom/tables.c
@@ -29,9 +29,10 @@ static struct dcom_proxy {
 	struct dcom_proxy *prev, *next;
 }  *proxies = NULL;
 
-NTSTATUS dcom_register_proxy(struct IUnknown_vtable *proxy_vtable)
+NTSTATUS dcom_register_proxy(TALLOC_CTX *ctx,
+		struct IUnknown_vtable *proxy_vtable)
 {
-	struct dcom_proxy *proxy = talloc(talloc_autofree_context(), struct dcom_proxy);
+	struct dcom_proxy *proxy = talloc(ctx, struct dcom_proxy);
 
 	proxy->vtable = proxy_vtable;
 	DLIST_ADD(proxies, proxy);
@@ -57,9 +58,10 @@ static struct dcom_marshal {
 	struct dcom_marshal *prev, *next;
 }  *marshals = NULL;
 
-NTSTATUS dcom_register_marshal(struct GUID *clsid, marshal_fn marshal, unmarshal_fn unmarshal)
+NTSTATUS dcom_register_marshal(TALLOC_CTX *ctx,
+		struct GUID *clsid, marshal_fn marshal, unmarshal_fn unmarshal)
 {
-	struct dcom_marshal *p = talloc(talloc_autofree_context(), struct dcom_marshal);
+	struct dcom_marshal *p = talloc(ctx, struct dcom_marshal);
 
 	p->clsid = *clsid;
 	p->marshal = marshal;
diff --git a/source4/lib/com/tables.c b/source4/lib/com/tables.c
index 842067e8a54..e1f93bcae59 100644
--- a/source4/lib/com/tables.c
+++ b/source4/lib/com/tables.c
@@ -96,9 +96,11 @@ struct IUnknown *com_class_by_clsid(struct com_context *ctx, const struct GUID *
 	return NULL;
 }
 
-NTSTATUS com_register_running_class(struct GUID *clsid, const char *progid, struct IUnknown *p)
+NTSTATUS com_register_running_class(TALLOC_CTX *ctx,
+		struct GUID *clsid, const char *progid, struct IUnknown *p)
 {
-	struct com_class *l = talloc_zero(running_classes?running_classes:talloc_autofree_context(), struct com_class);
+	struct com_class *l = talloc_zero(running_classes?
+				running_classes : ctx, struct com_class);
 
 	l->clsid = *clsid;
 	l->progid = talloc_strdup(l, progid);
diff --git a/source4/lib/wmi/wbemdata.c b/source4/lib/wmi/wbemdata.c
index 2aeda01a50e..df98da43a2a 100644
--- a/source4/lib/wmi/wbemdata.c
+++ b/source4/lib/wmi/wbemdata.c
@@ -432,11 +432,11 @@ struct composite_context *dcom_proxy_IEnumWbemClassObject_Release_send(struct IU
 	return c;
 }
 
-NTSTATUS dcom_proxy_IWbemClassObject_init(void)
+NTSTATUS dcom_proxy_IWbemClassObject_init(TALLOC_CTX *ctx)
 {
 	struct GUID clsid;
 	GUID_from_string("4590f812-1d3a-11d0-891f-00aa004b2e24", &clsid);
-	dcom_register_marshal(&clsid, marshal, unmarshal);
+	dcom_register_marshal(ctx, &clsid, marshal, unmarshal);
 
 #if 0
 	struct IEnumWbemClassObject_vtable *proxy_vtable;
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 439fd847b87aaea16a4b965f5d3c6cef47dd8f21 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 12:11:03 -0700
Subject: [PATCH 04/15] lib: ldb: Use NULL to allocate modules not
 talloc_autofree_context().

ldb modules are not (yet) unloaded and are only loaded once (there is a check
that makes sure of this). Allocate off the NULL context. We never want this
to be freed until process shutdown. If eventually we add the ability to
unload ldb modules we can add a deregister function that walks and frees the list.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/ldb/common/ldb_modules.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/ldb/common/ldb_modules.c b/lib/ldb/common/ldb_modules.c
index 3dd0438c128..25551e1a2ed 100644
--- a/lib/ldb/common/ldb_modules.c
+++ b/lib/ldb/common/ldb_modules.c
@@ -280,7 +280,17 @@ int ldb_register_module(const struct ldb_module_ops *ops)
 	if (ldb_find_module_ops(ops->name) != NULL)
 		return LDB_ERR_ENTRY_ALREADY_EXISTS;
 
-	entry = talloc(talloc_autofree_context(), struct ops_list_entry);
+	/*
+	 * ldb modules are not (yet) unloaded and
+	 * are only loaded once (the above check
+	 * makes sure of this). Allocate off the NULL
+	 * context. We never want this to be freed
+	 * until process shutdown. If eventually we
+	 * want to unload ldb modules we can add a
+	 * deregister function that walks and
+	 * frees the list.
+	 */
+	entry = talloc(NULL, struct ops_list_entry);
 	if (entry == NULL) {
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From fb5482875e14924e15a55c27a348452999052539 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 12:12:17 -0700
Subject: [PATCH 05/15] lib: ldb: Python. Take care of freeing the passed in
 module description if ldb_register_module() fails.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/ldb/pyldb.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index b65e25525ad..515a69e6981 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -3949,7 +3949,7 @@ static PyObject *py_register_module(PyObject *module, PyObject *args)
 	if (!PyArg_ParseTuple(args, "O", &input))
 		return NULL;
 
-	ops = talloc_zero(talloc_autofree_context(), struct ldb_module_ops);
+	ops = talloc_zero(NULL, struct ldb_module_ops);
 	if (ops == NULL) {
 		PyErr_NoMemory();
 		return NULL;
@@ -3972,6 +3972,9 @@ static PyObject *py_register_module(PyObject *module, PyObject *args)
 	ops->del_transaction = py_module_del_transaction;
 
 	ret = ldb_register_module(ops);
+	if (ret != LDB_SUCCESS) {
+		TALLOC_FREE(ops);
+	}
 
 	PyErr_LDB_ERROR_IS_ERR_RAISE(PyExc_LdbError, ret, NULL);
 
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From ba527e2e5cf61865e541847a2acbe75efc039d10 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 12:50:50 -0700
Subject: [PATCH 06/15] s4: schema: Allocate global_schema off the NULL
 context, not the talloc_autofree_context().

The ldb context keeps a talloc_reference to this also, so the long-live allocation
context can be NULL.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/dsdb/schema/schema_set.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index df27e19a944..28bedb30895 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -714,11 +714,11 @@ void dsdb_make_schema_global(struct ldb_context *ldb, struct dsdb_schema *schema
 	}
 
 	if (global_schema) {
-		talloc_unlink(talloc_autofree_context(), global_schema);
+		talloc_unlink(NULL, global_schema);
 	}
 
 	/* we want the schema to be around permanently */
-	talloc_reparent(ldb, talloc_autofree_context(), schema);
+	talloc_reparent(ldb, NULL, schema);
 	global_schema = schema;
 
 	/* This calls the talloc_reference() of the global schema back onto the ldb */
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 67c6657492dc399e368cea00aa58438ee0083e1c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 12:56:15 -0700
Subject: [PATCH 07/15] lib: cli: fname is a local variable already freed in
 the function scope, doesn't need to be on talloc_autofree_context()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 libcli/auth/netlogon_creds_cli.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcli/auth/netlogon_creds_cli.c b/libcli/auth/netlogon_creds_cli.c
index 367bf6caaff..6b32c653e2f 100644
--- a/libcli/auth/netlogon_creds_cli.c
+++ b/libcli/auth/netlogon_creds_cli.c
@@ -221,7 +221,7 @@ NTSTATUS netlogon_creds_cli_open_global_db(struct loadparm_context *lp_ctx)
 		return NT_STATUS_OK;
 	}
 
-	fname = lpcfg_private_db_path(talloc_autofree_context(), lp_ctx, "netlogon_creds_cli");
+	fname = lpcfg_private_db_path(NULL, lp_ctx, "netlogon_creds_cli");
 	if (fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 9828fa81f2c6a5ef33c2bca65c9e99d75f0d7a86 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 12:58:29 -0700
Subject: [PATCH 08/15] s3: rpc_client: Allocate struct db_context * off the
 local frame, as all other variables in this function.

On success, netlogon_creds_cli_set_global_db() reparents it to a long-lived context.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/rpc_client/cli_netlogon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/rpc_client/cli_netlogon.c b/source3/rpc_client/cli_netlogon.c
index d17c6c0c1ae..719b98584f3 100644
--- a/source3/rpc_client/cli_netlogon.c
+++ b/source3/rpc_client/cli_netlogon.c
@@ -67,7 +67,7 @@ NTSTATUS rpccli_pre_open_netlogon_creds(void)
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	global_db = db_open(talloc_autofree_context(), fname,
+	global_db = db_open(frame, fname,
 			    0, TDB_CLEAR_IF_FIRST|TDB_INCOMPATIBLE_HASH,
 			    O_RDWR|O_CREAT, 0600, DBWRAP_LOCK_ORDER_2,
 			    DBWRAP_FLAG_OPTIMIZE_READONLY_ACCESS);
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 119cea723bbca661d81990f53be4c7666e758931 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 13:10:30 -0700
Subject: [PATCH 09/15] s3: rpcclient: Split out initialization and free of
 event context.

Allows us to control shutdown.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/rpcclient/rpcclient.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/source3/rpcclient/rpcclient.c b/source3/rpcclient/rpcclient.c
index be7769672c0..835b288ee20 100644
--- a/source3/rpcclient/rpcclient.c
+++ b/source3/rpcclient/rpcclient.c
@@ -948,6 +948,7 @@ out_free:
 	const char *binding_string = NULL;
 	const char *host;
 	int signing_state = SMB_SIGNING_IPC_DEFAULT;
+	struct tevent_context *ev_ctx = NULL;
 
 	/* make sure the vars that get altered (4th field) are in
 	   a fixed location or certain compilers complain */
@@ -1013,8 +1014,15 @@ out_free:
 	poptFreeContext(pc);
 	popt_burn_cmdline_password(argc, argv);
 
+	ev_ctx = samba_tevent_context_init(frame);
+	if (ev_ctx == NULL) {
+		fprintf(stderr, "Could not init event context\n");
+		result = 1;
+		goto done;
+	}
+
 	nt_status = messaging_init_client(talloc_autofree_context(),
-					  samba_tevent_context_init(talloc_autofree_context()),
+					  ev_ctx,
 					  &rpcclient_msg_ctx);
 	if (geteuid() != 0 &&
 			NT_STATUS_EQUAL(nt_status, NT_STATUS_ACCESS_DENIED)) {
@@ -1246,6 +1254,7 @@ done:
 		cli_shutdown(cli);
 	}
 	popt_free_cmdline_auth_info();
+	TALLOC_FREE(ev_ctx);
 	TALLOC_FREE(frame);
 	return result;
 }
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 3d8226e474b4eb105b67a529ff01eb3ad13e8a86 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 13:12:20 -0700
Subject: [PATCH 10/15] s3: rpcclient: Use event context as the talloc parent
 of the rpcclient_msg_ctx.

Give control over shutdown.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/rpcclient/rpcclient.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/rpcclient/rpcclient.c b/source3/rpcclient/rpcclient.c
index 835b288ee20..96c7410868a 100644
--- a/source3/rpcclient/rpcclient.c
+++ b/source3/rpcclient/rpcclient.c
@@ -1021,7 +1021,7 @@ out_free:
 		goto done;
 	}
 
-	nt_status = messaging_init_client(talloc_autofree_context(),
+	nt_status = messaging_init_client(ev_ctx,
 					  ev_ctx,
 					  &rpcclient_msg_ctx);
 	if (geteuid() != 0 &&
@@ -1254,6 +1254,7 @@ done:
 		cli_shutdown(cli);
 	}
 	popt_free_cmdline_auth_info();
+	TALLOC_FREE(rpcclient_msg_ctx);
 	TALLOC_FREE(ev_ctx);
 	TALLOC_FREE(frame);
 	return result;
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 603c05eb392119578cc61c8ccf98fd3d70aadba8 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 13:14:08 -0700
Subject: [PATCH 11/15] s3: rpcclient: Use rpcclient_msg_ctx as the long-lived
 talloc context for rpcclient_netlogon_creds.

Guaranteed to stay around until the command exits.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/rpcclient/rpcclient.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/rpcclient/rpcclient.c b/source3/rpcclient/rpcclient.c
index 96c7410868a..0f3dcc6cb1a 100644
--- a/source3/rpcclient/rpcclient.c
+++ b/source3/rpcclient/rpcclient.c
@@ -766,7 +766,7 @@ static NTSTATUS do_cmd(struct cli_state *cli,
 				default_transport,
 				rpcclient_netlogon_domain,
 				&cmd_entry->rpc_pipe,
-				talloc_autofree_context(),
+				rpcclient_msg_ctx,
 				&rpcclient_netlogon_creds);
 			break;
 		default:
@@ -805,7 +805,7 @@ static NTSTATUS do_cmd(struct cli_state *cli,
 			ntresult = rpccli_create_netlogon_creds_with_creds(creds,
 							dc_name,
 							rpcclient_msg_ctx,
-							talloc_autofree_context(),
+							rpcclient_msg_ctx,
 							&rpcclient_netlogon_creds);
 			if (!NT_STATUS_IS_OK(ntresult)) {
 				DEBUG(0, ("Could not initialise credentials for %s.\n",
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From cff570ed2582af07b7fd54e69db2cf78e28c9ef3 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 14:49:47 -0700
Subject: [PATCH 12/15] lib: auth: Add a shutdown function for
 netlogon_creds_cli_global_db.

Will allow us to move off the talloc_autofree_context().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 libcli/auth/netlogon_creds_cli.c | 5 +++++
 libcli/auth/netlogon_creds_cli.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/libcli/auth/netlogon_creds_cli.c b/libcli/auth/netlogon_creds_cli.c
index 6b32c653e2f..f5350f210b3 100644
--- a/libcli/auth/netlogon_creds_cli.c
+++ b/libcli/auth/netlogon_creds_cli.c
@@ -244,6 +244,11 @@ NTSTATUS netlogon_creds_cli_open_global_db(struct loadparm_context *lp_ctx)
 	return NT_STATUS_OK;
 }
 
+void netlogon_creds_cli_close_global_db(void)
+{
+	TALLOC_FREE(netlogon_creds_cli_global_db);
+}
+
 NTSTATUS netlogon_creds_cli_context_global(struct loadparm_context *lp_ctx,
 				struct messaging_context *msg_ctx,
 				const char *client_account,
diff --git a/libcli/auth/netlogon_creds_cli.h b/libcli/auth/netlogon_creds_cli.h
index cecb0e605c4..32902f103a9 100644
--- a/libcli/auth/netlogon_creds_cli.h
+++ b/libcli/auth/netlogon_creds_cli.h
@@ -32,6 +32,7 @@ struct db_context;
 
 NTSTATUS netlogon_creds_cli_set_global_db(struct db_context **db);
 NTSTATUS netlogon_creds_cli_open_global_db(struct loadparm_context *lp_ctx);
+void netlogon_creds_cli_close_global_db(void);
 
 NTSTATUS netlogon_creds_cli_context_global(struct loadparm_context *lp_ctx,
 				struct messaging_context *msg_ctx,
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 2ec5890e9a2f931431a4437122049f1977319a6d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 16:12:45 -0700
Subject: [PATCH 13/15] s3: clients: Use netlogon_creds_cli_close_global_db()
 in all normal exit paths.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/includes.h    | 1 +
 source3/lib/netapi/netapi.c   | 2 ++
 source3/rpcclient/rpcclient.c | 1 +
 source3/smbd/server_exit.c    | 2 ++
 source3/winbindd/winbindd.c   | 3 +++
 5 files changed, 9 insertions(+)

diff --git a/source3/include/includes.h b/source3/include/includes.h
index e82bfad4147..58bfaa719a1 100644
--- a/source3/include/includes.h
+++ b/source3/include/includes.h
@@ -317,6 +317,7 @@ typedef char fstring[FSTRING_LEN];
 
 #include "../libcli/util/ntstatus.h"
 #include "../libcli/util/error.h"
+#include "../libcli/auth/netlogon_creds_cli.h"
 #include "../lib/util/charset/charset.h"
 #include "dynconfig/dynconfig.h"
 #include "locking.h"
diff --git a/source3/lib/netapi/netapi.c b/source3/lib/netapi/netapi.c
index 3ed72952821..093348baefa 100644
--- a/source3/lib/netapi/netapi.c
+++ b/source3/lib/netapi/netapi.c
@@ -184,6 +184,8 @@ NET_API_STATUS libnetapi_free(struct libnetapi_ctx *ctx)
 
 	secrets_shutdown();
 
+	netlogon_creds_cli_close_global_db();
+
 	if (ctx == stat_ctx) {
 		stat_ctx = NULL;
 	}
diff --git a/source3/rpcclient/rpcclient.c b/source3/rpcclient/rpcclient.c
index 0f3dcc6cb1a..3203df24c07 100644
--- a/source3/rpcclient/rpcclient.c
+++ b/source3/rpcclient/rpcclient.c
@@ -1254,6 +1254,7 @@ done:
 		cli_shutdown(cli);
 	}
 	popt_free_cmdline_auth_info();
+	netlogon_creds_cli_close_global_db();
 	TALLOC_FREE(rpcclient_msg_ctx);
 	TALLOC_FREE(ev_ctx);
 	TALLOC_FREE(frame);
diff --git a/source3/smbd/server_exit.c b/source3/smbd/server_exit.c
index bf50394f4bf..74ddd70bd3a 100644
--- a/source3/smbd/server_exit.c
+++ b/source3/smbd/server_exit.c
@@ -46,6 +46,7 @@
 #include "messages.h"
 #include "../lib/util/pidfile.h"
 #include "smbprofile.h"
+#include "libcli/auth/netlogon_creds_cli.h"
 
 static struct files_struct *log_writeable_file_fn(
 	struct files_struct *fsp, void *private_data)
@@ -226,6 +227,7 @@ static void exit_server_common(enum server_exit_reason how,
 	sconn = NULL;
 	xconn = NULL;
 	client = NULL;
+	netlogon_creds_cli_close_global_db();
 	TALLOC_FREE(global_smbXsrv_client);
 	smbprofile_dump();
 	server_messaging_context_free();
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 58e4d89e4af..f24451649b6 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -44,6 +44,7 @@
 #include "lib/param/param.h"
 #include "lib/async_req/async_sock.h"
 #include "libsmb/samlogon_cache.h"
+#include "libcli/auth/netlogon_creds_cli.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
@@ -245,6 +246,8 @@ static void terminate(bool is_parent)
 
 	gencache_stabilize();
 
+	netlogon_creds_cli_close_global_db();
+
 #if 0
 	if (interactive) {
 		TALLOC_CTX *mem_ctx = talloc_init("end_description");
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From e04577ea9eb860e41346c07557e0b49c8b50db0c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 16:14:00 -0700
Subject: [PATCH 14/15] lib: auth: Store the netlogon_creds_cli_global_db
 pointer on the NULL context.

Now we shutdown correctly it doesn't need the talloc_autofree_context().

Last use of talloc_autofree_context() ourside the talloc test code !

Please don't add it ever again :-).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 libcli/auth/netlogon_creds_cli.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcli/auth/netlogon_creds_cli.c b/libcli/auth/netlogon_creds_cli.c
index f5350f210b3..526ee3962fc 100644
--- a/libcli/auth/netlogon_creds_cli.c
+++ b/libcli/auth/netlogon_creds_cli.c
@@ -208,7 +208,7 @@ NTSTATUS netlogon_creds_cli_set_global_db(struct db_context **db)
 		return NT_STATUS_INVALID_PARAMETER_MIX;
 	}
 
-	netlogon_creds_cli_global_db = talloc_move(talloc_autofree_context(), db);
+	netlogon_creds_cli_global_db = talloc_move(NULL, db);
 	return NT_STATUS_OK;
 }
 
@@ -226,7 +226,7 @@ NTSTATUS netlogon_creds_cli_open_global_db(struct loadparm_context *lp_ctx)
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	global_db = dbwrap_local_open(talloc_autofree_context(), lp_ctx,
+	global_db = dbwrap_local_open(NULL, lp_ctx,
 				      fname, 0,
 				      TDB_CLEAR_IF_FIRST|TDB_INCOMPATIBLE_HASH,
 				      O_RDWR|O_CREAT,
-- 
2.14.0.rc0.284.gd933b75aa4-goog


From 8fe47ee9d223a273d6501c6b335569a2fe10e1f9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Jul 2017 16:21:38 -0700
Subject: [PATCH 15/15] s3/s4: Remove all uses of
 talloc_enable_null_tracking().

We are now able to use talloc NULL contexts safely within
threaded code.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12932

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/nmbd/nmbd.c         | 1 -
 source3/smbd/server.c       | 1 -
 source3/winbindd/winbindd.c | 1 -
 source4/smbd/server.c       | 2 --
 4 files changed, 5 deletions(-)

diff --git a/source3/nmbd/nmbd.c b/source3/nmbd/nmbd.c
index 14eaef647d0..6f929745499 100644
--- a/source3/nmbd/nmbd.c
+++ b/source3/nmbd/nmbd.c
@@ -808,7 +808,6 @@ static bool open_sockets(bool isdaemon, int port)
 	/*
 	 * Do this before any other talloc operation
 	 */
-	talloc_enable_null_tracking();
 	frame = talloc_stackframe();
 
 	/*
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 181bcd1e123..450135530f9 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -1632,7 +1632,6 @@ extern void build_options(bool screen);
 	/*
 	 * Do this before any other talloc operation
 	 */
-	talloc_enable_null_tracking();
 	frame = talloc_stackframe();
 
 	setup_logging(argv[0], DEBUG_DEFAULT_STDOUT);
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index f24451649b6..17ae3dfd98e 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -1545,7 +1545,6 @@ int main(int argc, const char **argv)
 	/*
 	 * Do this before any other talloc operation
 	 */
-	talloc_enable_null_tracking();
 	frame = talloc_stackframe();
 
 	/*
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index a8bad06bed3..15685793d59 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -414,8 +414,6 @@ static int binary_smbd_main(const char *binary_name,
 
 	poptFreeContext(pc);
 
-	talloc_enable_null_tracking();
-
 	setup_logging(binary_name, opt_interactive?DEBUG_STDOUT:DEBUG_FILE);
 	setup_signals();
 
-- 
2.14.0.rc0.284.gd933b75aa4-goog



More information about the samba-technical mailing list