s4 librpc rpc pyrpc: Ensure tevent_context deleted last

Ensure that the tevent_context is deleted after the connection, to
prevent a use after free.

Note: Py_DECREF calls dcerpc_interface_dealloc so the
TALLOC_FREE(ret->mem_ctx) calls in the error paths of
py_dcerpc_interface_init_helper needed removal.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13932

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

3 files changed, 39 insertions, 28 deletions

diff --git a/source4/librpc/rpc/pyrpc.c b/source4/librpc/rpc/pyrpc.c
index e00318a96be..0713920e3a5 100644
--- a/source4/librpc/rpc/pyrpc.c
+++ b/source4/librpc/rpc/pyrpc.c
@@ -300,9 +300,27 @@ static PyMethodDef dcerpc_interface_methods[] = {
 static void dcerpc_interface_dealloc(PyObject* self)
 {
 	dcerpc_InterfaceObject *interface = (dcerpc_InterfaceObject *)self;
+
+	/*
+	 * This can't fail, and if it did talloc_unlink(NULL, NULL) is
+	 * harmless below
+	 */
+	struct tevent_context *ev_save = talloc_reparent(
+		NULL, interface->mem_ctx, interface->ev);
+
 	interface->binding_handle = NULL;
 	interface->pipe = NULL;
+
+	/*
+	 * Free everything *except* the event context, which must go
+	 * away last
+	 */
 	TALLOC_FREE(interface->mem_ctx);
+
+	/*
+	 * Now wish a fond goodbye to the event context itself
+	 */
+	talloc_unlink(NULL, ev_save);
 	self->ob_type->tp_free(self);
 }
 
diff --git a/source4/librpc/rpc/pyrpc.h b/source4/librpc/rpc/pyrpc.h
index 7101e7345de..311ba2d294d 100644
--- a/source4/librpc/rpc/pyrpc.h
+++ b/source4/librpc/rpc/pyrpc.h
@@ -49,6 +49,7 @@ typedef struct {
 	TALLOC_CTX *mem_ctx;
 	struct dcerpc_pipe *pipe;
 	struct dcerpc_binding_handle *binding_handle;
+	struct tevent_context *ev;
 } dcerpc_InterfaceObject;
 
 
diff --git a/source4/librpc/rpc/pyrpc_util.c b/source4/librpc/rpc/pyrpc_util.c
index 8d5ec45817f..8015e709964 100644
--- a/source4/librpc/rpc/pyrpc_util.c
+++ b/source4/librpc/rpc/pyrpc_util.c
@@ -123,6 +123,7 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 
 	ret->pipe = NULL;
 	ret->binding_handle = NULL;
+	ret->ev = NULL;
 	ret->mem_ctx = talloc_new(NULL);
 	if (ret->mem_ctx == NULL) {
 		PyErr_NoMemory();
@@ -130,30 +131,26 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 	}
 
 	if (strncmp(binding_string, "irpc:", 5) == 0) {
-		struct tevent_context *event_ctx;
 		struct loadparm_context *lp_ctx;
 
-		event_ctx = s4_event_context_init(ret->mem_ctx);
-		if (event_ctx == NULL) {
+		ret->ev = s4_event_context_init(ret->mem_ctx);
+		if (ret->ev == NULL) {
 			PyErr_SetString(PyExc_TypeError, "Expected loadparm context");
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
 
-		lp_ctx = lpcfg_from_py_object(event_ctx, py_lp_ctx);
+		lp_ctx = lpcfg_from_py_object(ret->ev, py_lp_ctx);
 		if (lp_ctx == NULL) {
 			PyErr_SetString(PyExc_TypeError, "Expected loadparm context");
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
 
 		status = pyrpc_irpc_connect(ret->mem_ctx, binding_string+5, table,
-					    event_ctx, lp_ctx, &ret->binding_handle);
+					    ret->ev, lp_ctx, &ret->binding_handle);
 		if (!NT_STATUS_IS_OK(status)) {
 			PyErr_SetNTSTATUS(status);
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
@@ -164,7 +161,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 
 		py_base = PyImport_ImportModule("samba.dcerpc.base");
 		if (py_base == NULL) {
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
@@ -172,7 +168,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 		ClientConnection_Type = (PyTypeObject *)PyObject_GetAttrString(py_base, "ClientConnection");
 		if (ClientConnection_Type == NULL) {
 			PyErr_SetNone(PyExc_TypeError);
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			Py_DECREF(py_base);
 			return NULL;
@@ -180,7 +175,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 
 		if (!PyObject_TypeCheck(py_basis, ClientConnection_Type)) {
 			PyErr_SetString(PyExc_TypeError, "basis_connection must be a DCE/RPC connection");
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			Py_DECREF(py_base);
 			Py_DECREF(ClientConnection_Type);
@@ -191,7 +185,17 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 					 ((dcerpc_InterfaceObject *)py_basis)->pipe);
 		if (base_pipe == NULL) {
 			PyErr_NoMemory();
-			TALLOC_FREE(ret->mem_ctx);
+			Py_DECREF(ret);
+			Py_DECREF(py_base);
+			Py_DECREF(ClientConnection_Type);
+			return NULL;
+		}
+
+		ret->ev = talloc_reference(
+			ret->mem_ctx,
+			((dcerpc_InterfaceObject *)py_basis)->ev);
+		if (ret->ev == NULL) {
+			PyErr_NoMemory();
 			Py_DECREF(ret);
 			Py_DECREF(py_base);
 			Py_DECREF(ClientConnection_Type);
@@ -201,7 +205,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 		status = dcerpc_secondary_context(base_pipe, &ret->pipe, table);
 		if (!NT_STATUS_IS_OK(status)) {
 			PyErr_SetNTSTATUS(status);
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			Py_DECREF(py_base);
 			Py_DECREF(ClientConnection_Type);
@@ -212,22 +215,19 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 		Py_XDECREF(ClientConnection_Type);
 		Py_XDECREF(py_base);
 	} else {
-		struct tevent_context *event_ctx;
 		struct loadparm_context *lp_ctx;
 		struct cli_credentials *credentials;
 
-		event_ctx = s4_event_context_init(ret->mem_ctx);
-		if (event_ctx == NULL) {
+		ret->ev = s4_event_context_init(ret->mem_ctx);
+		if (ret->ev == NULL) {
 			PyErr_SetString(PyExc_TypeError, "Expected loadparm context");
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
 
-		lp_ctx = lpcfg_from_py_object(event_ctx, py_lp_ctx);
+		lp_ctx = lpcfg_from_py_object(ret->ev, py_lp_ctx);
 		if (lp_ctx == NULL) {
 			PyErr_SetString(PyExc_TypeError, "Expected loadparm context");
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
@@ -235,24 +235,16 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py
 		credentials = cli_credentials_from_py_object(py_credentials);
 		if (credentials == NULL) {
 			PyErr_SetString(PyExc_TypeError, "Expected credentials");
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
 		status = dcerpc_pipe_connect(ret->mem_ctx, &ret->pipe, binding_string,
-			     table, credentials, event_ctx, lp_ctx);
+			     table, credentials, ret->ev, lp_ctx);
 		if (!NT_STATUS_IS_OK(status)) {
 			PyErr_SetNTSTATUS(status);
-			TALLOC_FREE(ret->mem_ctx);
 			Py_DECREF(ret);
 			return NULL;
 		}
-
-		/*
-		 * the event context is cached under the connection,
-		 * so let it be a child of it.
-		 */
-		talloc_steal(ret->pipe->conn, event_ctx);
 	}
 
 	if (ret->pipe) {