Re: pkg/36853: clamav-milter doesn't start - _res is not supported for multi-threaded programs.

[christos%zoulas.com@localhost (Christos Zoulas)]

The following reply was made to PR pkg/36853; it has been noted by GNATS.

From: christos%zoulas.com@localhost (Christos Zoulas)
To: gnats-bugs%NetBSD.org@localhost, martti%NetBSD.org@localhost, 
gnats-admin%netbsd.org@localhost,
        pkgsrc-bugs%netbsd.org@localhost, carl%bl.echidna.id.au@localhost
Cc: 
Subject: Re: pkg/36853: clamav-milter doesn't start - _res is not supported for 
multi-threaded programs.
Date: Fri, 21 Dec 2007 11:48:16 -0500

 On Dec 21, 11:30am, carl%bl.echidna.id.au@localhost (Carl Brewer) wrote:
 -- Subject: Re: pkg/36853: clamav-milter doesn't start - _res is not supporte
 
 Here is an *UNTESTED* patch that fixes the race in res_query for non-bind9
 systems, and uses res_ninit/res_nquery in bind9 systems.
 
 Enjoy,
 
 christos
 
 --- clamav-milter.c.orig       2007-12-12 17:41:25.000000000 -0500
 +++ clamav-milter.c    2007-12-21 11:47:01.000000000 -0500
 @@ -90,6 +90,9 @@
  #if   HAVE_RESOLV_H
  #include <arpa/nameser.h>     /* for HEADER */
  #include <resolv.h>
 +#if __RES >= 20030124
 +#define       HAVE_BIND9
 +#endif
  #endif
  #ifdef        HAVE_UNISTD_H
  #include <unistd.h>
 @@ -505,6 +508,20 @@
  #endif        /*SESSION*/
  
  static        pthread_cond_t  watchdog_cond = PTHREAD_COND_INITIALIZER;
 +#ifndef HAVE_BIND9
 +static        pthread_mutex_t res_mutex = PTHREAD_MUTEX_INITIALIZER;
 +#define RES_QUERY(len, h, c, t, q, s) \
 +      do { \
 +              pthread_mutex_lock(&res_mutex); \
 +              len = res_query((h), (c), (t), (q), (s)); \
 +              pthread_mutex_unlock(&res_mutex); \
 +      } while (/*CONSTCOND*/0)
 +#else
 +static        pthread_key_t res_key;
 +#define RES_QUERY(len, h, c, t, q, s) \
 +      len = res_nquery((res_state)pthread_getspecific(res_key), \
 +          (h), (c), (t), (q), (s))
 +#endif
  
  #ifndef       SHUT_RD
  #define       SHUT_RD         0
 @@ -2061,12 +2078,19 @@
        logg(_("Starting %s\n"), clamav_version);
        logg(_("*Debugging is on\n"));
  
 +#ifndef HAVE_BIND9
        if(!(_res.options&RES_INIT))
                if(res_init() < 0) {
                        fprintf(stderr, "%s: Can't initialise the resolver\n",
                                argv[0]);
                        return EX_UNAVAILABLE;
                }
 +#else
 +      if (pthread_key_create(&res_key, free)) {
 +              perror("pthread_key_create");
 +              return EX_UNAVAILABLE;
 +      }
 +#endif
  
        if(blacklist_time) {
                char name[MAXHOSTNAMELEN + 1];
 @@ -2579,6 +2603,18 @@
        int sock = s->sock;
        struct sockaddr *server = (struct sockaddr *)s->server;
        int server_index = s->server_index;
 +#ifdef HAVE_BIND9
 +      res_state res = cli_calloc(1, sizeof(*res));
 +      if (pthread_setspecific(res_key, res)) {
 +              perror("pthread_setspecific");
 +              free(res);
 +              return NULL;
 +      }
 +      if (res_ninit(res) < 0) {
 +              perror("res_ninit");
 +              return NULL;
 +      }
 +#endif
  
        if(last_failed_pings[server_index]) {
                s->rc = 0;
 @@ -6150,7 +6186,7 @@
                        return NULL;
        }
  
 -      len = res_query(host, C_IN, T_MX, (u_char *)&q, sizeof(q));
 +      RES_QUERY(len, host, C_IN, T_MX, (u_char *)&q, sizeof(q));
        if(len < 0)
                return t;       /* Host has no MX records */
  
 @@ -6219,7 +6255,7 @@
        if((host == NULL) || (*host == '\0'))
                return t;
  
 -      len = res_query(host, C_IN, T_A, (u_char *)&q, sizeof(q));
 +      RES_QUERY(len, host, C_IN, T_A, (u_char *)&q, sizeof(q));
        if(len < 0)
                return t;       /* Host has no A records */
  
 @@ -6278,7 +6314,6 @@
   *    an SPF system, we ONLY use SPF records to reduce phish false positives
   * TODO: IPv6?
   * TODO: cache queries?
 - * TODO: check res_query is thread safe
   *
   * INPUT: prevhosts, a list of hosts already searched: stops include loops
   *    e.g. mercado.com includes medrcadosw.com which includes mercado.com,
 @@ -6330,7 +6365,7 @@
                *ptr = '\0';
  
        logg("*SPF query '%s'\n", host);
 -      len = res_query(host, C_IN, T_TXT, (u_char *)&q, sizeof(q));
 +      RES_QUERY(len, host, C_IN, T_TXT, (u_char *)&q, sizeof(q));
        if(len < 0) {
                free(host);
                return 0;       /* Host has no TXT records */