openldap: Fix plugin shutdown with connection failed
authorPavel Rochnyack <pavel2000@ngs.ru>
Mon, 24 Jul 2017 08:07:16 +0000 (15:07 +0700)
committerPavel Rochnyack <pavel2000@ngs.ru>
Tue, 25 Jul 2017 13:19:45 +0000 (20:19 +0700)
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.

src/openldap.c

index 227c8e1..afe2479 100644 (file)
@@ -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__)