From 6351491af76bd55207595fca7b41f23f636564ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Jacke?= Date: Thu, 4 Aug 2011 16:42:37 +0200 Subject: [PATCH 01/13] s3/ldap: delay the ldap search alarm termination a bit do the alarm termination of the the ldap search a bit delayed so the LDAP server has a chance to tell us that the time limit was reached and the search was abandoned. If the search is terminated this way we also get the correct LDAP return code in the logs. If alarm() stops the search the ldap search routine will report that the LDAP server is down which would trigger us to rebind to the server needlessly which we also want to avoid. (cherry picked from commit fd06cf379ded801adf830499c24875a7c60280be) --- source3/lib/smbldap.c | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 1d507fb..c06fbbd 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1431,6 +1431,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, char *utf8_filter; time_t endtime = time_mono(NULL)+lp_ldap_timeout(); struct timeval timeout; + int alarm_timer; size_t converted_size; SMB_ASSERT(ldap_state); @@ -1476,11 +1477,21 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, * covers the case where the server's operation takes too long. It * does not cover the case where the request hangs on its way to the * server. The server side timeout is not strictly necessary, it's - * just a bit more kind to the server. VL. */ + * just a bit more kind to the server. VL. + * We prefer to get the server termination though because otherwise we + * have to rebind to the LDAP server aѕ we get a LDAP_SERVER_DOWN in the + * alarm termination case. So let's call the alarm 2s later, which + * should be enough for the server to report the timelimit exceeded. + * in case the ldal timeout is 0 (no limit) we set the _no_ alarm + * accordingly. BJ. */ got_alarm = 0; CatchSignal(SIGALRM, gotalarm_sig); - alarm(lp_ldap_timeout()); + alarm_timer = lp_ldap_timeout(); + if (alarm_timer > 0) { + alarm_timer += 2; + } + alarm(alarm_timer); /* End setup timeout. */ while (another_ldap_try(ldap_state, &rc, &attempts, endtime)) { -- 1.7.0.4 From 83ba426bba52c1d695a4ae76ae911731225cfa67 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:02:39 -0700 Subject: [PATCH 02/13] If "ldap timeout" is non-zero, set the local search timeout to be one second longer than the remote search timeout (which is set to the "ldap timeout" value). This allows the remote search timeout to fire in preference. Allow lp_ldap_timeout() to be zero. Don't set the any local alarm if so. --- source3/libads/ldap.c | 55 ++++++++++++++++++++++++++++++++----------------- 1 files changed, 36 insertions(+), 19 deletions(-) diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c index 90c645c..417f725 100644 --- a/source3/libads/ldap.c +++ b/source3/libads/ldap.c @@ -67,11 +67,13 @@ static void gotalarm_sig(int signum) DEBUG(10, ("Opening connection to LDAP server '%s:%d', timeout " "%u seconds\n", server, port, to)); - /* Setup timeout */ - gotalarm = 0; - CatchSignal(SIGALRM, gotalarm_sig); - alarm(to); - /* End setup timeout. */ + if (to) { + /* Setup timeout */ + gotalarm = 0; + CatchSignal(SIGALRM, gotalarm_sig); + alarm(to); + /* End setup timeout. */ + } ldp = ldap_open(server, port); @@ -82,9 +84,11 @@ static void gotalarm_sig(int signum) DEBUG(10, ("Connected to LDAP server '%s:%d'\n", server, port)); } - /* Teardown timeout. */ - CatchSignal(SIGALRM, SIG_IGN); - alarm(0); + if (to) { + /* Teardown timeout. */ + alarm(0); + CatchSignal(SIGALRM, SIG_IGN); + } return ldp; } @@ -100,26 +104,39 @@ static int ldap_search_with_timeout(LDAP *ld, int sizelimit, LDAPMessage **res ) { + int to = lp_ldap_timeout(); struct timeval timeout; + struct timeval *timeout_ptr = NULL; int result; /* Setup timeout for the ldap_search_ext_s call - local and remote. */ - timeout.tv_sec = lp_ldap_timeout(); - timeout.tv_usec = 0; - - /* Setup alarm timeout.... Do we need both of these ? JRA. */ gotalarm = 0; - CatchSignal(SIGALRM, gotalarm_sig); - alarm(lp_ldap_timeout()); - /* End setup timeout. */ + + if (to) { + timeout.tv_sec = to; + timeout.tv_usec = 0; + timeout_ptr = &timeout; + + /* Setup alarm timeout. */ + CatchSignal(SIGALRM, gotalarm_sig); + /* Make the alarm time one second beyond + the timout we're setting for the + remote search timeout, to allow that + to fire in preference. */ + alarm(to+1); + /* End setup timeout. */ + } + result = ldap_search_ext_s(ld, base, scope, filter, attrs, - attrsonly, sctrls, cctrls, &timeout, + attrsonly, sctrls, cctrls, timeout_ptr, sizelimit, res); - /* Teardown timeout. */ - CatchSignal(SIGALRM, SIG_IGN); - alarm(0); + if (to) { + /* Teardown alarm timeout. */ + CatchSignal(SIGALRM, SIG_IGN); + alarm(0); + } if (gotalarm != 0) return LDAP_TIMELIMIT_EXCEEDED; -- 1.7.0.4 From 967ee1c3d452d8da899a49a6dddc4d8ebf823221 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:03:42 -0700 Subject: [PATCH 03/13] Change got_alarm from bool to the correct type of SIG_ATOMIC_T. --- source3/lib/smbldap.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index c06fbbd..265410e 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1344,13 +1344,13 @@ static NTSTATUS smbldap_close(struct smbldap_state *ldap_state) return NT_STATUS_OK; } -static bool got_alarm; +static SIG_ATOMIC_T got_alarm; static void (*old_handler)(int); static void gotalarm_sig(int dummy) { - got_alarm = True; + got_alarm = 1; } static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, @@ -1369,7 +1369,7 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, } if (*attempts == 0) { - got_alarm = False; + got_alarm = 0; old_handler = CatchSignal(SIGALRM, gotalarm_sig); alarm(endtime - now); -- 1.7.0.4 From 4d8e40f68aa8f2ca2855c2712061cecb7f27beb3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:04:04 -0700 Subject: [PATCH 04/13] Remove old_handler as alarms can't be nested. Use SIG_IGN instead. --- source3/lib/smbldap.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 265410e..dd1d0aa 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1346,8 +1346,6 @@ static NTSTATUS smbldap_close(struct smbldap_state *ldap_state) static SIG_ATOMIC_T got_alarm; -static void (*old_handler)(int); - static void gotalarm_sig(int dummy) { got_alarm = 1; @@ -1370,7 +1368,7 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, if (*attempts == 0) { got_alarm = 0; - old_handler = CatchSignal(SIGALRM, gotalarm_sig); + CatchSignal(SIGALRM, gotalarm_sig); alarm(endtime - now); if (ldap_state->pid != sys_getpid()) @@ -1411,7 +1409,7 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, } no_next: - CatchSignal(SIGALRM, old_handler); + CatchSignal(SIGALRM, SIG_IGN); alarm(0); ldap_state->last_use = now; return False; -- 1.7.0.4 From 177cc5466c3d5aaa50edd769f6dd768ec5446353 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:04:32 -0700 Subject: [PATCH 05/13] Always remove the alarm before changing the handler, not the other way around. --- source3/lib/smbldap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index dd1d0aa..331e875 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1409,8 +1409,8 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, } no_next: - CatchSignal(SIGALRM, SIG_IGN); alarm(0); + CatchSignal(SIGALRM, SIG_IGN); ldap_state->last_use = now; return False; } -- 1.7.0.4 From 0ef9b6f903063a6d3a06892ff05b7c9e272ad342 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:04:58 -0700 Subject: [PATCH 06/13] Make it clear the time here is an absolute endtime. Don't set the alarm if the LDAP timeout is zero. --- source3/lib/smbldap.c | 50 ++++++++++++++++++++++++++++++++++-------------- 1 files changed, 35 insertions(+), 15 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 331e875..e7e56ed 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1351,8 +1351,25 @@ static void gotalarm_sig(int dummy) got_alarm = 1; } +static time_t calc_ldap_abs_endtime(int ldap_to) +{ + if (ldap_to == 0) { + /* No timeout - don't + return a value for + the alarm. */ + return (time_t)0; + } + + /* Make the alarm time one second beyond + the timout we're setting for the + remote search timeout, to allow that + to fire in preference. */ + + return time_mono(NULL)+ldap_to+1; +} + static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, - int *attempts, time_t endtime) + int *attempts, time_t abs_endtime) { time_t now = time_mono(NULL); int open_rc = LDAP_SERVER_DOWN; @@ -1360,16 +1377,18 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, if (*rc != LDAP_SERVER_DOWN) goto no_next; - if (now >= endtime) { + if (abs_endtime && now >= abs_endtime) { smbldap_close(ldap_state); *rc = LDAP_TIMEOUT; goto no_next; } if (*attempts == 0) { - got_alarm = 0; - CatchSignal(SIGALRM, gotalarm_sig); - alarm(endtime - now); + if (abs_endtime) { + got_alarm = 0; + CatchSignal(SIGALRM, gotalarm_sig); + alarm(abs_endtime - now); + } if (ldap_state->pid != sys_getpid()) smbldap_close(ldap_state); @@ -1427,7 +1446,8 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, int rc = LDAP_SERVER_DOWN; int attempts = 0; char *utf8_filter; - time_t endtime = time_mono(NULL)+lp_ldap_timeout(); + int to = lp_ldap_timeout(); + time_t abs_endtime = calc_ldap_abs_endtime(to); struct timeval timeout; int alarm_timer; size_t converted_size; @@ -1492,7 +1512,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, alarm(alarm_timer); /* End setup timeout. */ - while (another_ldap_try(ldap_state, &rc, &attempts, endtime)) { + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_search_ext_s(ldap_state->ldap_struct, base, scope, utf8_filter, CONST_DISCARD(char **, attrs), @@ -1637,7 +1657,7 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at int rc = LDAP_SERVER_DOWN; int attempts = 0; char *utf8_dn; - time_t endtime = time_mono(NULL)+lp_ldap_timeout(); + time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); size_t converted_size; SMB_ASSERT(ldap_state); @@ -1648,7 +1668,7 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at return LDAP_NO_MEMORY; } - while (another_ldap_try(ldap_state, &rc, &attempts, endtime)) { + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_modify_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc != LDAP_SUCCESS) { char *ld_error = NULL; @@ -1681,7 +1701,7 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs int rc = LDAP_SERVER_DOWN; int attempts = 0; char *utf8_dn; - time_t endtime = time_mono(NULL)+lp_ldap_timeout(); + time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); size_t converted_size; SMB_ASSERT(ldap_state); @@ -1692,7 +1712,7 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs return LDAP_NO_MEMORY; } - while (another_ldap_try(ldap_state, &rc, &attempts, endtime)) { + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_add_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc != LDAP_SUCCESS) { char *ld_error = NULL; @@ -1725,7 +1745,7 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) int rc = LDAP_SERVER_DOWN; int attempts = 0; char *utf8_dn; - time_t endtime = time_mono(NULL)+lp_ldap_timeout(); + time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); size_t converted_size; SMB_ASSERT(ldap_state); @@ -1736,7 +1756,7 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) return LDAP_NO_MEMORY; } - while (another_ldap_try(ldap_state, &rc, &attempts, endtime)) { + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_delete_s(ldap_state->ldap_struct, utf8_dn); if (rc != LDAP_SUCCESS) { char *ld_error = NULL; @@ -1771,12 +1791,12 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, { int rc = LDAP_SERVER_DOWN; int attempts = 0; - time_t endtime = time_mono(NULL)+lp_ldap_timeout(); + time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); if (!ldap_state) return (-1); - while (another_ldap_try(ldap_state, &rc, &attempts, endtime)) { + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_extended_operation_s(ldap_state->ldap_struct, reqoid, reqdata, serverctrls, clientctrls, retoidp, retdatap); -- 1.7.0.4 From bc2217619562dd9d3aa2ff3acc0953b57fab7e68 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:06:40 -0700 Subject: [PATCH 07/13] Allow the timeout pointer to ldap_search_ext_s() to be NULL if lp_ldap_timeout() == 0. --- source3/lib/smbldap.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index e7e56ed..07e5eca 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1449,6 +1449,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, int to = lp_ldap_timeout(); time_t abs_endtime = calc_ldap_abs_endtime(to); struct timeval timeout; + struct timeval *timeout_ptr = NULL; int alarm_timer; size_t converted_size; @@ -1486,9 +1487,12 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, return LDAP_NO_MEMORY; } - /* Setup timeout for the ldap_search_ext_s call - local and remote. */ - timeout.tv_sec = lp_ldap_timeout(); - timeout.tv_usec = 0; + /* Setup remote timeout for the ldap_search_ext_s call. */ + if (to) { + timeout.tv_sec = to; + timeout.tv_usec = 0; + timeout_ptr = &timeout; + } /* Setup alarm timeout.... Do we need both of these ? JRA. * Yes, I think we do need both of these. The server timeout only @@ -1516,7 +1520,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, rc = ldap_search_ext_s(ldap_state->ldap_struct, base, scope, utf8_filter, CONST_DISCARD(char **, attrs), - attrsonly, sctrls, cctrls, &timeout, + attrsonly, sctrls, cctrls, timeout_ptr, sizelimit, res); if (rc != LDAP_SUCCESS) { char *ld_error = NULL; -- 1.7.0.4 From 2787c67516c6a87c04fa7853cec68427d99c5de5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:07:14 -0700 Subject: [PATCH 08/13] Move the alarm setup/teardown out of another_ldap_try() and into separate functions that bracket the another_ldap_try() loop. We now never leave a dangling alarm pending on success. --- source3/lib/smbldap.c | 91 +++++++++++++++++++++++-------------------------- 1 files changed, 43 insertions(+), 48 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 07e5eca..500c44b 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1368,6 +1368,35 @@ static time_t calc_ldap_abs_endtime(int ldap_to) return time_mono(NULL)+ldap_to+1; } +static int end_ldap_local_alarm(time_t absolute_endtime, int rc) +{ + if (absolute_endtime) { + alarm(0); + CatchSignal(SIGALRM, SIG_IGN); + if (got_alarm) { + /* Client timeout error code. */ + got_alarm = 0; + return LDAP_TIMEOUT; + } + } + return rc; +} + +static void setup_ldap_local_alarm(struct smbldap_state *ldap_state, time_t absolute_endtime) +{ + time_t now = time_mono(NULL); + + if (absolute_endtime) { + got_alarm = 0; + CatchSignal(SIGALRM, gotalarm_sig); + alarm(absolute_endtime - now); + } + + if (ldap_state->pid != sys_getpid()) { + smbldap_close(ldap_state); + } +} + static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, int *attempts, time_t abs_endtime) { @@ -1383,17 +1412,6 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, goto no_next; } - if (*attempts == 0) { - if (abs_endtime) { - got_alarm = 0; - CatchSignal(SIGALRM, gotalarm_sig); - alarm(abs_endtime - now); - } - - if (ldap_state->pid != sys_getpid()) - smbldap_close(ldap_state); - } - while (1) { if (*attempts != 0) @@ -1428,8 +1446,6 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, } no_next: - alarm(0); - CatchSignal(SIGALRM, SIG_IGN); ldap_state->last_use = now; return False; } @@ -1450,7 +1466,6 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, time_t abs_endtime = calc_ldap_abs_endtime(to); struct timeval timeout; struct timeval *timeout_ptr = NULL; - int alarm_timer; size_t converted_size; SMB_ASSERT(ldap_state); @@ -1494,27 +1509,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, timeout_ptr = &timeout; } - /* Setup alarm timeout.... Do we need both of these ? JRA. - * Yes, I think we do need both of these. The server timeout only - * covers the case where the server's operation takes too long. It - * does not cover the case where the request hangs on its way to the - * server. The server side timeout is not strictly necessary, it's - * just a bit more kind to the server. VL. - * We prefer to get the server termination though because otherwise we - * have to rebind to the LDAP server aѕ we get a LDAP_SERVER_DOWN in the - * alarm termination case. So let's call the alarm 2s later, which - * should be enough for the server to report the timelimit exceeded. - * in case the ldal timeout is 0 (no limit) we set the _no_ alarm - * accordingly. BJ. */ - - got_alarm = 0; - CatchSignal(SIGALRM, gotalarm_sig); - alarm_timer = lp_ldap_timeout(); - if (alarm_timer > 0) { - alarm_timer += 2; - } - alarm(alarm_timer); - /* End setup timeout. */ + setup_ldap_local_alarm(ldap_state, abs_endtime); while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_search_ext_s(ldap_state->ldap_struct, base, scope, @@ -1545,15 +1540,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, } TALLOC_FREE(utf8_filter); - - /* Teardown timeout. */ - CatchSignal(SIGALRM, SIG_IGN); - alarm(0); - - if (got_alarm != 0) - return LDAP_TIMELIMIT_EXCEEDED; - - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_search(struct smbldap_state *ldap_state, @@ -1672,6 +1659,8 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at return LDAP_NO_MEMORY; } + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_modify_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc != LDAP_SUCCESS) { @@ -1697,7 +1686,7 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at } TALLOC_FREE(utf8_dn); - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs[]) @@ -1716,6 +1705,8 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs return LDAP_NO_MEMORY; } + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_add_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc != LDAP_SUCCESS) { @@ -1741,7 +1732,7 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs } TALLOC_FREE(utf8_dn); - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) @@ -1760,6 +1751,8 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) return LDAP_NO_MEMORY; } + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_delete_s(ldap_state->ldap_struct, utf8_dn); if (rc != LDAP_SUCCESS) { @@ -1785,7 +1778,7 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) } TALLOC_FREE(utf8_dn); - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_extended_operation(struct smbldap_state *ldap_state, @@ -1800,6 +1793,8 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, if (!ldap_state) return (-1); + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_extended_operation_s(ldap_state->ldap_struct, reqoid, reqdata, serverctrls, @@ -1826,7 +1821,7 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, } } - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } /******************************************************************* -- 1.7.0.4 From f1ac41bfc872fa739d3ca10878654391d0041894 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:09:20 -0700 Subject: [PATCH 09/13] Simplify the logic on the another_ldap_try() loops by breaking early out of the loop on success. --- source3/lib/smbldap.c | 200 ++++++++++++++++++++++++++----------------------- 1 files changed, 105 insertions(+), 95 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 500c44b..980908a 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1512,30 +1512,32 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, setup_ldap_local_alarm(ldap_state, abs_endtime); while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + char *ld_error = NULL; + int ld_errno; + rc = ldap_search_ext_s(ldap_state->ldap_struct, base, scope, utf8_filter, CONST_DISCARD(char **, attrs), attrsonly, sctrls, cctrls, timeout_ptr, sizelimit, res); - if (rc != LDAP_SUCCESS) { - char *ld_error = NULL; - int ld_errno; - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); - DEBUG(10, ("Failed search for base: %s, error: %d (%s) " - "(%s)\n", base, ld_errno, - ldap_err2string(rc), - ld_error ? ld_error : "unknown")); - SAFE_FREE(ld_error); - - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; - } + if (rc == LDAP_SUCCESS) { + break; + } + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_NUMBER, &ld_errno); + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_STRING, &ld_error); + DEBUG(10, ("Failed search for base: %s, error: %d (%s) " + "(%s)\n", base, ld_errno, + ldap_err2string(rc), + ld_error ? ld_error : "unknown")); + SAFE_FREE(ld_error); + + if (ld_errno == LDAP_SERVER_DOWN) { + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } } @@ -1662,26 +1664,28 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at setup_ldap_local_alarm(ldap_state, abs_endtime); while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + char *ld_error = NULL; + int ld_errno; + rc = ldap_modify_s(ldap_state->ldap_struct, utf8_dn, attrs); - if (rc != LDAP_SUCCESS) { - char *ld_error = NULL; - int ld_errno; - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); - DEBUG(10, ("Failed to modify dn: %s, error: %d (%s) " - "(%s)\n", dn, ld_errno, - ldap_err2string(rc), - ld_error ? ld_error : "unknown")); - SAFE_FREE(ld_error); - - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; - } + if (rc == LDAP_SUCCESS) { + break; + } + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_NUMBER, &ld_errno); + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_STRING, &ld_error); + DEBUG(10, ("Failed to modify dn: %s, error: %d (%s) " + "(%s)\n", dn, ld_errno, + ldap_err2string(rc), + ld_error ? ld_error : "unknown")); + SAFE_FREE(ld_error); + + if (ld_errno == LDAP_SERVER_DOWN) { + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } } @@ -1708,26 +1712,28 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs setup_ldap_local_alarm(ldap_state, abs_endtime); while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + char *ld_error = NULL; + int ld_errno; + rc = ldap_add_s(ldap_state->ldap_struct, utf8_dn, attrs); - if (rc != LDAP_SUCCESS) { - char *ld_error = NULL; - int ld_errno; - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); - DEBUG(10, ("Failed to add dn: %s, error: %d (%s) " - "(%s)\n", dn, ld_errno, - ldap_err2string(rc), - ld_error ? ld_error : "unknown")); - SAFE_FREE(ld_error); - - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; - } + if (rc == LDAP_SUCCESS) { + break; + } + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_NUMBER, &ld_errno); + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_STRING, &ld_error); + DEBUG(10, ("Failed to add dn: %s, error: %d (%s) " + "(%s)\n", dn, ld_errno, + ldap_err2string(rc), + ld_error ? ld_error : "unknown")); + SAFE_FREE(ld_error); + + if (ld_errno == LDAP_SERVER_DOWN) { + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } } @@ -1754,26 +1760,28 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) setup_ldap_local_alarm(ldap_state, abs_endtime); while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + char *ld_error = NULL; + int ld_errno; + rc = ldap_delete_s(ldap_state->ldap_struct, utf8_dn); - if (rc != LDAP_SUCCESS) { - char *ld_error = NULL; - int ld_errno; - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); - DEBUG(10, ("Failed to delete dn: %s, error: %d (%s) " - "(%s)\n", dn, ld_errno, - ldap_err2string(rc), - ld_error ? ld_error : "unknown")); - SAFE_FREE(ld_error); - - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; - } + if (rc == LDAP_SUCCESS) { + break; + } + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_NUMBER, &ld_errno); + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_STRING, &ld_error); + DEBUG(10, ("Failed to delete dn: %s, error: %d (%s) " + "(%s)\n", dn, ld_errno, + ldap_err2string(rc), + ld_error ? ld_error : "unknown")); + SAFE_FREE(ld_error); + + if (ld_errno == LDAP_SERVER_DOWN) { + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } } @@ -1796,28 +1804,30 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, setup_ldap_local_alarm(ldap_state, abs_endtime); while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + char *ld_error = NULL; + int ld_errno; + rc = ldap_extended_operation_s(ldap_state->ldap_struct, reqoid, reqdata, serverctrls, clientctrls, retoidp, retdatap); - if (rc != LDAP_SUCCESS) { - char *ld_error = NULL; - int ld_errno; - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); - - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); - DEBUG(10, ("Extended operation failed with error: " - "%d (%s) (%s)\n", ld_errno, - ldap_err2string(rc), - ld_error ? ld_error : "unknown")); - SAFE_FREE(ld_error); - - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; - } + if (rc == LDAP_SUCCESS) { + break; + } + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_NUMBER, &ld_errno); + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_STRING, &ld_error); + DEBUG(10, ("Extended operation failed with error: " + "%d (%s) (%s)\n", ld_errno, + ldap_err2string(rc), + ld_error ? ld_error : "unknown")); + SAFE_FREE(ld_error); + + if (ld_errno == LDAP_SERVER_DOWN) { + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } } -- 1.7.0.4 From 090d9e12a4c6517afea18a3fd78bd0f51a1549ac Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:10:14 -0700 Subject: [PATCH 10/13] Factor out the ldap_get_option calls into a function. --- source3/lib/smbldap.c | 34 ++++++++++++++-------------------- 1 files changed, 14 insertions(+), 20 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 980908a..e35e697 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1397,6 +1397,15 @@ static void setup_ldap_local_alarm(struct smbldap_state *ldap_state, time_t abso } } +static void get_ldap_errs(struct smbldap_state *ldap_state, char **pp_ld_error, int *p_ld_errno) +{ + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_NUMBER, p_ld_errno); + + ldap_get_option(ldap_state->ldap_struct, + LDAP_OPT_ERROR_STRING, pp_ld_error); +} + static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, int *attempts, time_t abs_endtime) { @@ -1524,11 +1533,8 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, break; } - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); + get_ldap_errs(ldap_state, &ld_error, &ld_errno); - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); DEBUG(10, ("Failed search for base: %s, error: %d (%s) " "(%s)\n", base, ld_errno, ldap_err2string(rc), @@ -1672,11 +1678,8 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at break; } - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); + get_ldap_errs(ldap_state, &ld_error, &ld_errno); - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); DEBUG(10, ("Failed to modify dn: %s, error: %d (%s) " "(%s)\n", dn, ld_errno, ldap_err2string(rc), @@ -1720,11 +1723,8 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs break; } - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); + get_ldap_errs(ldap_state, &ld_error, &ld_errno); - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); DEBUG(10, ("Failed to add dn: %s, error: %d (%s) " "(%s)\n", dn, ld_errno, ldap_err2string(rc), @@ -1768,11 +1768,8 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) break; } - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); + get_ldap_errs(ldap_state, &ld_error, &ld_errno); - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); DEBUG(10, ("Failed to delete dn: %s, error: %d (%s) " "(%s)\n", dn, ld_errno, ldap_err2string(rc), @@ -1814,11 +1811,8 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, break; } - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_NUMBER, &ld_errno); + get_ldap_errs(ldap_state, &ld_error, &ld_errno); - ldap_get_option(ldap_state->ldap_struct, - LDAP_OPT_ERROR_STRING, &ld_error); DEBUG(10, ("Extended operation failed with error: " "%d (%s) (%s)\n", ld_errno, ldap_err2string(rc), -- 1.7.0.4 From 0cafa545417e1a73786527c7984ba2bb469a15e1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 22:10:45 -0700 Subject: [PATCH 11/13] Remove the tortured logic in another_ldap_try() and turn it into get_cached_ldap_connect(), which much better describes it's function. Now we always break at the right places in the loop, we can replace the while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) construct with simply while (1). --- source3/lib/smbldap.c | 128 +++++++++++++++++++++++++++---------------------- 1 files changed, 70 insertions(+), 58 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index e35e697..ce2585a 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1406,57 +1406,44 @@ static void get_ldap_errs(struct smbldap_state *ldap_state, char **pp_ld_error, LDAP_OPT_ERROR_STRING, pp_ld_error); } -static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, - int *attempts, time_t abs_endtime) +static int get_cached_ldap_connect(struct smbldap_state *ldap_state) { - time_t now = time_mono(NULL); - int open_rc = LDAP_SERVER_DOWN; - - if (*rc != LDAP_SERVER_DOWN) - goto no_next; - - if (abs_endtime && now >= abs_endtime) { - smbldap_close(ldap_state); - *rc = LDAP_TIMEOUT; - goto no_next; - } + int attempts = 0; while (1) { + int rc; - if (*attempts != 0) - smb_msleep(1000); - - *attempts += 1; + rc = smbldap_open(ldap_state); - open_rc = smbldap_open(ldap_state); + ldap_state->last_use = time_mono(NULL); - if (open_rc == LDAP_SUCCESS) { - ldap_state->last_use = now; - return True; + if (rc == LDAP_SUCCESS) { + return LDAP_SUCCESS; } - if (open_rc == LDAP_INSUFFICIENT_ACCESS) { + attempts++; + DEBUG(1, ("Connection to LDAP server failed for the " + "%d try!\n", attempts)); + + if (rc == LDAP_INSUFFICIENT_ACCESS) { /* The fact that we are non-root or any other * access-denied condition will not change in the next * round of trying */ - *rc = open_rc; - break; + return rc; } if (got_alarm) { - *rc = LDAP_TIMEOUT; - break; + smbldap_close(ldap_state); + return LDAP_TIMEOUT; } - if (open_rc != LDAP_SUCCESS) { - DEBUG(1, ("Connection to LDAP server failed for the " - "%d try!\n", *attempts)); + smb_msleep(1000); + + if (got_alarm) { + smbldap_close(ldap_state); + return LDAP_TIMEOUT; } } - - no_next: - ldap_state->last_use = now; - return False; } /********************************************************************* @@ -1469,7 +1456,6 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, int sizelimit, LDAPMessage **res) { int rc = LDAP_SERVER_DOWN; - int attempts = 0; char *utf8_filter; int to = lp_ldap_timeout(); time_t abs_endtime = calc_ldap_abs_endtime(to); @@ -1520,10 +1506,15 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, setup_ldap_local_alarm(ldap_state, abs_endtime); - while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + while (1) { char *ld_error = NULL; int ld_errno; + rc = get_cached_ldap_connect(ldap_state); + if (rc != LDAP_SUCCESS) { + break; + } + rc = ldap_search_ext_s(ldap_state->ldap_struct, base, scope, utf8_filter, CONST_DISCARD(char **, attrs), @@ -1541,10 +1532,11 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, ld_error ? ld_error : "unknown")); SAFE_FREE(ld_error); - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; + if (ld_errno != LDAP_SERVER_DOWN) { + break; } + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } TALLOC_FREE(utf8_filter); @@ -1654,7 +1646,6 @@ done: int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs[]) { int rc = LDAP_SERVER_DOWN; - int attempts = 0; char *utf8_dn; time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); size_t converted_size; @@ -1669,10 +1660,15 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at setup_ldap_local_alarm(ldap_state, abs_endtime); - while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + while (1) { char *ld_error = NULL; int ld_errno; + rc = get_cached_ldap_connect(ldap_state); + if (rc != LDAP_SUCCESS) { + break; + } + rc = ldap_modify_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc == LDAP_SUCCESS) { break; @@ -1686,10 +1682,11 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at ld_error ? ld_error : "unknown")); SAFE_FREE(ld_error); - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; + if (ld_errno != LDAP_SERVER_DOWN) { + break; } + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } TALLOC_FREE(utf8_dn); @@ -1699,7 +1696,6 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs[]) { int rc = LDAP_SERVER_DOWN; - int attempts = 0; char *utf8_dn; time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); size_t converted_size; @@ -1714,10 +1710,15 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs setup_ldap_local_alarm(ldap_state, abs_endtime); - while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + while (1) { char *ld_error = NULL; int ld_errno; + rc = get_cached_ldap_connect(ldap_state); + if (rc != LDAP_SUCCESS) { + break; + } + rc = ldap_add_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc == LDAP_SUCCESS) { break; @@ -1731,10 +1732,11 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs ld_error ? ld_error : "unknown")); SAFE_FREE(ld_error); - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; + if (ld_errno != LDAP_SERVER_DOWN) { + break; } + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } TALLOC_FREE(utf8_dn); @@ -1744,7 +1746,6 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) { int rc = LDAP_SERVER_DOWN; - int attempts = 0; char *utf8_dn; time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); size_t converted_size; @@ -1759,10 +1760,15 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) setup_ldap_local_alarm(ldap_state, abs_endtime); - while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + while (1) { char *ld_error = NULL; int ld_errno; + rc = get_cached_ldap_connect(ldap_state); + if (rc != LDAP_SUCCESS) { + break; + } + rc = ldap_delete_s(ldap_state->ldap_struct, utf8_dn); if (rc == LDAP_SUCCESS) { break; @@ -1776,10 +1782,11 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) ld_error ? ld_error : "unknown")); SAFE_FREE(ld_error); - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; + if (ld_errno != LDAP_SERVER_DOWN) { + break; } + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } TALLOC_FREE(utf8_dn); @@ -1792,7 +1799,6 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, char **retoidp, struct berval **retdatap) { int rc = LDAP_SERVER_DOWN; - int attempts = 0; time_t abs_endtime = calc_ldap_abs_endtime(lp_ldap_timeout()); if (!ldap_state) @@ -1800,10 +1806,15 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, setup_ldap_local_alarm(ldap_state, abs_endtime); - while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { + while (1) { char *ld_error = NULL; int ld_errno; + rc = get_cached_ldap_connect(ldap_state); + if (rc != LDAP_SUCCESS) { + break; + } + rc = ldap_extended_operation_s(ldap_state->ldap_struct, reqoid, reqdata, serverctrls, clientctrls, retoidp, retdatap); @@ -1819,10 +1830,11 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, ld_error ? ld_error : "unknown")); SAFE_FREE(ld_error); - if (ld_errno == LDAP_SERVER_DOWN) { - ldap_unbind(ldap_state->ldap_struct); - ldap_state->ldap_struct = NULL; + if (ld_errno != LDAP_SERVER_DOWN) { + break; } + ldap_unbind(ldap_state->ldap_struct); + ldap_state->ldap_struct = NULL; } return end_ldap_local_alarm(abs_endtime, rc); -- 1.7.0.4 From 089d36cc8aba209f1e86ef2e55b9d429635a1535 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sat, 20 Aug 2011 09:37:04 -0700 Subject: [PATCH 12/13] Ensure we never wait past absolute entime to do a get_cached_ldap_connect(). --- source3/lib/smbldap.c | 23 +++++++++++++++-------- 1 files changed, 15 insertions(+), 8 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index ce2585a..adeee65 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1406,16 +1406,23 @@ static void get_ldap_errs(struct smbldap_state *ldap_state, char **pp_ld_error, LDAP_OPT_ERROR_STRING, pp_ld_error); } -static int get_cached_ldap_connect(struct smbldap_state *ldap_state) +static int get_cached_ldap_connect(struct smbldap_state *ldap_state, time_t abs_endtime) { int attempts = 0; while (1) { int rc; + time_t now; - rc = smbldap_open(ldap_state); + now = time_mono(NULL); + ldap_state->last_use = now; + + if (abs_endtime && now > abs_endtime) { + smbldap_close(ldap_state); + return LDAP_TIMEOUT; + } - ldap_state->last_use = time_mono(NULL); + rc = smbldap_open(ldap_state); if (rc == LDAP_SUCCESS) { return LDAP_SUCCESS; @@ -1510,7 +1517,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, char *ld_error = NULL; int ld_errno; - rc = get_cached_ldap_connect(ldap_state); + rc = get_cached_ldap_connect(ldap_state, abs_endtime); if (rc != LDAP_SUCCESS) { break; } @@ -1664,7 +1671,7 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at char *ld_error = NULL; int ld_errno; - rc = get_cached_ldap_connect(ldap_state); + rc = get_cached_ldap_connect(ldap_state, abs_endtime); if (rc != LDAP_SUCCESS) { break; } @@ -1714,7 +1721,7 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs char *ld_error = NULL; int ld_errno; - rc = get_cached_ldap_connect(ldap_state); + rc = get_cached_ldap_connect(ldap_state, abs_endtime); if (rc != LDAP_SUCCESS) { break; } @@ -1764,7 +1771,7 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) char *ld_error = NULL; int ld_errno; - rc = get_cached_ldap_connect(ldap_state); + rc = get_cached_ldap_connect(ldap_state, abs_endtime); if (rc != LDAP_SUCCESS) { break; } @@ -1810,7 +1817,7 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, char *ld_error = NULL; int ld_errno; - rc = get_cached_ldap_connect(ldap_state); + rc = get_cached_ldap_connect(ldap_state, abs_endtime); if (rc != LDAP_SUCCESS) { break; } -- 1.7.0.4 From 6cee5107402b17495b72790dcf1b7f5f83e60b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Jacke?= Date: Mon, 15 Aug 2011 14:46:12 +0200 Subject: [PATCH 13/13] s3/ldap: don't continue if we couldn't get the domain info on startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit while some things work without the domain info, some important things don't, which is highly irritating. As even calls like EnumTrustDom fail and thus clients' domain logins fail we are sufficiently broken to refuse to go on. Autobuild-User: Björn Jacke Autobuild-Date: Thu Aug 18 12:48:37 CEST 2011 on sn-devel-104 (cherry picked from commit dcb5720ad01fabac300da478368c140e2b6313d2) (cherry picked from commit 45eafcef64be262272cffaa9cf98c7dbf4d04882) --- source3/passdb/pdb_ldap.c | 11 ++++------- 1 files changed, 4 insertions(+), 7 deletions(-) diff --git a/source3/passdb/pdb_ldap.c b/source3/passdb/pdb_ldap.c index 7ce5edc..10a4531 100644 --- a/source3/passdb/pdb_ldap.c +++ b/source3/passdb/pdb_ldap.c @@ -6599,13 +6599,10 @@ NTSTATUS pdb_init_ldapsam(struct pdb_methods **pdb_method, const char *location) ldap_state->domain_name, True); if ( !NT_STATUS_IS_OK(nt_status) ) { - DEBUG(2, ("pdb_init_ldapsam: WARNING: Could not get domain " - "info, nor add one to the domain\n")); - DEBUGADD(2, ("pdb_init_ldapsam: Continuing on regardless, " - "will be unable to allocate new users/groups, " - "and will risk BDCs having inconsistent SIDs\n")); - sid_copy(&ldap_state->domain_sid, get_global_sam_sid()); - return NT_STATUS_OK; + DEBUG(0, ("pdb_init_ldapsam: WARNING: Could not get domain " + "info, nor add one to the domain. " + "We cannot work reliably without it.\n")); + return NT_STATUS_CANT_ACCESS_DOMAIN_INFO; } /* Given that the above might fail, everything below this must be -- 1.7.0.4