From: Garming Sam <garming@catalyst.net.nz>
Date: Thu, 2 Mar 2017 22:14:24 +0000 (+1300)
Subject: getncchanges: Prevent a small, but possible race condition in build_object
X-Git-Tag: tdb-1.3.13~488
X-Git-Url: http://git.samba.org/samba.git/?p=nivanova%2Fsamba-autobuild%2F.git;a=commitdiff_plain;h=2e02f3602d4d81497b141ba696a59088284f057e

getncchanges: Prevent a small, but possible race condition in build_object

Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---

diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
index 5bde00b4745..4de66470407 100644
--- a/source4/rpc_server/drsuapi/getncchanges.c
+++ b/source4/rpc_server/drsuapi/getncchanges.c
@@ -491,27 +491,6 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem
 		}
 		revealed_list_msg->dn = machine_dn;
 
-		ldb_err = dsdb_search_dn(sam_ctx, obj, &res, machine_dn, machine_attrs, 0);
-		if (ldb_err != LDB_SUCCESS || res->count != 1) {
-			return WERR_DS_DRA_INTERNAL_ERROR;
-		}
-
-		existing_revealed_list_msg = res->msgs[0];
-	}
-
-	werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n,
-					   highest_usn, rdn_sa, schema,
-					   extended_op == DRSUAPI_EXOP_REPL_SECRET,
-					   &revealed_list_msg,
-					   existing_revealed_list_msg,
-					   uptodateness_vector,
-					   partial_attribute_set, local_pas,
-					   attids);
-	if (!W_ERROR_IS_OK(werr)) {
-		return werr;
-	}
-
-	if (revealed_list_msg != NULL) {
 		ret = ldb_transaction_start(sam_ctx);
 		if (ret != LDB_SUCCESS) {
 			DEBUG(0,(__location__ ": Failed transaction start - %s\n",
@@ -519,20 +498,55 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem
 			return WERR_DS_DRA_INTERNAL_ERROR;
 		}
 
-		ret = ldb_modify(sam_ctx, revealed_list_msg);
-		if (ret != LDB_SUCCESS) {
-			DEBUG(0,(__location__ ": Failed to commit revealed links - %s\n",
-				 ldb_errstring(sam_ctx)));
+		ldb_err = dsdb_search_dn(sam_ctx, obj, &res, machine_dn, machine_attrs, 0);
+		if (ldb_err != LDB_SUCCESS || res->count != 1) {
 			ldb_transaction_cancel(sam_ctx);
 			return WERR_DS_DRA_INTERNAL_ERROR;
 		}
 
+		existing_revealed_list_msg = res->msgs[0];
+
+		werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n,
+						   highest_usn, rdn_sa, schema,
+						   true,
+						   &revealed_list_msg,
+						   existing_revealed_list_msg,
+						   uptodateness_vector,
+						   partial_attribute_set, local_pas,
+						   attids);
+		if (!W_ERROR_IS_OK(werr)) {
+			ldb_transaction_cancel(sam_ctx);
+			return werr;
+		}
+
+		if (revealed_list_msg != NULL) {
+			ret = ldb_modify(sam_ctx, revealed_list_msg);
+			if (ret != LDB_SUCCESS) {
+				DEBUG(0,(__location__ ": Failed to alter revealed links - %s\n",
+					 ldb_errstring(sam_ctx)));
+				ldb_transaction_cancel(sam_ctx);
+				return WERR_DS_DRA_INTERNAL_ERROR;
+			}
+		}
+
 		ret = ldb_transaction_commit(sam_ctx);
 		if (ret != LDB_SUCCESS) {
 			DEBUG(0,(__location__ ": Failed transaction commit - %s\n",
 				 ldb_errstring(sam_ctx)));
 			return WERR_DS_DRA_INTERNAL_ERROR;
 		}
+	} else {
+		werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n,
+						   highest_usn, rdn_sa, schema,
+						   false,
+						   &revealed_list_msg,
+						   existing_revealed_list_msg,
+						   uptodateness_vector,
+						   partial_attribute_set, local_pas,
+						   attids);
+		if (!W_ERROR_IS_OK(werr)) {
+			return werr;
+		}
 	}
 
 	/* ignore it if its an empty change. Note that renames always