[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Sat Feb 23 08:24:02 UTC 2019


The branch, master has been updated
       via  9b9565c3e69 winbindd: set idmap cache entries as the last step in async wb_xids2sids
       via  62f54229fce winbindd: track whether a result from xid2sid was coming from the cache
       via  8e9c2a1f6ce winbindd: switch send-next/done order
       via  7f23ef7b2cf winbindd: update xid in wb_xids2sids_state->xids with what we got
       via  f8bf4fc6086 winbindd: convert id to a pointer in wb_xids2sids_dom_done()
       via  5d277ea7ea2 winbindd: make xids a const argument to wb_xids2sids_send()
       via  f5a8bc2f945 winbindd: make a copy of xid's in wb_xids2sids_send()
      from  0a1d1a57092 s3:winbindd: Remove unused arcfour.h from PAM handling

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 9b9565c3e69b92c298c7168e516387bb249c9e36
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Feb 21 16:55:09 2019 +0100

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Sat Feb 23 09:23:22 CET 2019 on sn-devel-144

commit 62f54229fced20102e11ad1da02faef45c2a7c2e
Author: Ralph Boehme <slow at samba.org>
Date:   Fri Feb 22 11:00:00 2019 +0100

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 8e9c2a1f6ceb06d695a6572701b96a3e3821ac42
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Feb 21 16:52:21 2019 +0100

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 7f23ef7b2cf7bd6e8dc087aa15137292b421a689
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Feb 21 18:40:20 2019 +0100

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit f8bf4fc608639695651f75c52b31f95e796a5a26
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Feb 21 18:39:46 2019 +0100

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 5d277ea7ea258676b9ea5081a451a5874af115f6
Author: Ralph Boehme <slow at samba.org>
Date:   Fri Feb 22 16:29:07 2019 +0100

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit f5a8bc2f945be45cdade5f70d4f975bae8337f67
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Feb 21 18:34:51 2019 +0100

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/winbindd/wb_xids2sids.c   | 74 +++++++++++++++++++++++++++------------
 source3/winbindd/winbindd_proto.h |  2 +-
 2 files changed, 52 insertions(+), 24 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c
index 310a645cdff..95dda89e40f 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;
 		}
@@ -369,17 +369,7 @@ static void wb_xids2sids_dom_done(struct tevent_req *subreq)
 		}
 
 		sid_copy(&state->all_sids[i], &state->dom_sids[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]);
+		*id = state->dom_xids[dom_sid_idx];
 
 		dom_sid_idx += 1;
 	}
@@ -431,6 +421,7 @@ struct wb_xids2sids_state {
 	struct unixid *xids;
 	size_t num_xids;
 	struct dom_sid *sids;
+	bool *cached;
 
 	size_t dom_idx;
 };
@@ -440,7 +431,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;
@@ -452,14 +443,24 @@ 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);
 	}
 
+	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;
 
@@ -487,6 +488,7 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx,
 					  dom_sid_str_buf(&sid, &buf));
 
 				sid_copy(&state->sids[i], &sid);
+				state->cached[i] = true;
 			}
 		}
 	}
@@ -538,6 +540,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);
@@ -548,18 +551,43 @@ 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;
+
+	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_set_callback(subreq, wb_xids2sids_done, req);
+
+	tevent_req_done(req);
+	return;
 }
 
 NTSTATUS wb_xids2sids_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index c524d2050df..2a829b0171a 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -919,7 +919,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);


-- 
Samba Shared Repository



More information about the samba-cvs mailing list