Attachment 6592 Details for Bug 8215 – Revert patch and valgrind fixes for v3-6-test

[patch] Revert patch and valgrind fixes for v3-6-test

tmp.diff (text/plain), 5.63 KB, created by Stefan Metzmacher on 2011-06-16 18:04:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2011-06-16 18:04:18 UTC

Size: 5.63 KB

patch

obsolete

>From 6c50e815c5a3b8d50fde09a1f662c6535a9ca0d7 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Jun 2011 18:40:04 +0200
>Subject: [PATCH 1/3] Revert "s3-winbind: Fix paranoia checks in winbindd_samr.c."
>
>This reverts commit 207a84d725b905c2b119d2ef0f4f4d4eb391140d.
>
>This is the wrong fix for the problem, see bug #8215.
>
>metze
>(cherry picked from commit 283f8a7fb5089a7126f07e26315fd06ab59997d8)
>---
> source3/winbindd/winbindd_samr.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
>index 98997b6..ee5ff08 100644
>--- a/source3/winbindd/winbindd_samr.c
>+++ b/source3/winbindd/winbindd_samr.c
>@@ -762,8 +762,8 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
> 	ZERO_STRUCT(lsa_policy);
> 
> 	/* Paranoia check */
>-	if (!sid_check_is_in_builtin(domain_sid) &&
>-	    !sid_check_is_in_our_domain(domain_sid) &&
>+	if (!sid_check_is_builtin(domain_sid) &&
>+	    !sid_check_is_domain(domain_sid) &&
> 	    !sid_check_is_unix_users(domain_sid) &&
> 	    !sid_check_is_unix_groups(domain_sid) &&
> 	    !sid_check_is_in_wellknown_domain(domain_sid)) {
>-- 
>1.7.4.1
>
>
>From 96b988cf6ea5c66cbb8c945f5ac103adb7d8da7b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Jun 2011 18:16:15 +0200
>Subject: [PATCH 2/3] s3:wb_lookupsids: don't ignore 'result' and check if we got useable values
>
>The wrong fix for bug #8215 discovered this bug, as it caused
>sam_rids_to_names() to always return NT_STATUS_NONE_MAPPED.
>
>metze
>(cherry picked from commit 85809ccbe3a79f307af1fdd227f33b899d8db1b4)
>---
> source3/winbindd/wb_lookupsids.c |   52 +++++++++++++++++++++++++++++++++++---
> 1 files changed, 48 insertions(+), 4 deletions(-)
>
>diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
>index 05601ad..9998b1b 100644
>--- a/source3/winbindd/wb_lookupsids.c
>+++ b/source3/winbindd/wb_lookupsids.c
>@@ -428,6 +428,7 @@ static void wb_lookupsids_done(struct tevent_req *subreq)
> 		req, struct wb_lookupsids_state);
> 	struct wb_lookupsids_domain *d;
> 	uint32_t i;
>+	bool fallback = false;
> 
> 	NTSTATUS status, result;
> 
>@@ -437,13 +438,31 @@ static void wb_lookupsids_done(struct tevent_req *subreq)
> 		return;
> 	}
> 
>+	d = &state->domains[state->domains_done];
>+
>+	if (NT_STATUS_IS_ERR(result)) {
>+		fallback = true;
>+	} else if (state->tmp_names.count != d->sids.num_sids) {
>+		fallback = true;
>+	}
>+
>+	if (fallback) {
>+		for (i=0; i < d->sids.num_sids; i++) {
>+			uint32_t res_sid_index = d->sid_indexes[i];
>+
>+			state->single_sids[state->num_single_sids] =
>+				res_sid_index;
>+			state->num_single_sids += 1;
>+		}
>+		state->domains_done += 1;
>+		wb_lookupsids_next(req, state);
>+		return;
>+	}
>+
> 	/*
>-	 * Ignore "result" here. We depend on the individual states in
>-	 * the translated names.
>+	 * Look at the individual states in the translated names.
> 	 */
> 
>-	d = &state->domains[state->domains_done];
>-
> 	for (i=0; i<state->tmp_names.count; i++) {
> 
> 		uint32_t res_sid_index = d->sid_indexes[i];
>@@ -544,6 +563,7 @@ static void wb_lookupsids_lookuprids_done(struct tevent_req *subreq)
> 	NTSTATUS status, result;
> 	struct wb_lookupsids_domain *d;
> 	uint32_t i;
>+	bool fallback = false;
> 
> 	status = dcerpc_wbint_LookupRids_recv(subreq, state, &result);
> 	TALLOC_FREE(subreq);
>@@ -552,6 +572,30 @@ static void wb_lookupsids_lookuprids_done(struct tevent_req *subreq)
> 	}
> 
> 	d = &state->domains[state->domains_done];
>+
>+	if (NT_STATUS_IS_ERR(result)) {
>+		fallback = true;
>+	} else if (state->rid_names.num_principals != d->sids.num_sids) {
>+		fallback = true;
>+	}
>+
>+	if (fallback) {
>+		for (i=0; i < d->sids.num_sids; i++) {
>+			uint32_t res_sid_index = d->sid_indexes[i];
>+
>+			state->single_sids[state->num_single_sids] =
>+				res_sid_index;
>+			state->num_single_sids += 1;
>+		}
>+		state->domains_done += 1;
>+		wb_lookupsids_next(req, state);
>+		return;
>+	}
>+
>+	/*
>+	 * Look at the individual states in the translated names.
>+	 */
>+
> 	sid_copy(&src_domain_sid, get_global_sam_sid());
> 	src_domain.name.string = get_global_sam_name();
> 	src_domain.sid = &src_domain_sid;
>-- 
>1.7.4.1
>
>
>From 39e9995ae9ac5c608603d1219093577757fc2469 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Jun 2011 18:25:15 +0200
>Subject: [PATCH 3/3] s3:wb_lookupsids: add some paranoia checks to wb_lookupsids_recv()
>
>This hopefully catches future bugs.
>
>metze
>
>Autobuild-User: Stefan Metzmacher <metze@samba.org>
>Autobuild-Date: Thu Jun 16 19:50:16 CEST 2011 on sn-devel-104
>(cherry picked from commit 5961852d9c0e5cf64cea988586d610af9d63d487)
>---
> source3/winbindd/wb_lookupsids.c |   18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
>diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
>index 9998b1b..bf2ddb3 100644
>--- a/source3/winbindd/wb_lookupsids.c
>+++ b/source3/winbindd/wb_lookupsids.c
>@@ -639,6 +639,24 @@ NTSTATUS wb_lookupsids_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
> 	if (tevent_req_is_nterror(req, &status)) {
> 		return status;
> 	}
>+
>+	/*
>+	 * The returned names need to match the given sids,
>+	 * if not we have a bug in the code!
>+	 *
>+	 */
>+	SMB_ASSERT(state->res_names->count == state->num_sids);
>+
>+	/*
>+	 * Not strictly needed, but it might make debugging in the callers
>+	 * easier in future, if the talloc_array_length() returns the
>+	 * expected result...
>+	 */
>+	state->res_domains->domains = talloc_realloc(state->res_domains,
>+						     state->res_domains->domains,
>+						     struct lsa_DomainInfo,
>+						     state->res_domains->count);
>+
> 	*domains = talloc_move(mem_ctx, &state->res_domains);
> 	*names = talloc_move(mem_ctx, &state->res_names);
> 	return NT_STATUS_OK;
>-- 
>1.7.4.1
>

Actions: View

Attachments on bug 8215: 6551 | 6592