From 1a1eec0f9252b25d517a77a9e38449277c884afe Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 27 Dec 2018 12:41:25 +0100 Subject: [PATCH 1/5] Revert "s4:messaging: make sure only imessaging_client_init() can be used with a wrapper tevent_context wrapper" This reverts commit e186d6a06b1b300256a2cb4138f0532d518d0597. See the discussion in https://lists.samba.org/archive/samba-technical/2018-December/131731.html for the reasoning behind this revert. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14033 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 0bd10a48e4c08d1eb3a20e79d952b3c0f12be46a) --- source4/lib/messaging/messaging.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c index 4a71b2b76bb..890d0dc57c1 100644 --- a/source4/lib/messaging/messaging.c +++ b/source4/lib/messaging/messaging.c @@ -381,7 +381,7 @@ NTSTATUS imessaging_reinit_all(void) /* create the listening socket and setup the dispatcher */ -static struct imessaging_context *imessaging_init_internal(TALLOC_CTX *mem_ctx, +struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, struct loadparm_context *lp_ctx, struct server_id server_id, struct tevent_context *ev) @@ -645,30 +645,6 @@ static void imessaging_dgm_recv(struct tevent_context *ev, } } -struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, - struct loadparm_context *lp_ctx, - struct server_id server_id, - struct tevent_context *ev) -{ - if (ev == NULL) { - return NULL; - } - - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - * - * The main/raw tevent context should - * have been registered first! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - errno = EINVAL; - return NULL; - } - - return imessaging_init_internal(mem_ctx, lp_ctx, server_id, ev); -} - /* A hack, for the short term until we get 'client only' messaging in place */ @@ -685,7 +661,7 @@ struct imessaging_context *imessaging_client_init(TALLOC_CTX *mem_ctx, /* This is because we are not in the s3 serverid database */ id.unique_id = SERVERID_UNIQUE_ID_NOT_TO_VERIFY; - return imessaging_init_internal(mem_ctx, lp_ctx, id, ev); + return imessaging_init(mem_ctx, lp_ctx, id, ev); } /* a list of registered irpc server functions -- 2.22.0 From b19abbae74155993356d9dcfd25c3a6ea5171f46 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 27 Dec 2018 12:45:15 +0100 Subject: [PATCH 2/5] Revert "s3:messages: allow messaging_filtered_read_send() to use wrapper tevent_context" This reverts commit 2b05f1098187e00166649c8ea7c63e6901b9d242. See the discussion in https://lists.samba.org/archive/samba-technical/2018-December/131731.html for the reasoning behind this revert. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14033 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit e2a5272ac6831b407a0c51bb8615252ec68be6a8) --- source3/lib/messages.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/source3/lib/messages.c b/source3/lib/messages.c index 90fffa2c872..319a55e3da3 100644 --- a/source3/lib/messages.c +++ b/source3/lib/messages.c @@ -206,7 +206,7 @@ static bool messaging_register_event_context(struct messaging_context *ctx, continue; } - if (tevent_context_same_loop(reg->ev, ev)) { + if (reg->ev == ev) { reg->refcount += 1; return true; } @@ -255,7 +255,7 @@ static bool messaging_deregister_event_context(struct messaging_context *ctx, continue; } - if (tevent_context_same_loop(reg->ev, ev)) { + if (reg->ev == ev) { reg->refcount -= 1; if (reg->refcount == 0) { @@ -1025,9 +1025,7 @@ struct tevent_req *messaging_filtered_read_send( state->filter = filter; state->private_data = private_data; - if (tevent_context_is_wrapper(ev) && - !tevent_context_same_loop(ev, msg_ctx->event_ctx)) - { + if (tevent_context_is_wrapper(ev)) { /* This is really a programmer error! */ DBG_ERR("Wrapper tevent context doesn't use main context.\n"); tevent_req_error(req, EINVAL); @@ -1036,11 +1034,7 @@ struct tevent_req *messaging_filtered_read_send( /* * We have to defer the callback here, as we might be called from - * within a different tevent_context than state->ev. - * - * This is important for two cases: - * 1. nested event contexts, used by blocking ctdb calls - * 2. possible impersonation using wrapper tevent contexts. + * within a different tevent_context than state->ev */ tevent_req_defer_callback(req, state->ev); @@ -1336,7 +1330,7 @@ static bool messaging_dispatch_waiters(struct messaging_context *msg_ctx, state = tevent_req_data( req, struct messaging_filtered_read_state); - if (tevent_context_same_loop(ev, state->ev) && + if ((ev == state->ev) && state->filter(rec, state->private_data)) { messaging_filtered_read_done(req, rec); return true; -- 2.22.0 From 8d266a06955d1182f7486ac072af9ef319f39828 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 27 Dec 2018 12:45:28 +0100 Subject: [PATCH 3/5] Revert "s3:messages: allow messaging_dgm_ref() to use wrapper tevent_context" This reverts commit 9dc332060cf5f249ea887dbc60ec7a39b6f91120. See the discussion in https://lists.samba.org/archive/samba-technical/2018-December/131731.html for the reasoning behind this revert. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14033 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 26107832cd9d200fb171ef1f991d7ef5478cac18) --- source3/lib/messages_dgm_ref.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/source3/lib/messages_dgm_ref.c b/source3/lib/messages_dgm_ref.c index 12ff21ca628..fd170961746 100644 --- a/source3/lib/messages_dgm_ref.c +++ b/source3/lib/messages_dgm_ref.c @@ -55,6 +55,18 @@ void *messaging_dgm_ref(TALLOC_CTX *mem_ctx, struct tevent_context *ev, { struct msg_dgm_ref *result, *tmp_refs; + if (tevent_context_is_wrapper(ev)) { + /* + * This is really a programmer error! + * + * The main/raw tevent context should + * have been registered first! + */ + DBG_ERR("Should not be used with a wrapper tevent context\n"); + *err = EINVAL; + return NULL; + } + result = talloc(mem_ctx, struct msg_dgm_ref); if (result == NULL) { *err = ENOMEM; @@ -75,18 +87,6 @@ void *messaging_dgm_ref(TALLOC_CTX *mem_ctx, struct tevent_context *ev, if (refs == NULL) { int ret; - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - * - * The main/raw tevent context should - * have been registered first! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - *err = EINVAL; - return NULL; - } - ret = messaging_dgm_init(ev, unique, socket_dir, lockfile_dir, msg_dgm_ref_recv, NULL); DBG_DEBUG("messaging_dgm_init returned %s\n", strerror(ret)); -- 2.22.0 From 5d0c271ec7e06bcb25340d487ff04a93a5264915 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 27 Dec 2018 12:45:42 +0100 Subject: [PATCH 4/5] Revert "s3:messages: allow messaging_{dgm,ctdb}_register_tevent_context() to use wrapper tevent_context" This reverts commit 660cf86639753edaa7a7a21a5b5ae207ae7d4260. See the discussion in https://lists.samba.org/archive/samba-technical/2018-December/131731.html for the reasoning behind this revert. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14033 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 1c3676f3aa9c1564eb140a24ced5ee72b859b87f) --- source3/lib/messages_ctdb.c | 38 +++++++++---------------------------- source3/lib/messages_dgm.c | 38 +++++++++---------------------------- 2 files changed, 18 insertions(+), 58 deletions(-) diff --git a/source3/lib/messages_ctdb.c b/source3/lib/messages_ctdb.c index 11fe72661cc..a1aeb37af19 100644 --- a/source3/lib/messages_ctdb.c +++ b/source3/lib/messages_ctdb.c @@ -209,6 +209,14 @@ struct messaging_ctdb_fde *messaging_ctdb_register_tevent_context( return NULL; } + if (tevent_context_is_wrapper(ev)) { + /* + * This is really a programmer error! + */ + DBG_ERR("Should not be used with a wrapper tevent context\n"); + return NULL; + } + fde = talloc(mem_ctx, struct messaging_ctdb_fde); if (fde == NULL) { return NULL; @@ -226,24 +234,7 @@ struct messaging_ctdb_fde *messaging_ctdb_register_tevent_context( */ continue; } - - /* - * We can only have one tevent_fd - * per low level tevent_context. - * - * This means any wrapper tevent_context - * needs to share the structure with - * the main tevent_context and/or - * any sibling wrapper tevent_context. - * - * This means we need to use tevent_context_same_loop() - * instead of just (fde_ev->ev == ev). - * - * Note: the tevent_context_is_wrapper() check below - * makes sure that fde_ev->ev is always a raw - * tevent context. - */ - if (tevent_context_same_loop(fde_ev->ev, ev)) { + if (fde_ev->ev == ev) { break; } } @@ -251,17 +242,6 @@ struct messaging_ctdb_fde *messaging_ctdb_register_tevent_context( if (fde_ev == NULL) { int sock = ctdbd_conn_get_fd(ctx->conn); - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - * - * The main/raw tevent context should - * have been registered first! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - return NULL; - } - fde_ev = talloc(fde, struct messaging_ctdb_fde_ev); if (fde_ev == NULL) { return NULL; diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c index 90d253d8e26..60ac9988db7 100644 --- a/source3/lib/messages_dgm.c +++ b/source3/lib/messages_dgm.c @@ -1695,6 +1695,14 @@ struct messaging_dgm_fde *messaging_dgm_register_tevent_context( return NULL; } + if (tevent_context_is_wrapper(ev)) { + /* + * This is really a programmer error! + */ + DBG_ERR("Should not be used with a wrapper tevent context\n"); + return NULL; + } + fde = talloc(mem_ctx, struct messaging_dgm_fde); if (fde == NULL) { return NULL; @@ -1712,40 +1720,12 @@ struct messaging_dgm_fde *messaging_dgm_register_tevent_context( */ continue; } - - /* - * We can only have one tevent_fd - * per low level tevent_context. - * - * This means any wrapper tevent_context - * needs to share the structure with - * the main tevent_context and/or - * any sibling wrapper tevent_context. - * - * This means we need to use tevent_context_same_loop() - * instead of just (fde_ev->ev == ev). - * - * Note: the tevent_context_is_wrapper() check below - * makes sure that fde_ev->ev is always a raw - * tevent context. - */ - if (tevent_context_same_loop(fde_ev->ev, ev)) { + if (fde_ev->ev == ev) { break; } } if (fde_ev == NULL) { - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - * - * The main/raw tevent context should - * have been registered first! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - return NULL; - } - fde_ev = talloc(fde, struct messaging_dgm_fde_ev); if (fde_ev == NULL) { return NULL; -- 2.22.0 From b9cc578bdac273d167e2158c9576343a09f9feff Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 27 Dec 2018 12:48:30 +0100 Subject: [PATCH 5/5] Revert "s3:messages: protect against usage of wrapper tevent_context objects for messaging" This reverts commit 7f2afc20e1b6397c364a98d1be006377c95e4665. See the discussion in https://lists.samba.org/archive/samba-technical/2018-December/131731.html for the reasoning behind this revert. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14033 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 2a62a98f5c7107f2f83c0bfc2892243d83e2c88a) --- source3/lib/messages.c | 23 ----------------------- source3/lib/messages_ctdb.c | 8 -------- source3/lib/messages_ctdb_ref.c | 12 ------------ source3/lib/messages_dgm.c | 14 -------------- source3/lib/messages_dgm_ref.c | 12 ------------ 5 files changed, 69 deletions(-) diff --git a/source3/lib/messages.c b/source3/lib/messages.c index 319a55e3da3..df7af2e50f1 100644 --- a/source3/lib/messages.c +++ b/source3/lib/messages.c @@ -365,11 +365,6 @@ static bool messaging_alert_event_contexts(struct messaging_context *ctx) * alternatively would be to track whether the * immediate has already been scheduled. For * now, avoid that complexity here. - * - * reg->ev and ctx->event_ctx can't - * be wrapper tevent_context pointers - * so we don't need to use - * tevent_context_same_loop(). */ if (reg->ev == ctx->event_ctx) { @@ -498,12 +493,6 @@ static NTSTATUS messaging_init_internal(TALLOC_CTX *mem_ctx, sec_init(); - if (tevent_context_is_wrapper(ev)) { - /* This is really a programmer error! */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - return NT_STATUS_INVALID_PARAMETER; - } - lck_path = lock_path("msg.lock"); if (lck_path == NULL) { return NT_STATUS_NO_MEMORY; @@ -1025,13 +1014,6 @@ struct tevent_req *messaging_filtered_read_send( state->filter = filter; state->private_data = private_data; - if (tevent_context_is_wrapper(ev)) { - /* This is really a programmer error! */ - DBG_ERR("Wrapper tevent context doesn't use main context.\n"); - tevent_req_error(req, EINVAL); - return tevent_req_post(req, ev); - } - /* * We have to defer the callback here, as we might be called from * within a different tevent_context than state->ev @@ -1352,11 +1334,6 @@ static void messaging_dispatch_rec(struct messaging_context *msg_ctx, bool consumed; size_t i; - /* - * ev and msg_ctx->event_ctx can't be wrapper tevent_context pointers - * so we don't need to use tevent_context_same_loop(). - */ - if (ev == msg_ctx->event_ctx) { consumed = messaging_dispatch_classic(msg_ctx, rec); if (consumed) { diff --git a/source3/lib/messages_ctdb.c b/source3/lib/messages_ctdb.c index a1aeb37af19..d3e2e3f8589 100644 --- a/source3/lib/messages_ctdb.c +++ b/source3/lib/messages_ctdb.c @@ -209,14 +209,6 @@ struct messaging_ctdb_fde *messaging_ctdb_register_tevent_context( return NULL; } - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - return NULL; - } - fde = talloc(mem_ctx, struct messaging_ctdb_fde); if (fde == NULL) { return NULL; diff --git a/source3/lib/messages_ctdb_ref.c b/source3/lib/messages_ctdb_ref.c index 47b4b758dac..3570ed8ae4c 100644 --- a/source3/lib/messages_ctdb_ref.c +++ b/source3/lib/messages_ctdb_ref.c @@ -52,18 +52,6 @@ void *messaging_ctdb_ref(TALLOC_CTX *mem_ctx, struct tevent_context *ev, { struct msg_ctdb_ref *result, *tmp_refs; - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - * - * The main/raw tevent context should - * have been registered first! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - *err = EINVAL; - return NULL; - } - result = talloc(mem_ctx, struct msg_ctdb_ref); if (result == NULL) { *err = ENOMEM; diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c index 60ac9988db7..72b330eadb1 100644 --- a/source3/lib/messages_dgm.c +++ b/source3/lib/messages_dgm.c @@ -993,12 +993,6 @@ int messaging_dgm_init(struct tevent_context *ev, return EEXIST; } - if (tevent_context_is_wrapper(ev)) { - /* This is really a programmer error! */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - return EINVAL; - } - ctx = talloc_zero(NULL, struct messaging_dgm_context); if (ctx == NULL) { goto fail_nomem; @@ -1695,14 +1689,6 @@ struct messaging_dgm_fde *messaging_dgm_register_tevent_context( return NULL; } - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - return NULL; - } - fde = talloc(mem_ctx, struct messaging_dgm_fde); if (fde == NULL) { return NULL; diff --git a/source3/lib/messages_dgm_ref.c b/source3/lib/messages_dgm_ref.c index fd170961746..470dfbeabc7 100644 --- a/source3/lib/messages_dgm_ref.c +++ b/source3/lib/messages_dgm_ref.c @@ -55,18 +55,6 @@ void *messaging_dgm_ref(TALLOC_CTX *mem_ctx, struct tevent_context *ev, { struct msg_dgm_ref *result, *tmp_refs; - if (tevent_context_is_wrapper(ev)) { - /* - * This is really a programmer error! - * - * The main/raw tevent context should - * have been registered first! - */ - DBG_ERR("Should not be used with a wrapper tevent context\n"); - *err = EINVAL; - return NULL; - } - result = talloc(mem_ctx, struct msg_dgm_ref); if (result == NULL) { *err = ENOMEM; -- 2.22.0