[PATCH] winbind: Fix idmap initialization

Volker Lendecke Volker.Lendecke at SerNet.DE
Thu Jan 22 05:19:43 MST 2015


Hi!

Review&push appreciated!

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 27f5b6f4dd567368b447e19a6dd8ba8370970b9d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 22 Jan 2015 12:08:52 +0000
Subject: [PATCH] winbind: Fix idmap initialization

The fix is in the sscanf line: %u in the sscanf format mandates the use of
a pointer to an "unsigned". idmap_domain->[low|high]_id are uint32_t. On
little endian 64-bit this might at least put the correct values into
low_id and high_id, but might overwrite the read_only bit set earlier,
depending on structure alignment and packing. On big endian 64-bit,
this will just fail.

Automatic conversion to uint32_t will happen only at assignment, not
when you take a pointer of such a thing.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/winbindd/idmap.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index a8beab7..841f710 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -172,6 +172,7 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
 	NTSTATUS status;
 	char *config_option = NULL;
 	const char *range;
+	unsigned low_id, high_id;
 
 	result = talloc_zero(mem_ctx, struct idmap_domain);
 	if (result == NULL) {
@@ -230,23 +231,24 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
 				  result->name));
 			goto fail;
 		}
-	} else if (sscanf(range, "%u - %u", &result->low_id,
-			  &result->high_id) != 2)
+	} else if (sscanf(range, "%u - %u", &low_id, &high_id) != 2)
 	{
 		DEBUG(1, ("invalid range '%s' specified for domain "
 			  "'%s'\n", range, result->name));
 		if (check_range) {
 			goto fail;
 		}
-	} else if (result->low_id > result->high_id) {
-		DEBUG(1, ("Error: invalid idmap range detected: %lu - %lu\n",
-			  (unsigned long)result->low_id,
-			  (unsigned long)result->high_id));
+	} else if (low_id > high_id) {
+		DEBUG(1, ("Error: invalid idmap range detected: %u - %u\n",
+			  low_id, high_id));
 		if (check_range) {
 			goto fail;
 		}
 	}
 
+	result->low_id = low_id;
+	result->high_id = high_id;
+
 	status = result->methods->init(result);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("idmap initialization returned %s\n",
-- 
1.9.1



More information about the samba-technical mailing list