[PATCHES] s3-winbind-idmap initialization fixes / improvements
Michael Adam
obnox at samba.org
Wed Jul 23 03:53:49 MDT 2014
Hi,
attached find a few patches that improve idmap_init_domain().
The first one is a fix for bug #10737 which I just created:
It prevents a very misleading log message at level 1 in
the case where idmap_init_domain() is called from
idmap_passdb_domain().
review/push/comments appreciated.
Thanks - Michael
-------------- next part --------------
From 72ed6013a8ae6ca493c406daa66773299144bc7f Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 20 Jul 2014 11:49:37 +0200
Subject: [PATCH 1/5] s3:idmap: don't log missing range config if range
checking not requested
idmap_init_domain() is called with check_range == false from
idmap_passdb_domain(). In this case, we usually don't have an
idmap range at all, and we don't want to level 1 debug
messages complaining about the fact are irritating at least.
This patch removes the debug in the case of check_range == false.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=10737
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/winbindd/idmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index 674f54c..33b397c 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -198,9 +198,9 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
range = lp_parm_const_string(-1, config_option, "range", NULL);
if (range == NULL) {
- DEBUG(1, ("idmap range not specified for domain %s\n",
- result->name));
if (check_range) {
+ DEBUG(1, ("idmap range not specified for domain %s\n",
+ result->name));
goto fail;
}
} else if (sscanf(range, "%u - %u", &result->low_id,
--
1.9.1
From 7c6ad4f723e6e23b6125dce31b091159ce757136 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 20 Jul 2014 11:53:32 +0200
Subject: [PATCH 2/5] s3:idmap: in idmap_init_domain() load methods before
loading further config
Check whether the requested backend exists at all, before going
further into the config parsing.
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/winbindd/idmap.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index 33b397c..c76c350 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -186,6 +186,29 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
}
/*
+ * Check whether the requested backend module exists and
+ * load the methods.
+ */
+
+ result->methods = get_methods(modulename);
+ if (result->methods == NULL) {
+ DEBUG(3, ("idmap backend %s not found\n", modulename));
+
+ status = smb_probe_module("idmap", modulename);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(3, ("Could not probe idmap module %s\n",
+ modulename));
+ goto fail;
+ }
+
+ result->methods = get_methods(modulename);
+ }
+ if (result->methods == NULL) {
+ DEBUG(1, ("idmap backend %s not found\n", modulename));
+ goto fail;
+ }
+
+ /*
* load ranges and read only information from the config
*/
@@ -226,24 +249,6 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
}
}
- result->methods = get_methods(modulename);
- if (result->methods == NULL) {
- DEBUG(3, ("idmap backend %s not found\n", modulename));
-
- status = smb_probe_module("idmap", modulename);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(3, ("Could not probe idmap module %s\n",
- modulename));
- goto fail;
- }
-
- result->methods = get_methods(modulename);
- }
- if (result->methods == NULL) {
- DEBUG(1, ("idmap backend %s not found\n", modulename));
- goto fail;
- }
-
status = result->methods->init(result);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1, ("idmap initialization returned %s\n",
--
1.9.1
From f4e4298cf147b3f959b330feace7b61497d67099 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 20 Jul 2014 12:11:16 +0200
Subject: [PATCH 3/5] s3:idmap: move loading of idmap options together before
range checking in idmap_init_domain()
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/winbindd/idmap.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index c76c350..e62f477 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -219,7 +219,11 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
goto fail;
}
+ result->read_only = lp_parm_bool(-1, config_option, "read only", false);
range = lp_parm_const_string(-1, config_option, "range", NULL);
+
+ talloc_free(config_option);
+
if (range == NULL) {
if (check_range) {
DEBUG(1, ("idmap range not specified for domain %s\n",
@@ -236,10 +240,6 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
}
}
- result->read_only = lp_parm_bool(-1, config_option, "read only", false);
-
- talloc_free(config_option);
-
if (result->low_id > result->high_id) {
DEBUG(1, ("Error: invalid idmap range detected: %lu - %lu\n",
(unsigned long)result->low_id,
--
1.9.1
From 790eb8e1e2eb6f15a8e613606aebf56dc3bf982e Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 20 Jul 2014 12:12:16 +0200
Subject: [PATCH 4/5] s3:idmap: only check the range values if a range setting
has been found.
Otherwise, the check is superfluous since high and low values are
initialized to 0.
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/winbindd/idmap.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index e62f477..dc972fe 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -238,9 +238,7 @@ static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
if (check_range) {
goto fail;
}
- }
-
- if (result->low_id > result->high_id) {
+ } 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));
--
1.9.1
From 475ef2f3035d190d321b18bc3aa04271e0f68912 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 23 Jul 2014 11:42:57 +0200
Subject: [PATCH 5/5] s3:idmap: fix talloc hierarchy in idmap_passdb_domain()
(don't init to NULL context - we got one handed in...)
Signed-off-by: Michael Adam <obnox at samba.org>
---
source3/winbindd/idmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index dc972fe..a8beab7 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -350,7 +350,7 @@ static struct idmap_domain *idmap_passdb_domain(TALLOC_CTX *mem_ctx)
return passdb_idmap_domain;
}
- passdb_idmap_domain = idmap_init_domain(NULL, get_global_sam_name(),
+ passdb_idmap_domain = idmap_init_domain(mem_ctx, get_global_sam_name(),
"passdb", false);
if (passdb_idmap_domain == NULL) {
DEBUG(1, ("Could not init passdb idmap domain\n"));
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140723/94f38b3f/attachment.pgp>
More information about the samba-technical
mailing list