libunbound: Protect calls to ub_ctx_config() by mutex

[dilyanpalauzov]

libunbound:ub_ctx_config() uses internally a flex generated non-reentrant lexer and bison generated non-pure paprser. Calling simultaneously two non-reentrant lexers or parsers is a bad idea, and two threads can alter the state of the lexer/parser without synchronization.

The calls to ub_ctx_config in opendkim/opendkim-dns.c:dkimf_ub_config need to be protected by mutex. The documentation of libunbound:ub_ctx_config() says the function is thread-safe, but this is only true when only one instance of libunbound:ub_ctx * is used in the application, which is not the case with opendkim. See also https://www.nlnetlabs.nl/bugs-script/show_bug.cgi?id=1913.

Copy of https://sourceforge.net/p/opendkim/bugs/265/.

diff --git a/opendkim/opendkim-dns.c b/opendkim/opendkim-dns.c
--- a/opendkim/opendkim-dns.c
+++ b/opendkim/opendkim-dns.c
@@ -75,6 +75,7 @@ struct dkimf_unbound
        pthread_cond_t          ub_ready;
 };

+pthread_mutex_t                ub_config_mutex;
 /* struct dkimf_unbound_cb_data -- libunbound callback data */
 struct dkimf_unbound_cb_data
 {
@@ -440,7 +441,9 @@ dkimf_ub_config(void *srv, const char *file)

        ub = srv;

+       pthread_mutex_lock(&ub_config_mutex);
        status = ub_ctx_config(ub->ub_ub, (char *) file);
+       pthread_mutex_unlock(&ub_config_mutex);

        return (status == 0 ? 0 : -1);
 }
diff --git a/opendkim/opendkim-dns.h b/opendkim/opendkim-dns.h
--- a/opendkim/opendkim-dns.h
+++ b/opendkim/opendkim-dns.h
@@ -33,6 +33,7 @@ struct dkimf_filedns;
 /* libunbound includes */
 # include <unbound.h>

+extern pthread_mutex_t         ub_config_mutex;
 /* prototypes */
 extern int dkimf_unbound_setup __P((DKIM_LIB *));
 # ifdef _FFR_RBL
diff --git a/opendkim/opendkim.c b/opendkim/opendkim.c
--- a/opendkim/opendkim.c
+++ b/opendkim/opendkim.c
@@ -15555,6 +15555,9 @@ main(int argc, char **argv)
        no_i_whine = TRUE;
        conffile = NULL;

+#ifdef USE_UNBOUND
+       pthread_mutex_init(&ub_config_mutex, NULL);
+#endif
        memset(myhostname, '\0', sizeof myhostname);
        (void) gethostname(myhostname, sizeof myhostname);

[mskucherawy]

Patch applied.

[dilyanpalauzov]

The lock protecting the calls to ub_ctx_config() must be global/application-wide, not per libunbound instance.