Attachment 12848 Details for Bug 12538 – patchset for 4.6

[patch] patchset for 4.6

winbind-v4-6.patch (text/plain), 13.80 KB, created by Andreas Schneider on 2017-01-25 07:16:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andreas Schneider

Created: 2017-01-25 07:16:41 UTC

Size: 13.80 KB

patch

obsolete

>From faf6b99248fc060fb3813761b1b2ea4b463bb4f9 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Tue, 10 Jan 2017 13:24:22 +0000
>Subject: [PATCH 1/9] winbind: Fix CID 1398533 Resource leak
>
>Not really a leak due to talloc, but this way it's clear
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> source3/winbindd/winbindd_msrpc.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c
>index 4b742c4c58b..0d0e4ca6506 100644
>--- a/source3/winbindd/winbindd_msrpc.c
>+++ b/source3/winbindd/winbindd_msrpc.c
>@@ -90,6 +90,7 @@ static NTSTATUS msrpc_query_user_list(struct winbindd_domain *domain,
> 	}
> 
> done:
>+	TALLOC_FREE(rids);
> 	TALLOC_FREE(tmp_ctx);
> 	return status;
> }
>-- 
>2.11.0
>
>
>From c544dd9a5b2525ff2e0d01d5671d45c370f520fd Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Tue, 10 Jan 2017 13:26:13 +0000
>Subject: [PATCH 2/9] winbind: Fix CID 1398533 Resource leak
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> source3/winbindd/winbindd_rpc.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
>index bb8af45c896..fcc636620a4 100644
>--- a/source3/winbindd/winbindd_rpc.c
>+++ b/source3/winbindd/winbindd_rpc.c
>@@ -77,6 +77,7 @@ NTSTATUS rpc_query_user_list(TALLOC_CTX *mem_ctx,
> 		}
> 		if (!NT_STATUS_IS_OK(result)) {
> 			if (!NT_STATUS_EQUAL(result, STATUS_MORE_ENTRIES)) {
>+				TALLOC_FREE(rids);
> 				return result;
> 			}
> 		}
>-- 
>2.11.0
>
>
>From 43c2c848c9af29065dd6f1386c621363e8dc2789 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Tue, 10 Jan 2017 13:24:22 +0000
>Subject: [PATCH 3/9] winbind: Fix CID 1398531 Resource leak
>
>Not really a leak due to talloc, but this way it's clear
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> source3/winbindd/winbindd_samr.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
>index dd674965f17..224f1058348 100644
>--- a/source3/winbindd/winbindd_samr.c
>+++ b/source3/winbindd/winbindd_samr.c
>@@ -208,6 +208,7 @@ done:
> 		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
> 	}
> 
>+	TALLOC_FREE(rids);
> 	TALLOC_FREE(tmp_ctx);
> 	return status;
> }
>-- 
>2.11.0
>
>
>From e9c117a89022b69a2f30337137964ab21fd3ac27 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Tue, 10 Jan 2017 13:28:41 +0000
>Subject: [PATCH 4/9] winbind: Fix CID 1398530 Resource leak
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> source3/winbindd/winbindd_ads.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
>index 83579686ff4..b14f21e3644 100644
>--- a/source3/winbindd/winbindd_ads.c
>+++ b/source3/winbindd/winbindd_ads.c
>@@ -356,8 +356,9 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain,
> 		}
> 
> 		if (!ads_pull_sid(ads, msg, "objectSid", &user_sid)) {
>-			DBG_INFO("No sid for %s !?\n",
>-				 ads_get_dn(ads, talloc_tos(), msg));
>+			char *dn = ads_get_dn(ads, talloc_tos(), msg);
>+			DBG_INFO("No sid for %s !?\n", dn);
>+			TALLOC_FREE(dn);
> 			continue;
> 		}
> 
>-- 
>2.11.0
>
>
>From 9065b1c3fd01af6573a3da0a57f70ebb4f535ba2 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Tue, 10 Jan 2017 13:29:38 +0000
>Subject: [PATCH 5/9] winbind: Fix CID 1398530 Resource leak
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Wed Jan 11 04:38:25 CET 2017 on sn-devel-144
>---
> source3/winbindd/winbindd_cache.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
>index d92c050ee70..4431cb52b64 100644
>--- a/source3/winbindd/winbindd_cache.c
>+++ b/source3/winbindd/winbindd_cache.c
>@@ -1457,6 +1457,7 @@ do_fetch_cache:
> 
> 	rids = talloc_array(mem_ctx, uint32_t, num_rids);
> 	if (rids == NULL) {
>+		centry_free(centry);
> 		return NT_STATUS_NO_MEMORY;
> 	}
> 
>-- 
>2.11.0
>
>
>From 816d307e660d20fabc8bfe290da1bbd6c4b63d78 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 11 Jan 2017 11:52:44 -0800
>Subject: [PATCH 6/9] winbind: Fix CID 1398534 Dereference before null check
>
>Make all query_user_list backends consistent.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>
>Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
>Autobuild-Date(master): Fri Jan 13 13:33:37 CET 2017 on sn-devel-144
>---
> source3/winbindd/winbindd_ads.c  | 8 ++++----
> source3/winbindd/winbindd_samr.c | 6 ++----
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
>diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
>index b14f21e3644..077c6ec7b7c 100644
>--- a/source3/winbindd/winbindd_ads.c
>+++ b/source3/winbindd/winbindd_ads.c
>@@ -293,14 +293,12 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain,
> 	ADS_STRUCT *ads = NULL;
> 	const char *attrs[] = { "sAMAccountType", "objectSid", NULL };
> 	int count;
>-	uint32_t *rids;
>+	uint32_t *rids = NULL;
> 	ADS_STATUS rc;
> 	LDAPMessage *res = NULL;
> 	LDAPMessage *msg = NULL;
> 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
> 
>-	*prids = NULL;
>-
> 	DEBUG(3,("ads: query_user_list\n"));
> 
> 	if ( !winbindd_can_contact_domain( domain ) ) {
>@@ -375,7 +373,9 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain,
> 	}
> 
> 	rids = talloc_realloc(mem_ctx, rids, uint32_t, count);
>-	*prids = rids;
>+	if (prids != NULL) {
>+		*prids = rids;
>+	}
> 
> 	status = NT_STATUS_OK;
> 
>diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
>index 224f1058348..1a73fc4fcc6 100644
>--- a/source3/winbindd/winbindd_samr.c
>+++ b/source3/winbindd/winbindd_samr.c
>@@ -169,15 +169,13 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
> {
> 	struct rpc_pipe_client *samr_pipe = NULL;
> 	struct policy_handle dom_pol = { 0 };
>-	uint32_t *rids;
>+	uint32_t *rids = NULL;
> 	TALLOC_CTX *tmp_ctx;
> 	NTSTATUS status, result;
> 	struct dcerpc_binding_handle *b = NULL;
> 
> 	DEBUG(3,("samr_query_user_list\n"));
> 
>-	*prids = NULL;
>-
> 	tmp_ctx = talloc_stackframe();
> 	if (tmp_ctx == NULL) {
> 		return NT_STATUS_NO_MEMORY;
>@@ -199,7 +197,7 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
> 		goto done;
> 	}
> 
>-	if (prids) {
>+	if (prids != NULL) {
> 		*prids = talloc_move(mem_ctx, &rids);
> 	}
> 
>-- 
>2.11.0
>
>
>From 0450fe73ae6acf399997571d3631744a283277de Mon Sep 17 00:00:00 2001
>From: Aurelien Aptel <aaptel@suse.com>
>Date: Tue, 17 Jan 2017 14:39:03 +0100
>Subject: [PATCH 7/9] s3/winbindd: fix invalid free
>
>coverity fix.
>
>TALLOC_FREE() might be called on uninitialized 'rids' at the end of the
>function in case of an early error. Initialize it to NULL to turn the
>TALLOC_FREE() to a noop in this case.
>
>Signed-off-by: Aurelien Aptel <aaptel@suse.com>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>Autobuild-Date(master): Wed Jan 18 17:19:39 CET 2017 on sn-devel-144
>---
> source3/winbindd/winbindd_msrpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c
>index 0d0e4ca6506..5ace4d1c0bd 100644
>--- a/source3/winbindd/winbindd_msrpc.c
>+++ b/source3/winbindd/winbindd_msrpc.c
>@@ -53,7 +53,7 @@ static NTSTATUS msrpc_query_user_list(struct winbindd_domain *domain,
> {
> 	struct rpc_pipe_client *samr_pipe = NULL;
> 	struct policy_handle dom_pol;
>-	uint32_t *rids;
>+	uint32_t *rids = NULL;
> 	TALLOC_CTX *tmp_ctx;
> 	NTSTATUS status;
> 
>-- 
>2.11.0
>
>
>From 41398725a6ee2f629c9a6908e6d73f64ac444c58 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Fri, 13 Jan 2017 07:33:24 +0100
>Subject: [PATCH 8/9] winbind: Fix a typo
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> source3/winbindd/wb_sids2xids.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c
>index 25260be3384..9bb8fa87cf8 100644
>--- a/source3/winbindd/wb_sids2xids.c
>+++ b/source3/winbindd/wb_sids2xids.c
>@@ -40,7 +40,7 @@ struct wb_sids2xids_state {
> 	/*
> 	 * Domain array to use for the idmap call. The output from
> 	 * lookupsids cannot be used directly since for migrated
>-	 * objects the returned domain SID can be different that the
>+	 * objects the returned domain SID can be different than the
> 	 * original one. The new domain SID cannot be combined with
> 	 * the RID from the previous domain.
> 	 *
>-- 
>2.11.0
>
>
>From c23f0eeaaab4e36ff7c9ee03319f6ff4699efe0a Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Wed, 18 Jan 2017 16:54:03 +0100
>Subject: [PATCH 9/9] winbind: Don't add duplicate IDs in wbinfo -r
>
>We look at the netsamlogon_cache entry twice: Once in queryuser and
>once in lookupusergroups_cached. This can add the group SID twice.
>
>Use add_sid_to_array_unique to avoid this.
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Tue Jan 24 02:36:19 CET 2017 on sn-devel-144
>---
> source3/winbindd/wb_gettoken.c | 81 ++++++++++++++++++++----------------------
> 1 file changed, 38 insertions(+), 43 deletions(-)
>
>diff --git a/source3/winbindd/wb_gettoken.c b/source3/winbindd/wb_gettoken.c
>index d8867c36b9d..07c7fc79a31 100644
>--- a/source3/winbindd/wb_gettoken.c
>+++ b/source3/winbindd/wb_gettoken.c
>@@ -27,14 +27,15 @@ struct wb_gettoken_state {
> 	struct tevent_context *ev;
> 	struct dom_sid usersid;
> 	bool expand_local_aliases;
>-	int num_sids;
>+	uint32_t num_sids;
> 	struct dom_sid *sids;
> };
> 
>-static bool wb_add_rids_to_sids(TALLOC_CTX *mem_ctx,
>-				int *pnum_sids, struct dom_sid **psids,
>-				const struct dom_sid *domain_sid,
>-				int num_rids, uint32_t *rids);
>+static NTSTATUS wb_add_rids_to_sids(TALLOC_CTX *mem_ctx,
>+				    uint32_t *pnum_sids,
>+				    struct dom_sid **psids,
>+				    const struct dom_sid *domain_sid,
>+				    int num_rids, uint32_t *rids);
> 
> static void wb_gettoken_gotuser(struct tevent_req *subreq);
> static void wb_gettoken_gotlocalgroups(struct tevent_req *subreq);
>@@ -70,10 +71,9 @@ static void wb_gettoken_gotuser(struct tevent_req *subreq)
> 		subreq, struct tevent_req);
> 	struct wb_gettoken_state *state = tevent_req_data(
> 		req, struct wb_gettoken_state);
>-        struct dom_sid *sids;
> 	struct winbindd_domain *domain;
> 	struct wbint_userinfo *info;
>-	uint32_t num_groups;
>+	uint32_t i, num_groups;
> 	struct dom_sid *groups;
> 	NTSTATUS status;
> 
>@@ -83,11 +83,10 @@ static void wb_gettoken_gotuser(struct tevent_req *subreq)
> 		return;
> 	}
> 
>-	sids = talloc_array(state, struct dom_sid, 2);
>-	if (tevent_req_nomem(sids, req)) {
>+	state->sids = talloc_array(state, struct dom_sid, 2);
>+	if (tevent_req_nomem(state->sids, req)) {
> 		return;
> 	}
>-	state->sids = sids;
> 	state->num_sids = 2;
> 
> 	sid_copy(&state->sids[0], &info->user_sid);
>@@ -102,21 +101,14 @@ static void wb_gettoken_gotuser(struct tevent_req *subreq)
> 		return;
> 	}
> 
>-	if (num_groups + state->num_sids < num_groups) {
>-		tevent_req_nterror(req, NT_STATUS_INTEGER_OVERFLOW);
>-		return;
>-	}
>+	for (i=0; i<num_groups; i++) {
>+		status = add_sid_to_array_unique(
>+			state, &groups[i], &state->sids, &state->num_sids);
> 
>-	sids = talloc_realloc(state, state->sids, struct dom_sid,
>-			      state->num_sids+num_groups);
>-	if (tevent_req_nomem(sids, req)) {
>-		return;
>+		if (tevent_req_nterror(req, status)) {
>+			return;
>+		}
> 	}
>-	state->sids = sids;
>-
>-	memcpy(&state->sids[state->num_sids], groups,
>-	       num_groups * sizeof(struct dom_sid));
>-	state->num_sids += num_groups;
> 
> 	if (!state->expand_local_aliases) {
> 		tevent_req_done(req);
>@@ -156,9 +148,10 @@ static void wb_gettoken_gotlocalgroups(struct tevent_req *subreq)
> 	if (tevent_req_nterror(req, status)) {
> 		return;
> 	}
>-	if (!wb_add_rids_to_sids(state, &state->num_sids, &state->sids,
>-				 get_global_sam_sid(), num_rids, rids)) {
>-		tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
>+
>+	status = wb_add_rids_to_sids(state, &state->num_sids, &state->sids,
>+				     get_global_sam_sid(), num_rids, rids);
>+	if (tevent_req_nterror(req, status)) {
> 		return;
> 	}
> 	TALLOC_FREE(rids);
>@@ -196,9 +189,9 @@ static void wb_gettoken_gotbuiltins(struct tevent_req *subreq)
> 	if (tevent_req_nterror(req, status)) {
> 		return;
> 	}
>-	if (!wb_add_rids_to_sids(state, &state->num_sids, &state->sids,
>-				 &global_sid_Builtin, num_rids, rids)) {
>-		tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
>+	status = wb_add_rids_to_sids(state, &state->num_sids, &state->sids,
>+				     &global_sid_Builtin, num_rids, rids);
>+	if (tevent_req_nterror(req, status)) {
> 		return;
> 	}
> 	tevent_req_done(req);
>@@ -219,24 +212,26 @@ NTSTATUS wb_gettoken_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
> 	return NT_STATUS_OK;
> }
> 
>-static bool wb_add_rids_to_sids(TALLOC_CTX *mem_ctx,
>-				int *pnum_sids, struct dom_sid **psids,
>-				const struct dom_sid *domain_sid,
>-				int num_rids, uint32_t *rids)
>+static NTSTATUS wb_add_rids_to_sids(TALLOC_CTX *mem_ctx,
>+				    uint32_t *pnum_sids,
>+				    struct dom_sid **psids,
>+				    const struct dom_sid *domain_sid,
>+				    int num_rids, uint32_t *rids)
> {
>-	struct dom_sid *sids;
> 	int i;
> 
>-	sids = talloc_realloc(mem_ctx, *psids, struct dom_sid,
>-			      *pnum_sids + num_rids);
>-	if (sids == NULL) {
>-		return false;
>-	}
> 	for (i=0; i<num_rids; i++) {
>-		sid_compose(&sids[i+*pnum_sids], domain_sid, rids[i]);
>+		NTSTATUS status;
>+		struct dom_sid sid;
>+
>+		sid_compose(&sid, domain_sid, rids[i]);
>+
>+		status = add_sid_to_array_unique(
>+			mem_ctx, &sid, psids, pnum_sids);
>+		if (!NT_STATUS_IS_OK(status)) {
>+			return status;
>+		}
> 	}
> 
>-	*pnum_sids += num_rids;
>-	*psids = sids;
>-	return true;
>+	return NT_STATUS_OK;
> }
>-- 
>2.11.0
>

Flags:

jra: review+
metze: review+

Actions: View

Attachments on bug 12538: 12848