Attachment 14868 Details for Bug 13802 – Patch for 4.8, 4.9 and 4.10 cherry-picked from master

[patch] Patch for 4.8, 4.9 and 4.10 cherry-picked from master

bug13802-v48,v49,v410.patch (text/plain), 12.61 KB, created by Ralph Böhme on 2019-02-25 08:40:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2019-02-25 08:40:33 UTC

Size: 12.61 KB

patch

obsolete

>From 2eccbe4c4e51b3c49a12a9d9f13b25005e973d12 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>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 <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 <slow@samba.org>
>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 <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 <slow@samba.org>
>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 <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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; i<state->num_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 <slow@samba.org>
>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 <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 <slow@samba.org>
>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 <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 <slow@samba.org>
>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 <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 <slow@samba.org>
>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 <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>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
>

Flags:

vl: review+
bjacke: review+

Actions: View

Attachments on bug 13802: 14868