Attachment 10085 Details for Bug 10701 – Patches for v4-1-test

[patch] Patches for v4-1-test

tmp41.diff (text/plain), 15.65 KB, created by Stefan Metzmacher on 2014-07-08 13:44:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2014-07-08 13:44:56 UTC

Size: 15.65 KB

patch

obsolete

>From c0e6fb81579126a8a7525a3ac377b9afad8bff4f Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 27 Feb 2014 09:29:36 +0100
>Subject: [PATCH 1/9] s4:dsdb/ldb_modules: avoid declaration after code
> warnings
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit de773f3785d8fedba605437fbd434a49b9d18b0e)
>---
> source4/dsdb/samdb/ldb_modules/dirsync.c     | 5 ++++-
> source4/dsdb/samdb/ldb_modules/operational.c | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
>index e769948..0d68b59 100644
>--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
>+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
>@@ -336,10 +336,13 @@ skip:
> 	 * if not we remove the attribute.
> 	 */
> 	for (i = msg->num_elements - 1; i >= 0; i--) {
>+		const char *ldapattrname;
>+
> 		el = &(msg->elements[i]);
>+		ldapattrname = el->name;
>+
> 		attr = dsdb_attribute_by_lDAPDisplayName(dsc->schema,
> 				el->name);
>-		const char *ldapattrname = el->name;
> 		keep = false;
> 
> 		if (attr->linkID & 1) {
>diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c
>index 9337faa..2ebefac 100644
>--- a/source4/dsdb/samdb/ldb_modules/operational.c
>+++ b/source4/dsdb/samdb/ldb_modules/operational.c
>@@ -131,7 +131,7 @@ static int construct_token_groups(struct ldb_module *module,
> 				  struct ldb_message *msg, enum ldb_scope scope,
> 				  struct ldb_request *parent)
> {
>-	struct ldb_context *ldb = ldb_module_get_ctx(module);;
>+	struct ldb_context *ldb = ldb_module_get_ctx(module);
> 	TALLOC_CTX *tmp_ctx = talloc_new(msg);
> 	unsigned int i;
> 	int ret;
>-- 
>1.9.1
>
>
>From dfa81abf98968072bfed92a7b34c6798fd579af2 Mon Sep 17 00:00:00 2001
>From: Christian Ambach <ambi@samba.org>
>Date: Tue, 10 Dec 2013 17:53:15 +0100
>Subject: [PATCH 2/9] s4:dsdb fix compiler warnings
>
>about potentially uninitialized variables
>
>Signed-off-by: Christian Ambach <ambi@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 9c2951a9ca8228c714a1c1c834392077d050b569)
>---
> source4/dsdb/samdb/ldb_modules/dirsync.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
>index 0d68b59..c93189e 100644
>--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
>+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
>@@ -70,7 +70,7 @@ static int dirsync_filter_entry(struct ldb_request *req,
> 					bool referral)
> {
> 	struct ldb_context *ldb;
>-	uint64_t val;
>+	uint64_t val = 0;
> 	enum ndr_err_code ndr_err;
> 	uint32_t n;
> 	int i;
>-- 
>1.9.1
>
>
>From 8360fe6fe9a34dda25e6848290212d9dcf3fa413 Mon Sep 17 00:00:00 2001
>From: Christian Ambach <ambi@samba.org>
>Date: Tue, 10 Dec 2013 17:52:29 +0100
>Subject: [PATCH 3/9] s4:dsdb fix compiler warnings
>
>about potentially uninitialized variables
>
>Signed-off-by: Christian Ambach <ambi@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit e5cb10f59122acc56a465c19885fe74a39985700)
>---
> source4/dsdb/samdb/ldb_modules/descriptor.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
>index ceac8db..cc0a9c2 100644
>--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
>+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
>@@ -501,7 +501,7 @@ static int descriptor_search_callback(struct ldb_request *req, struct ldb_reply
> 	struct ldb_val *sd_val = NULL;
> 	struct ldb_message_element *sd_el;
> 	DATA_BLOB *show_sd;
>-	int ret;
>+	int ret = LDB_SUCCESS;
> 
> 	ac = talloc_get_type(req->context, struct descriptor_context);
> 
>-- 
>1.9.1
>
>
>From 465084d15b893d5386c568621e5ad577efc40891 Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Tue, 12 Nov 2013 14:09:56 +0100
>Subject: [PATCH 4/9] s4-dsdb: Fix a use after free segfault.
>
>Signed-off-by: Andreas Schneider <asn@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>
>Autobuild-User(master): David Disseldorp <ddiss@samba.org>
>Autobuild-Date(master): Tue Nov 12 19:22:28 CET 2013 on sn-devel-104
>
>(cherry picked from commit 744abc882284bfde41b087bc06e13160b915f371)
>---
> source4/dsdb/samdb/ldb_modules/rootdse.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c
>index f905aa2..7e1d277 100644
>--- a/source4/dsdb/samdb/ldb_modules/rootdse.c
>+++ b/source4/dsdb/samdb/ldb_modules/rootdse.c
>@@ -1373,6 +1373,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq)
> 	int ret;
> 	struct ldb_request *req = fsmo->req;
> 	struct ldb_context *ldb = fsmo->ldb;
>+	struct ldb_module *module = fsmo->module;
> 
> 	status = dcerpc_drepl_takeFSMORole_recv(treq, fsmo, &werr);
> 	talloc_free(fsmo);
>@@ -1382,7 +1383,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq)
> 		 * Now that it is failed, start the transaction up
> 		 * again so the wrappers can close it without additional error
> 		 */
>-		ldb_next_start_trans(fsmo->module);
>+		ldb_next_start_trans(module);
> 		ldb_module_done(req, NULL, NULL, LDB_ERR_UNAVAILABLE);
> 		return;
> 	}
>@@ -1392,7 +1393,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq)
> 		 * Now that it is failed, start the transaction up
> 		 * again so the wrappers can close it without additional error
> 		 */
>-		ldb_next_start_trans(fsmo->module);
>+		ldb_next_start_trans(module);
> 		ldb_module_done(req, NULL, NULL, LDB_ERR_UNAVAILABLE);
> 		return;
> 	}
>@@ -1401,7 +1402,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq)
> 	 * Now that it is done, start the transaction up again so the
> 	 * wrappers can close it without error
> 	 */
>-	ret = ldb_next_start_trans(fsmo->module);
>+	ret = ldb_next_start_trans(module);
> 	ldb_module_done(req, NULL, NULL, ret);
> }
> 
>-- 
>1.9.1
>
>
>From 18ec41155182fb0300223615d557151826ff1f5d Mon Sep 17 00:00:00 2001
>From: Christian Ambach <ambi@samba.org>
>Date: Tue, 10 Dec 2013 17:52:29 +0100
>Subject: [PATCH 5/9] s4:dsdb fix compiler warnings
>
>about potentially uninitialized variables
>
>Signed-off-by: Christian Ambach <ambi@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 2bd15d1b830b177ea234aa29ff696379abbcd683)
>---
> source4/dsdb/samdb/ldb_modules/samldb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
>index a63a44e..3118751 100644
>--- a/source4/dsdb/samdb/ldb_modules/samldb.c
>+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
>@@ -2002,7 +2002,7 @@ static int samldb_service_principal_names_change(struct samldb_ctx *ac)
> 
> 	/* Create a temporary message for fetching the "sAMAccountName" */
> 	if (el2 != NULL) {
>-		char *tempstr, *tempstr2;
>+		char *tempstr, *tempstr2 = NULL;
> 		const char *acct_attrs[] = { "sAMAccountName", NULL };
> 
> 		msg = ldb_msg_new(ac->msg);
>-- 
>1.9.1
>
>
>From 5ba24597a894ec9873a517f95c10cc3a2e3edb04 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 27 Feb 2014 09:29:36 +0100
>Subject: [PATCH 6/9] s4:dsdb/ldb_modules: avoid invalid pointer type warnings
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit ee06cbce30ccdb057ffc5b3da2d7fb1010a13837)
>---
> source4/dsdb/samdb/ldb_modules/samldb.c        | 1 -
> source4/dsdb/samdb/ldb_modules/update_keytab.c | 8 ++++----
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
>index 3118751..b73e45e 100644
>--- a/source4/dsdb/samdb/ldb_modules/samldb.c
>+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
>@@ -2757,7 +2757,6 @@ static int check_rename_constraints(struct ldb_message *msg,
> static int samldb_rename_search_base_callback(struct ldb_request *req,
> 					       struct ldb_reply *ares)
> {
>-	struct ldb_request *rename_req;
> 	struct samldb_ctx *ac;
> 	int ret;
> 
>diff --git a/source4/dsdb/samdb/ldb_modules/update_keytab.c b/source4/dsdb/samdb/ldb_modules/update_keytab.c
>index bec4a83..3291a8e 100644
>--- a/source4/dsdb/samdb/ldb_modules/update_keytab.c
>+++ b/source4/dsdb/samdb/ldb_modules/update_keytab.c
>@@ -397,7 +397,7 @@ static int update_kt_prepare_commit(struct ldb_module *module)
> 		const char *realm;
> 		char *upper_realm;
> 		struct ldb_message_element *spn_el = ldb_msg_find_element(p->msg, "servicePrincipalName");
>-		char **SPNs = NULL;
>+		const char **SPNs = NULL;
> 		int num_SPNs = 0;
> 		int i;
> 
>@@ -411,13 +411,13 @@ static int update_kt_prepare_commit(struct ldb_module *module)
> 			}
> 
> 			num_SPNs = spn_el->num_values;
>-			SPNs = talloc_array(tmp_ctx, char *, num_SPNs);
>+			SPNs = talloc_array(tmp_ctx, const char *, num_SPNs);
> 			if (!SPNs) {
> 				ldb_oom(ldb);
> 				goto fail;
> 			}
> 			for (i = 0; i < num_SPNs; i++) {
>-				SPNs[i] = talloc_asprintf(tmp_ctx, "%*.*s@%s",
>+				SPNs[i] = talloc_asprintf(SPNs, "%*.*s@%s",
> 							  (int)spn_el->values[i].length,
> 							  (int)spn_el->values[i].length,
> 							  (const char *)spn_el->values[i].data,
>@@ -432,7 +432,7 @@ static int update_kt_prepare_commit(struct ldb_module *module)
> 		krb5_ret = smb_krb5_update_keytab(tmp_ctx, smb_krb5_context->krb5_context,
> 						  keytab_name_from_msg(tmp_ctx, ldb, p->msg),
> 						  ldb_msg_find_attr_as_string(p->msg, "samAccountName", NULL),
>-						  realm, (const char **)SPNs, num_SPNs,
>+						  realm, SPNs, num_SPNs,
> 						  ldb_msg_find_attr_as_string(p->msg, "saltPrincipal", NULL),
> 						  ldb_msg_find_attr_as_string(p->msg, "secret", NULL),
> 						  ldb_msg_find_attr_as_string(p->msg, "priorSecret", NULL),
>-- 
>1.9.1
>
>
>From f56d65396d99f6562e8e237add4cac94c0f3a0fd Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sun, 18 Aug 2013 19:37:56 +0000
>Subject: [PATCH 7/9] samdb: Fix CID 1034910 Dereference before null check
>
>strncmp("tdb://", sam_name, 6) dereferences sam_name. Check for
>NULL before that.
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 8c4e6f0cba164c91661a654e2ccc13c265a06953)
>---
> source4/dsdb/samdb/ldb_modules/schema_load.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
>index 3397669..75e065f 100644
>--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
>+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
>@@ -68,13 +68,13 @@ static int schema_metadata_open(struct ldb_module *module)
> 	}
> 
> 	sam_name = (const char *)ldb_get_opaque(ldb, "ldb_url");
>-	if (strncmp("tdb://", sam_name, 6) == 0) {
>-		sam_name += 6;
>-	}
> 	if (!sam_name) {
> 		talloc_free(tmp_ctx);
> 		return ldb_operr(ldb);
> 	}
>+	if (strncmp("tdb://", sam_name, 6) == 0) {
>+		sam_name += 6;
>+	}
> 	filename = talloc_asprintf(tmp_ctx, "%s.d/metadata.tdb", sam_name);
> 	if (!filename) {
> 		talloc_free(tmp_ctx);
>-- 
>1.9.1
>
>
>From 2844aefc0c2bf6a35a38b620220ca528293762b4 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sun, 18 Aug 2013 19:37:56 +0000
>Subject: [PATCH 8/9] samdb: Fix CID 1034910 Dereference before null check
>
>strncmp("tdb://", secrets_ldb, 6) dereferences secrets_ldb. Check for
>NULL before that.
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 35330aa2c8b255d74e94bc9dd742e621953c21f9)
>---
> source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c b/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c
>index e3d8485..284aa1b 100644
>--- a/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c
>+++ b/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c
>@@ -489,12 +489,12 @@ static int secrets_tdb_sync_init(struct ldb_module *module)
> 	ldb_module_set_private(module, data);
> 
> 	secrets_ldb = (const char *)ldb_get_opaque(ldb, "ldb_url");
>-	if (strncmp("tdb://", secrets_ldb, 6) == 0) {
>-		secrets_ldb += 6;
>-	}
> 	if (!secrets_ldb) {
> 		return ldb_operr(ldb);
> 	}
>+	if (strncmp("tdb://", secrets_ldb, 6) == 0) {
>+		secrets_ldb += 6;
>+	}
> 	private_dir = talloc_strdup(data, secrets_ldb);
> 	p = strrchr(private_dir, '/');
> 	if (p) {
>-- 
>1.9.1
>
>
>From eaa94eb818d4763848d8332559f121fea3fc682e Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 13 May 2014 17:47:03 +1200
>Subject: [PATCH 9/9] passdb: Do not routinely clear the global memory returned
> by get_global_sam_sid()
>
>This avoids use-after-free errors and tdb database churn.
>
>Andrew Bartlett
>
>Change-Id: If7ab2e24556d9dffc7ad22c0489d665dd75a0cab
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Kamen Mazdrashki <kamenim@samba.org>
>(cherry picked from commit cda32d4e47aa3efb040eb60f1a0332ea8dd58417)
>---
> source3/passdb/machine_account_secrets.c | 10 ++++---
> source3/passdb/pdb_samba_dsdb.c          | 46 +++++++++++++++++++++++---------
> 2 files changed, 41 insertions(+), 15 deletions(-)
>
>diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
>index 5758efe..4e35a72 100644
>--- a/source3/passdb/machine_account_secrets.c
>+++ b/source3/passdb/machine_account_secrets.c
>@@ -29,6 +29,7 @@
> #include "dbwrap/dbwrap.h"
> #include "../librpc/ndr/libndr.h"
> #include "util_tdb.h"
>+#include "libcli/security/security.h"
> 
> #undef DBGC_CLASS
> #define DBGC_CLASS DBGC_PASSDB
>@@ -106,9 +107,12 @@ bool secrets_store_domain_sid(const char *domain, const struct dom_sid  *sid)
> 
> 	ret = secrets_store(domain_sid_keystr(domain), sid, sizeof(struct dom_sid ));
> 
>-	/* Force a re-query, in case we modified our domain */
>-	if (ret)
>-		reset_global_sam_sid();
>+	/* Force a re-query, in the case where we modified our domain */
>+	if (ret) {
>+		if (dom_sid_equal(get_global_sam_sid(), sid) == false) {
>+			reset_global_sam_sid();
>+		}
>+	}
> 	return ret;
> }
> 
>diff --git a/source3/passdb/pdb_samba_dsdb.c b/source3/passdb/pdb_samba_dsdb.c
>index 3fc266c..cbeb332 100644
>--- a/source3/passdb/pdb_samba_dsdb.c
>+++ b/source3/passdb/pdb_samba_dsdb.c
>@@ -2193,6 +2193,10 @@ static void free_private_data(void **vp)
> static NTSTATUS pdb_samba_dsdb_init_secrets(struct pdb_methods *m)
> {
> 	struct pdb_domain_info *dom_info;
>+	struct dom_sid stored_sid;
>+	struct GUID stored_guid;
>+	bool sid_exists_and_matches = false;
>+	bool guid_exists_and_matches = false;
> 	bool ret;
> 
> 	dom_info = pdb_samba_dsdb_get_domain_info(m, m);
>@@ -2200,20 +2204,38 @@ static NTSTATUS pdb_samba_dsdb_init_secrets(struct pdb_methods *m)
> 		return NT_STATUS_UNSUCCESSFUL;
> 	}
> 
>-	secrets_clear_domain_protection(dom_info->name);
>-	ret = secrets_store_domain_sid(dom_info->name,
>-				       &dom_info->sid);
>-	if (!ret) {
>-		goto done;
>+	ret = secrets_fetch_domain_sid(dom_info->name, &stored_sid);
>+	if (ret) {
>+		if (dom_sid_equal(&stored_sid, &dom_info->sid)) {
>+			sid_exists_and_matches = true;
>+		}
> 	}
>-	ret = secrets_store_domain_guid(dom_info->name,
>-				        &dom_info->guid);
>-	if (!ret) {
>-		goto done;
>+
>+	if (sid_exists_and_matches == false) {
>+		secrets_clear_domain_protection(dom_info->name);
>+		ret = secrets_store_domain_sid(dom_info->name,
>+					       &dom_info->sid);
>+		ret &= secrets_mark_domain_protected(dom_info->name);
>+		if (!ret) {
>+			goto done;
>+		}
> 	}
>-	ret = secrets_mark_domain_protected(dom_info->name);
>-	if (!ret) {
>-		goto done;
>+
>+	ret = secrets_fetch_domain_guid(dom_info->name, &stored_guid);
>+	if (ret) {
>+		if (GUID_equal(&stored_guid, &dom_info->guid)) {
>+			guid_exists_and_matches = true;
>+		}
>+	}
>+
>+	if (guid_exists_and_matches == false) {
>+		secrets_clear_domain_protection(dom_info->name);
>+		ret = secrets_store_domain_guid(dom_info->name,
>+					       &dom_info->guid);
>+		ret &= secrets_mark_domain_protected(dom_info->name);
>+		if (!ret) {
>+			goto done;
>+		}
> 	}
> 
> done:
>-- 
>1.9.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 10701: 10085