From 2eccbe4c4e51b3c49a12a9d9f13b25005e973d12 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 21 Feb 2019 18:34:51 +0100 Subject: [PATCH 1/7] winbindd: make a copy of xid's in wb_xids2sids_send() This is in preparation of setting the result of the mapping in the top- level callback wb_xids2sids_done(), not in the per-idmap-domain callback wb_xids2sids_dom_done(). When caching the mapping we need the id-type from the backend, so we need a way to pass up that information from wb_xids2sids_dom_done() up to wb_xids2sids_done() The xids array copy gets passed from wb_xids2sids_send() to wb_xids2sids_dom_send(), so wb_xids2sids_dom_done() can then directly update the top-level copy. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13802 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit f5a8bc2f945be45cdade5f70d4f975bae8337f67) --- source3/winbindd/wb_xids2sids.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index fa4ba983720..f0c4ed850f4 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -452,9 +452,14 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, return NULL; } state->ev = ev; - state->xids = xids; state->num_xids = num_xids; + state->xids = talloc_array(state, struct unixid, num_xids); + if (tevent_req_nomem(state->xids, req)) { + return tevent_req_post(req, ev); + } + memcpy(state->xids, xids, num_xids * sizeof(struct unixid)); + state->sids = talloc_zero_array(state, struct dom_sid, num_xids); if (tevent_req_nomem(state->sids, req)) { return tevent_req_post(req, ev); -- 2.17.2 From d2846e44783947e6212a0bec74f5b69037e825dc Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 22 Feb 2019 16:29:07 +0100 Subject: [PATCH 2/7] winbindd: make xids a const argument to wb_xids2sids_send() The previous commit made an internal copy of xids, this commit makes it more obvious that we must not mess with the xids argument but treat it as an in-parameter and don't write to it. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13802 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 5d277ea7ea258676b9ea5081a451a5874af115f6) --- source3/winbindd/wb_xids2sids.c | 2 +- source3/winbindd/winbindd_proto.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index f0c4ed850f4..4b62fa28bc3 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -440,7 +440,7 @@ static void wb_xids2sids_init_dom_maps_done(struct tevent_req *subreq); struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, - struct unixid *xids, + const struct unixid *xids, uint32_t num_xids) { struct tevent_req *req, *subreq; diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h index be3626dc477..ae8f555ec45 100644 --- a/source3/winbindd/winbindd_proto.h +++ b/source3/winbindd/winbindd_proto.h @@ -914,7 +914,7 @@ NTSTATUS winbindd_sids_to_xids_recv(struct tevent_req *req, struct winbindd_response *response); struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, - struct unixid *xids, + const struct unixid *xids, uint32_t num_xids); NTSTATUS wb_xids2sids_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, struct dom_sid **sids); -- 2.17.2 From 9d68263dfcbb364c7801a8098ae9f51a40ff6f47 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 21 Feb 2019 18:39:46 +0100 Subject: [PATCH 3/7] winbindd: convert id to a pointer in wb_xids2sids_dom_done() No change in behaviour. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13802 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit f8bf4fc608639695651f75c52b31f95e796a5a26) --- source3/winbindd/wb_xids2sids.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index 4b62fa28bc3..300d8e04148 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -357,9 +357,9 @@ static void wb_xids2sids_dom_done(struct tevent_req *subreq) dom_sid_idx = 0; for (i=0; inum_all_xids; i++) { - struct unixid id = state->all_xids[i]; + struct unixid *id = &state->all_xids[i]; - if ((id.id < dom_map->low_id) || (id.id > dom_map->high_id)) { + if ((id->id < dom_map->low_id) || (id->id > dom_map->high_id)) { /* out of range */ continue; } -- 2.17.2 From 6a64880a8688565767441736490404d228766a8e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 21 Feb 2019 18:40:20 +0100 Subject: [PATCH 4/7] winbindd: update xid in wb_xids2sids_state->xids with what we got In preparation of priming the idmap cache in the top-level wb_xids2sids_done(), not in the per-idmap-domain callback wb_xids2sids_dom_done(). Bug: https://bugzilla.samba.org/show_bug.cgi?id=13802 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 7f23ef7b2cf7bd6e8dc087aa15137292b421a689) --- source3/winbindd/wb_xids2sids.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index 300d8e04148..afdacb56ae6 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -369,6 +369,7 @@ static void wb_xids2sids_dom_done(struct tevent_req *subreq) } sid_copy(&state->all_sids[i], &state->dom_sids[dom_sid_idx]); + *id = state->dom_xids[dom_sid_idx]; /* * Prime the cache after an xid2sid call. It's -- 2.17.2 From f29912ab0178d05403092ba8ce7a67e9e78b92f1 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 21 Feb 2019 16:52:21 +0100 Subject: [PATCH 5/7] winbindd: switch send-next/done order In preparation of adding more logic to the done step. No change in behaviour. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13802 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 8e9c2a1f6ceb06d695a6572701b96a3e3821ac42) --- source3/winbindd/wb_xids2sids.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index afdacb56ae6..c4ee0920d46 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -549,18 +549,22 @@ static void wb_xids2sids_done(struct tevent_req *subreq) state->dom_idx += 1; - if (state->dom_idx >= num_domains) { - tevent_req_done(req); + if (state->dom_idx < num_domains) { + subreq = wb_xids2sids_dom_send(state, + state->ev, + &dom_maps[state->dom_idx], + state->xids, + state->num_xids, + state->sids); + if (tevent_req_nomem(subreq, req)) { + return; + } + tevent_req_set_callback(subreq, wb_xids2sids_done, req); return; } - subreq = wb_xids2sids_dom_send( - state, state->ev, &dom_maps[state->dom_idx], - state->xids, state->num_xids, state->sids); - if (tevent_req_nomem(subreq, req)) { - return; - } - tevent_req_set_callback(subreq, wb_xids2sids_done, req); + tevent_req_done(req); + return; } NTSTATUS wb_xids2sids_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, -- 2.17.2 From a26ef29e7db77918b4ce03cc43b39224956e3d49 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 22 Feb 2019 11:00:00 +0100 Subject: [PATCH 6/7] winbindd: track whether a result from xid2sid was coming from the cache This is needed in preparation of moving the step to update the idmap cache from the per-idmap-domain callback wb_xids2sids_dom_done() to the top-level callback wb_xids2sids_done(). Currently the sequence of action is: * check cache, if not found: * ask backends * cache result from backend * return results Iow, if we got something from the cache, we don't write the cache. The next commit defers updating the cache to the top-level callback, so the sequence becomes * check cache, if not found: * ask backends * cache results * return results This has two problems: * it needlessly writes to the cache what we just got from it * it possibly overwrites the ID_TYPE_BOTH for a SID-to-xid mapping in the following case: - existing ID_TYPE_BOTH mapping in the cache, eg: IDMAP/SID2XID/S-1-5-21-2180672342-2513613279-2566592647-512 -> Value: 3000000:B - someone calls wb_xids2sids_send() with xid.id=3000000,xid.type=ID_TYPE_GID - cache lookup with idmap_cache_find_gid2sid() succeeds - when caching results we'd call idmap_cache_set_sid2unixid() with the callers xid.type=ID_TYPE_GID, so idmap_cache_set_sid2unixid() will overwrite the SID-to-xid mapping with ID_TYPE_GID Bug: https://bugzilla.samba.org/show_bug.cgi?id=13802 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 62f54229fced20102e11ad1da02faef45c2a7c2e) --- source3/winbindd/wb_xids2sids.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index c4ee0920d46..888bc22aa7c 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -432,6 +432,7 @@ struct wb_xids2sids_state { struct unixid *xids; size_t num_xids; struct dom_sid *sids; + bool *cached; size_t dom_idx; }; @@ -466,6 +467,11 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } + state->cached = talloc_zero_array(state, bool, num_xids); + if (tevent_req_nomem(state->cached, req)) { + return tevent_req_post(req, ev); + } + if (winbindd_use_idmap_cache()) { uint32_t i; @@ -488,6 +494,7 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, if (ok && !expired) { sid_copy(&state->sids[i], &sid); + state->cached[i] = true; } } } -- 2.17.2 From 23e59c1f7b3effa904e6908428e418ee4f0db578 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 21 Feb 2019 16:55:09 +0100 Subject: [PATCH 7/7] winbindd: set idmap cache entries as the last step in async wb_xids2sids Bug: https://bugzilla.samba.org/show_bug.cgi?id=13802 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Sat Feb 23 09:23:22 CET 2019 on sn-devel-144 (cherry picked from commit 9b9565c3e69b92c298c7168e516387bb249c9e36) --- source3/winbindd/wb_xids2sids.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index 888bc22aa7c..fdd98a3d9bf 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -371,17 +371,6 @@ static void wb_xids2sids_dom_done(struct tevent_req *subreq) sid_copy(&state->all_sids[i], &state->dom_sids[dom_sid_idx]); *id = state->dom_xids[dom_sid_idx]; - /* - * Prime the cache after an xid2sid call. It's - * important that we use state->dom_xids for the xid - * value, not state->all_xids: state->all_xids carries - * what we asked for, e.g. a - * ID_TYPE_UID. state->dom_xids holds something the - * idmap child possibly changed to ID_TYPE_BOTH. - */ - idmap_cache_set_sid2unixid( - &state->all_sids[i], &state->dom_xids[dom_sid_idx]); - dom_sid_idx += 1; } @@ -546,6 +535,7 @@ static void wb_xids2sids_done(struct tevent_req *subreq) struct wb_xids2sids_state *state = tevent_req_data( req, struct wb_xids2sids_state); size_t num_domains = talloc_array_length(dom_maps); + size_t i; NTSTATUS status; status = wb_xids2sids_dom_recv(subreq); @@ -570,6 +560,27 @@ static void wb_xids2sids_done(struct tevent_req *subreq) return; } + + for (i = 0; i < state->num_xids; i++) { + /* + * Prime the cache after an xid2sid call. It's important that we + * use the xid value returned from the backend for the xid value + * passed to idmap_cache_set_sid2unixid(), not the input to + * wb_xids2sids_send: the input carries what was asked for, + * e.g. a ID_TYPE_UID. The result from the backend something the + * idmap child possibly changed to ID_TYPE_BOTH. + * + * And of course If the value was from the cache don't update + * the cache. + */ + + if (state->cached[i]) { + continue; + } + + idmap_cache_set_sid2unixid(&state->sids[i], &state->xids[i]); + } + tevent_req_done(req); return; } -- 2.17.2