[Patches] The way to remove gensec_update_ev()
Stefan Metzmacher
metze at samba.org
Mon May 29 07:55:53 UTC 2017
Hi Andrew,
it seems I was to optimistic while removing the
inhibit_timeout_processing hack. There's a bit more to do
before we can remove it.
I have also a pending commit to fix a talloc_steal vs. talloc_reparent
and the removale of py_com.
Please review and push.
Thanks!
metze
> ==15975== Invalid read of size 8
> ==15975== at 0xCAEB4E9: gensec_update_cleanup (gensec.c:471)
> ==15975== by 0x70F7F7F: tevent_req_cleanup (tevent_req.c:139)
> ==15975== by 0x70F8263: tevent_req_received (tevent_req.c:253)
> ==15975== by 0x70F7EB9: tevent_req_destructor (tevent_req.c:107)
> ==15975== by 0x69578BD: _tc_free_internal (talloc.c:1078)
> ==15975== by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975== by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975== by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975== by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975== by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975== by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975== by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975== by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975== by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975== by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975== by 0x6957D5A: _talloc_free_internal (talloc.c:1174)
> ==15975== by 0x6959022: _talloc_free (talloc.c:1716)
> ==15975== by 0xA036CC7: dcerpc_pipe_connect_b_recv
> (dcerpc_connect.c:1116)
> ==15975== by 0xA036F78: continue_pipe_connect_b
> (dcerpc_connect.c:1207)
> ==15975== by 0xD6EF182: composite_error (composite.c:114)
> ==15975== by 0xA036928: dcerpc_connect_timeout_handler
> (dcerpc_connect.c:1009)
> ==15975== by 0x70FE311: tevent_common_loop_timer_delay
> (tevent_timed.c:369)
> ==15975== by 0x70FFE24: epoll_event_loop (tevent_epoll.c:659)
> ==15975== by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
> ==15975== by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
> ==15975== by 0x70F61C5: _tevent_loop_once (tevent.c:721)
> ==15975== by 0xD91760F: smb_krb5_send_and_recv_func_int
> (krb5_init_context.c:342)
> ==15975== by 0xD9179A5: smb_krb5_send_and_recv_func
> (krb5_init_context.c:431)
> ==15975== by 0xF353620: krb5_sendto (send_to_kdc.c:391)
> ==15975== by 0xF353D50: krb5_sendto_context (send_to_kdc.c:626)
> ==15975== by 0xF3365F7: krb5_init_creds_get (init_creds_pw.c:1959)
> ==15975== by 0xF3368D0: krb5_get_init_creds_password
> (init_creds_pw.c:2038)
> ==15975== Address 0x5e516b0 is 224 bytes inside a block of size 232
> free'd
> ==15975== at 0x4C2CDDB: free (vg_replace_malloc.c:530)
> ==15975== by 0x6957CB6: _tc_free_internal (talloc.c:1148)
> ==15975== by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975== by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975== by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975== by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975== by 0x6957D5A: _talloc_free_internal (talloc.c:1174)
> ==15975== by 0x6959022: _talloc_free (talloc.c:1716)
> ==15975== by 0xA036CC7: dcerpc_pipe_connect_b_recv
> (dcerpc_connect.c:1116)
> ==15975== by 0xA036F78: continue_pipe_connect_b
> (dcerpc_connect.c:1207)
> ==15975== by 0xD6EF182: composite_error (composite.c:114)
> ==15975== by 0xA036928: dcerpc_connect_timeout_handler
> (dcerpc_connect.c:1009)
> ==15975== by 0x70FE311: tevent_common_loop_timer_delay
> (tevent_timed.c:369)
> ==15975== by 0x70FFE24: epoll_event_loop (tevent_epoll.c:659)
> ==15975== by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
> ==15975== by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
> ==15975== by 0x70F61C5: _tevent_loop_once (tevent.c:721)
> ==15975== by 0xD91760F: smb_krb5_send_and_recv_func_int
> (krb5_init_context.c:342)
> ==15975== by 0xD9179A5: smb_krb5_send_and_recv_func
> (krb5_init_context.c:431)
> ==15975== by 0xF353620: krb5_sendto (send_to_kdc.c:391)
> ==15975== by 0xF353D50: krb5_sendto_context (send_to_kdc.c:626)
> ==15975== by 0xF3365F7: krb5_init_creds_get (init_creds_pw.c:1959)
> ==15975== by 0xF3368D0: krb5_get_init_creds_password
> (init_creds_pw.c:2038)
> ==15975== by 0xEAA7153: smb_krb5_kinit_password_ccache
> (krb5_samba.c:1890)
> ==15975== by 0xA258B53: kinit_to_ccache (kerberos_util.c:351)
> ==15975== by 0xA2545D4: cli_credentials_get_named_ccache
> (credentials_krb5.c:558)
> ==15975== by 0xA254691: cli_credentials_get_ccache
> (credentials_krb5.c:581)
> ==15975== by 0xA254B17: cli_credentials_get_client_gss_creds
> (credentials_krb5.c:703)
> ==15975== by 0xCAEF276: gensec_gssapi_client_creds
> (gensec_gssapi.c:316)
> ==15975== by 0xCAEFA50: gensec_gssapi_update_internal
> (gensec_gssapi.c:463)
> ==15975== by 0xCAF17DD: gensec_gssapi_update_send
> (gensec_gssapi.c:1053)
> ==15975== by 0xCAEB200: gensec_update_ev (gensec.c:353)
> ==15975== Block was alloc'd at
> ==15975== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
> ==15975== by 0x6956D20: __talloc_with_prefix (talloc.c:698)
> ==15975== by 0x6956EB2: __talloc (talloc.c:739)
> ==15975== by 0x69572C8: _talloc_named_const (talloc.c:896)
> ==15975== by 0x695A69A: _talloc_zero (talloc.c:2341)
> ==15975== by 0xCAECFCC: gensec_start (gensec_start.c:580)
> ==15975== by 0xCAED36E: gensec_client_start (gensec_start.c:665)
> ==15975== by 0xA02A06F: dcerpc_bind_auth_send (dcerpc_auth.c:339)
> ==15975== by 0xA02D3C3: dcerpc_pipe_auth_send (dcerpc_util.c:698)
> ==15975== by 0xA0367EC: continue_pipe_connect (dcerpc_connect.c:976)
> ==15975== by 0xA0365CA: continue_pipe_connect_ncacn_ip_tcp
> (dcerpc_connect.c:908)
> ==15975== by 0xD6EF292: composite_done (composite.c:143)
> ==15975== by 0xA03523F: continue_pipe_open_ncacn_ip_tcp
> (dcerpc_connect.c:360)
> ==15975== by 0xD6EF292: composite_done (composite.c:143)
> ==15975== by 0xA02EEC7: continue_ip_open_socket (dcerpc_sock.c:273)
> ==15975== by 0xD6EF292: composite_done (composite.c:143)
> ==15975== by 0xA02E796: continue_socket_connect (dcerpc_sock.c:118)
> ==15975== by 0xD6EF292: composite_done (composite.c:143)
> ==15975== by 0xD6F1814: socket_connect_handler (connect.c:131)
> ==15975== by 0x7100061: epoll_event_loop (tevent_epoll.c:728)
> ==15975== by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
> ==15975== by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
> ==15975== by 0x70F61C5: _tevent_loop_once (tevent.c:721)
> ==15975== by 0xD6EF020: composite_wait (composite.c:58)
> ==15975== by 0xA036FE5: dcerpc_pipe_connect_recv
> (dcerpc_connect.c:1226)
> ==15975== by 0xA0370B5: dcerpc_pipe_connect (dcerpc_connect.c:1251)
> ==15975== by 0x9A027E1: py_dcerpc_interface_init_helper
> (pyrpc_util.c:217)
> ==15975== by 0x20C4EF4A: interface_drsuapi_new (py_drsuapi.c:47159)
> ==15975== by 0x1F5112: type_call.lto_priv.96 (typeobject.c:749)
> ==15975== by 0x1EF672: PyObject_Call (abstract.c:2547)
> ==15975== by 0x208E2E: do_call (ceval.c:4569)
> ==15975== by 0x208E2E: call_function (ceval.c:4374)
> ==15975== by 0x208E2E: PyEval_EvalFrameEx (ceval.c:2989)
> ==15975== by 0x208C1E: fast_function (ceval.c:4437)
> ==15975== by 0x208C1E: call_function (ceval.c:4372)
> ==15975== by 0x208C1E: PyEval_EvalFrameEx (ceval.c:2989)
> ==15975==
>
>
-------------- next part --------------
From 870c5d3441e3d7bb77f4286d9ba3b32ba693c094 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 24 May 2017 06:11:17 +0200
Subject: [PATCH 1/4] s4:auth: use talloc_reparent() in
samba_server_gensec_krb5_start()
This matches logic of samba_server_gensec_start() and avoids warnings like this:
WARNING: talloc_steal with references at ../source4/auth/samba_server_gensec.c:150
reference at ../auth/gensec/gensec_start.c:586
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/auth/samba_server_gensec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/auth/samba_server_gensec.c b/source4/auth/samba_server_gensec.c
index ee3396a..f2b0551 100644
--- a/source4/auth/samba_server_gensec.c
+++ b/source4/auth/samba_server_gensec.c
@@ -147,6 +147,6 @@ NTSTATUS samba_server_gensec_krb5_start(TALLOC_CTX *mem_ctx,
return status;
}
- talloc_steal(*gensec_context, settings);
+ talloc_reparent(mem_ctx, *gensec_context, settings);
return NT_STATUS_OK;
}
--
1.9.1
From 09b52f9503193da352e2923059cf17f225db6692 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 29 May 2017 09:32:12 +0200
Subject: [PATCH 2/4] Revert "s4:librpc: simplify
dcerpc_connect_timeout_handler() logic"
This reverts commit 2c3e99d1697b83f7dd498596a274fe2e8e96116d.
As the source4 backends for kerberos still use nested event loops,
we need to restore this for now.
We should reapply this once all backends are fully async.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/librpc/rpc/dcerpc.h | 8 ++++++++
source4/librpc/rpc/dcerpc_connect.c | 10 +++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/source4/librpc/rpc/dcerpc.h b/source4/librpc/rpc/dcerpc.h
index eaa3c62..3223948 100644
--- a/source4/librpc/rpc/dcerpc.h
+++ b/source4/librpc/rpc/dcerpc.h
@@ -136,6 +136,14 @@ struct dcerpc_pipe {
/** timeout for individual rpc requests, in seconds */
uint32_t request_timeout;
+ /*
+ * Set for the timeout in dcerpc_pipe_connect_b_send(), to
+ * allow the timeout not to destory the stack during a nested
+ * event loop caused by gensec_update()
+ */
+ bool inhibit_timeout_processing;
+ bool timed_out;
+
bool verified_pcontext;
};
diff --git a/source4/librpc/rpc/dcerpc_connect.c b/source4/librpc/rpc/dcerpc_connect.c
index 723e657..8ed1257 100644
--- a/source4/librpc/rpc/dcerpc_connect.c
+++ b/source4/librpc/rpc/dcerpc_connect.c
@@ -1004,7 +1004,12 @@ static void dcerpc_connect_timeout_handler(struct tevent_context *ev, struct tev
{
struct composite_context *c = talloc_get_type_abort(private_data,
struct composite_context);
- composite_error(c, NT_STATUS_IO_TIMEOUT);
+ struct pipe_connect_state *s = talloc_get_type_abort(c->private_data, struct pipe_connect_state);
+ if (!s->pipe->inhibit_timeout_processing) {
+ composite_error(c, NT_STATUS_IO_TIMEOUT);
+ } else {
+ s->pipe->timed_out = true;
+ }
}
/*
@@ -1048,6 +1053,9 @@ _PUBLIC_ struct composite_context* dcerpc_pipe_connect_b_send(TALLOC_CTX *parent
s->credentials = credentials;
s->lp_ctx = lp_ctx;
+ s->pipe->timed_out = false;
+ s->pipe->inhibit_timeout_processing = false;
+
tevent_add_timer(c->event_ctx, c,
timeval_current_ofs(DCERPC_REQUEST_TIMEOUT, 0),
dcerpc_connect_timeout_handler, c);
--
1.9.1
From e38e8afec67186fc483ffe37e8d06f16a7e66629 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 29 May 2017 09:37:09 +0200
Subject: [PATCH 3/4] s4:librpc: restore inhibit_timeout_processing = true
during gensec_update_send/recv()
As not all gensec backends are fully async yet, we need the
inhibit_timeout_processing workarround in order to protect
against nested event loops.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/librpc/rpc/dcerpc_auth.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/source4/librpc/rpc/dcerpc_auth.c b/source4/librpc/rpc/dcerpc_auth.c
index a2d1418..ea222c2 100644
--- a/source4/librpc/rpc/dcerpc_auth.c
+++ b/source4/librpc/rpc/dcerpc_auth.c
@@ -187,6 +187,9 @@ static void bind_auth_next_step(struct composite_context *c)
* it doesn't like that either
*/
+ state->pipe->inhibit_timeout_processing = true;
+ state->pipe->timed_out = false;
+
subreq = gensec_update_send(state,
state->pipe->conn->event_ctx,
sec->generic_state,
@@ -207,6 +210,8 @@ static void bind_auth_next_gensec_done(struct tevent_req *subreq)
struct dcecli_security *sec = &p->conn->security_state;
bool more_processing = false;
+ state->pipe->inhibit_timeout_processing = false;
+
c->status = gensec_update_recv(subreq, state,
&state->out_auth_info.credentials);
TALLOC_FREE(subreq);
@@ -434,6 +439,9 @@ struct composite_context *dcerpc_bind_auth_send(TALLOC_CTX *mem_ctx,
* it doesn't like that either
*/
+ state->pipe->inhibit_timeout_processing = true;
+ state->pipe->timed_out = false;
+
subreq = gensec_update_send(state,
p->conn->event_ctx,
sec->generic_state,
@@ -455,6 +463,8 @@ static void dcerpc_bind_auth_gensec_done(struct tevent_req *subreq)
struct dcerpc_pipe *p = state->pipe;
struct dcecli_security *sec = &p->conn->security_state;
+ state->pipe->inhibit_timeout_processing = false;
+
c->status = gensec_update_recv(subreq, state,
&state->out_auth_info.credentials);
TALLOC_FREE(subreq);
--
1.9.1
From b173212f504c7b85d648bdb25a0b34f6f1979065 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 26 Apr 2017 15:25:43 +0200
Subject: [PATCH 4/4] s4:lib/com: remove unused pycom binding
This is completely untested and from reading the code it doesn't really
do anything beside always returning None from the get_class_object() method.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/lib/com/pycom.c | 83 -------------------------------------------
source4/lib/com/wscript_build | 7 ----
2 files changed, 90 deletions(-)
delete mode 100644 source4/lib/com/pycom.c
diff --git a/source4/lib/com/pycom.c b/source4/lib/com/pycom.c
deleted file mode 100644
index b445812..0000000
--- a/source4/lib/com/pycom.c
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- Unix SMB/CIFS implementation.
- Python bindings for COM library.
- Copyright (C) Jelmer Vernooij <jelmer at samba.org> 2008
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-
- You should have received a copy of the GNU General Public License
- along with this program. If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include <Python.h>
-#include "includes.h"
-#include "lib/com/com.h"
-#include "librpc/ndr/libndr.h"
-#include "libcli/util/pyerrors.h"
-
-void initcom(void);
-
-static struct com_context *py_com_ctx = NULL; /* FIXME: evil global */
-
-static PyObject *py_get_class_object(PyObject *self, PyObject *args)
-{
- char *s_clsid, *s_iid;
- struct GUID clsid, iid;
- struct IUnknown *object;
- NTSTATUS status;
- WERROR error;
-
- if (!PyArg_ParseTuple(args, "ss", &s_clsid, &s_iid))
- return NULL;
-
- status = GUID_from_string(s_clsid, &clsid);
- if (!NT_STATUS_IS_OK(status)) {
- PyErr_FromNTSTATUS(status);
- return NULL;
- }
-
- status = GUID_from_string(s_iid, &iid);
- if (!NT_STATUS_IS_OK(status)) {
- PyErr_FromNTSTATUS(status);
- return NULL;
- }
-
- error = com_get_class_object(py_com_ctx, &clsid, &iid, &object);
- if (!W_ERROR_IS_OK(error)) {
- PyErr_FromWERROR(error);
- return NULL;
- }
-
- /* FIXME: Magic, integrate with stubs generated by pidl. */
-
- Py_RETURN_NONE;
-}
-
-static struct PyMethodDef com_methods[] = {
- { "get_class_object", (PyCFunction)py_get_class_object, METH_VARARGS, "S.get_class_object(clsid, iid) -> instance" },
- { NULL },
-};
-
-void initcom(void)
-{
- PyObject *m;
- WERROR error;
-
- error = com_init_ctx(&py_com_ctx, NULL);
- if (!W_ERROR_IS_OK(error)) {
- PyErr_FromWERROR(error);
- return;
- }
-
- m = Py_InitModule3("com", com_methods, "Simple COM implementation");
- if (m == NULL)
- return;
-}
diff --git a/source4/lib/com/wscript_build b/source4/lib/com/wscript_build
index 763de1f..b96f39f 100644
--- a/source4/lib/com/wscript_build
+++ b/source4/lib/com/wscript_build
@@ -26,10 +26,3 @@ bld.SAMBA_MODULE('com_simple',
init_function='com_simple_init'
)
-
-bld.SAMBA_PYTHON('pycom',
- source='pycom.c',
- deps='COM',
- realname='samba/com.so',
- )
-
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170529/7d01dfb8/signature.sig>
More information about the samba-technical
mailing list