From: Pavel Rochnyack Date: Mon, 24 Jul 2017 08:07:16 +0000 (+0700) Subject: openldap: Fix plugin shutdown with connection failed X-Git-Tag: collectd-5.8.0~87^2 X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=ee2af3ba8ed42f5f04962f56edb59b80eb545f77;p=collectd.git openldap: Fix plugin shutdown with connection failed When plugin failes to connect to LDAP server, the `ldap_unbind_ext_s(st->ld)` is called. According to `man 3 ldap_unbind_ext_s`, that call is used to free the resources contained in the ld structure. When plugin shutdown, the `ldap_unbind_ext_s` called again, which causes coredump. The plugin code was changed to set st->ld to NULL after `ldap_unbind_ext_s()` call. --- diff --git a/src/openldap.c b/src/openldap.c index 227c8e11..afe2479e 100644 --- a/src/openldap.c +++ b/src/openldap.c @@ -47,7 +47,6 @@ struct cldap_s /* {{{ */ char *password; char *cacert; char *host; - int state; _Bool starttls; int timeout; char *url; @@ -58,11 +57,10 @@ struct cldap_s /* {{{ */ }; typedef struct cldap_s cldap_t; /* }}} */ -static cldap_t **databases = NULL; -static size_t databases_num = 0; - -static void cldap_free(cldap_t *st) /* {{{ */ +static void cldap_free(void *arg) /* {{{ */ { + cldap_t *st = arg; + if (st == NULL) return; @@ -73,32 +71,30 @@ static void cldap_free(cldap_t *st) /* {{{ */ sfree(st->name); sfree(st->url); if (st->ld) - ldap_memfree(st->ld); + ldap_unbind_ext_s(st->ld, NULL, NULL); + sfree(st); } /* }}} void cldap_free */ /* initialize ldap for each host */ static int cldap_init_host(cldap_t *st) /* {{{ */ { - LDAP *ld; int rc; - if (st->state && st->ld) { + if (st->ld) { DEBUG("openldap plugin: Already connected to %s", st->url); return 0; } - rc = ldap_initialize(&ld, st->url); + rc = ldap_initialize(&st->ld, st->url); if (rc != LDAP_SUCCESS) { ERROR("openldap plugin: ldap_initialize failed: %s", ldap_err2string(rc)); - st->state = 0; - if (ld != NULL) - ldap_unbind_ext_s(ld, NULL, NULL); + if (st->ld != NULL) + ldap_unbind_ext_s(st->ld, NULL, NULL); + st->ld = NULL; return (-1); } - st->ld = ld; - ldap_set_option(st->ld, LDAP_OPT_PROTOCOL_VERSION, &st->version); ldap_set_option(st->ld, LDAP_OPT_TIMEOUT, @@ -115,13 +111,12 @@ static int cldap_init_host(cldap_t *st) /* {{{ */ } if (st->starttls != 0) { - rc = ldap_start_tls_s(ld, NULL, NULL); + rc = ldap_start_tls_s(st->ld, NULL, NULL); if (rc != LDAP_SUCCESS) { ERROR("openldap plugin: Failed to start tls on %s: %s", st->url, ldap_err2string(rc)); - st->state = 0; - if (st->ld != NULL) - ldap_unbind_ext_s(st->ld, NULL, NULL); + ldap_unbind_ext_s(st->ld, NULL, NULL); + st->ld = NULL; return (-1); } } @@ -140,13 +135,11 @@ static int cldap_init_host(cldap_t *st) /* {{{ */ if (rc != LDAP_SUCCESS) { ERROR("openldap plugin: Failed to bind to %s: %s", st->url, ldap_err2string(rc)); - st->state = 0; - if (st->ld != NULL) - ldap_unbind_ext_s(st->ld, NULL, NULL); + ldap_unbind_ext_s(st->ld, NULL, NULL); + st->ld = NULL; return (-1); } else { DEBUG("openldap plugin: Successfully connected to %s", st->url); - st->state = 1; return 0; } } /* }}} static cldap_init_host */ @@ -216,9 +209,8 @@ static int cldap_read_host(user_data_t *ud) /* {{{ */ if (rc != LDAP_SUCCESS) { ERROR("openldap plugin: Failed to execute search: %s", ldap_err2string(rc)); ldap_msgfree(result); - st->state = 0; - if (st->ld != NULL) - ldap_unbind_ext_s(st->ld, NULL, NULL); + ldap_unbind_ext_s(st->ld, NULL, NULL); + st->ld = NULL; return (-1); } @@ -463,42 +455,24 @@ static int cldap_config_add(oconfig_item_t *ci) /* {{{ */ ldap_free_urldesc(ludpp); } - if (status == 0) { - cldap_t **temp; - - temp = (cldap_t **)realloc(databases, - sizeof(*databases) * (databases_num + 1)); - - if (temp == NULL) { - ERROR("openldap plugin: realloc failed"); - status = -1; - } else { - char callback_name[3 * DATA_MAX_NAME_LEN] = {0}; - - databases = temp; - databases[databases_num] = st; - databases_num++; - - snprintf(callback_name, sizeof(callback_name), "openldap/%s/%s", - (st->host != NULL) ? st->host : hostname_g, - (st->name != NULL) ? st->name : "default"); - - status = plugin_register_complex_read(/* group = */ NULL, - /* name = */ callback_name, - /* callback = */ cldap_read_host, - /* interval = */ 0, - &(user_data_t){ - .data = st, - }); - } - } - if (status != 0) { cldap_free(st); return -1; } - return 0; + char callback_name[3 * DATA_MAX_NAME_LEN] = {0}; + + snprintf(callback_name, sizeof(callback_name), "openldap/%s/%s", + (st->host != NULL) ? st->host : hostname_g, + (st->name != NULL) ? st->name : "default"); + + return plugin_register_complex_read(/* group = */ NULL, + /* name = */ callback_name, + /* callback = */ cldap_read_host, + /* interval = */ 0, + &(user_data_t){ + .data = st, .free_func = cldap_free, + }); } /* }}} int cldap_config_add */ static int cldap_config(oconfig_item_t *ci) /* {{{ */ @@ -532,22 +506,10 @@ static int cldap_init(void) /* {{{ */ return 0; } /* }}} int cldap_init */ -static int cldap_shutdown(void) /* {{{ */ -{ - for (size_t i = 0; i < databases_num; i++) - if (databases[i] != NULL && databases[i]->ld != NULL) - ldap_unbind_ext_s(databases[i]->ld, NULL, NULL); - sfree(databases); - databases_num = 0; - - return 0; -} /* }}} int cldap_shutdown */ - void module_register(void) /* {{{ */ { plugin_register_complex_config("openldap", cldap_config); plugin_register_init("openldap", cldap_init); - plugin_register_shutdown("openldap", cldap_shutdown); } /* }}} void module_register */ #if defined(__APPLE__)