thread deadlock: make sure channel lock isn't used recursively

v1.26
Brad House 5 months ago
parent 5b1aacd8bd
commit fc57af0174
  1. 3
      docs/ares_init_options.3
  2. 14
      src/lib/ares__threads.c
  3. 6
      src/lib/ares_gethostbyaddr.c
  4. 7
      src/lib/ares_getnameinfo.c
  5. 5
      src/lib/ares_init.c
  6. 7
      src/lib/ares_private.h
  7. 14
      src/lib/ares_search.c
  8. 16
      src/lib/ares_update_servers.c

@ -217,7 +217,8 @@ The value of
.I sock_state_cb_data .I sock_state_cb_data
will be passed as the will be passed as the
.I data .I data
argument. argument. Since this may be called cross-thread, a channel lock is NOT held
when this callback is called.
Cannot be used with \fBARES_OPT_EVENT_THREAD\fP. Cannot be used with \fBARES_OPT_EVENT_THREAD\fP.
.TP 18 .TP 18

@ -145,30 +145,18 @@ struct ares__thread_mutex {
ares__thread_mutex_t *ares__thread_mutex_create(void) ares__thread_mutex_t *ares__thread_mutex_create(void)
{ {
pthread_mutexattr_t attr;
ares__thread_mutex_t *mut = ares_malloc_zero(sizeof(*mut)); ares__thread_mutex_t *mut = ares_malloc_zero(sizeof(*mut));
if (mut == NULL) { if (mut == NULL) {
return NULL; return NULL;
} }
if (pthread_mutexattr_init(&attr) != 0) { if (pthread_mutex_init(&mut->mutex, NULL) != 0) {
ares_free(mut);
return NULL;
}
if (pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) != 0) {
goto fail;
}
if (pthread_mutex_init(&mut->mutex, &attr) != 0) {
goto fail; goto fail;
} }
pthread_mutexattr_destroy(&attr);
return mut; return mut;
fail: fail:
pthread_mutexattr_destroy(&attr);
ares_free(mut); ares_free(mut);
return NULL; return NULL;
} }

@ -68,7 +68,7 @@ static ares_status_t file_lookup(ares_channel_t *channel,
const struct ares_addr *addr, const struct ares_addr *addr,
struct hostent **host); struct hostent **host);
static void ares_gethostbyaddr_int(ares_channel_t *channel, const void *addr, void ares_gethostbyaddr_nolock(ares_channel_t *channel, const void *addr,
int addrlen, int family, int addrlen, int family,
ares_host_callback callback, void *arg) ares_host_callback callback, void *arg)
{ {
@ -118,7 +118,7 @@ void ares_gethostbyaddr(ares_channel_t *channel, const void *addr, int addrlen,
return; return;
} }
ares__channel_lock(channel); ares__channel_lock(channel);
ares_gethostbyaddr_int(channel, addr, addrlen, family, callback, arg); ares_gethostbyaddr_nolock(channel, addr, addrlen, family, callback, arg);
ares__channel_unlock(channel); ares__channel_unlock(channel);
} }
@ -138,7 +138,7 @@ static void next_lookup(struct addr_query *aquery)
return; return;
} }
aquery->remaining_lookups = p + 1; aquery->remaining_lookups = p + 1;
ares_query(aquery->channel, name, C_IN, T_PTR, addr_callback, aquery); ares_query_qid(aquery->channel, name, C_IN, T_PTR, addr_callback, aquery, NULL);
ares_free(name); ares_free(name);
return; return;
case 'f': case 'f':

@ -173,12 +173,13 @@ static void ares_getnameinfo_int(ares_channel_t *channel,
if (sa->sa_family == AF_INET) { if (sa->sa_family == AF_INET) {
niquery->family = AF_INET; niquery->family = AF_INET;
memcpy(&niquery->addr.addr4, addr, sizeof(niquery->addr.addr4)); memcpy(&niquery->addr.addr4, addr, sizeof(niquery->addr.addr4));
ares_gethostbyaddr(channel, &addr->sin_addr, sizeof(struct in_addr), ares_gethostbyaddr_nolock(channel, &addr->sin_addr,
AF_INET, nameinfo_callback, niquery); sizeof(struct in_addr), AF_INET,
nameinfo_callback, niquery);
} else { } else {
niquery->family = AF_INET6; niquery->family = AF_INET6;
memcpy(&niquery->addr.addr6, addr6, sizeof(niquery->addr.addr6)); memcpy(&niquery->addr.addr6, addr6, sizeof(niquery->addr.addr6));
ares_gethostbyaddr(channel, &addr6->sin6_addr, ares_gethostbyaddr_nolock(channel, &addr6->sin6_addr,
sizeof(struct ares_in6_addr), AF_INET6, sizeof(struct ares_in6_addr), AF_INET6,
nameinfo_callback, niquery); nameinfo_callback, niquery);
} }

@ -422,7 +422,6 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src)
*dest = NULL; /* in case of failure return NULL explicitly */ *dest = NULL; /* in case of failure return NULL explicitly */
ares__channel_lock(src);
/* First get the options supported by the old ares_save_options() function, /* First get the options supported by the old ares_save_options() function,
which is most of them */ which is most of them */
rc = (ares_status_t)ares_save_options(src, &opts, &optmask); rc = (ares_status_t)ares_save_options(src, &opts, &optmask);
@ -441,6 +440,7 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src)
goto done; goto done;
} }
ares__channel_lock(src);
/* Now clone the options that ares_save_options() doesn't support, but are /* Now clone the options that ares_save_options() doesn't support, but are
* user-provided */ * user-provided */
(*dest)->sock_create_cb = src->sock_create_cb; (*dest)->sock_create_cb = src->sock_create_cb;
@ -454,7 +454,7 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src)
sizeof((*dest)->local_dev_name)); sizeof((*dest)->local_dev_name));
(*dest)->local_ip4 = src->local_ip4; (*dest)->local_ip4 = src->local_ip4;
memcpy((*dest)->local_ip6, src->local_ip6, sizeof(src->local_ip6)); memcpy((*dest)->local_ip6, src->local_ip6, sizeof(src->local_ip6));
ares__channel_unlock(src);
/* Servers are a bit unique as ares_init_options() only allows ipv4 servers /* Servers are a bit unique as ares_init_options() only allows ipv4 servers
* and not a port per server, but there are other user specified ways, that * and not a port per server, but there are other user specified ways, that
@ -486,7 +486,6 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src)
rc = ARES_SUCCESS; rc = ARES_SUCCESS;
done: done:
ares__channel_unlock(src);
return (int)rc; /* everything went fine */ return (int)rc; /* everything went fine */
} }

@ -490,6 +490,11 @@ ares_status_t ares__hosts_entry_to_addrinfo(const ares_hosts_entry_t *entry,
ares_bool_t want_cnames, ares_bool_t want_cnames,
struct ares_addrinfo *ai); struct ares_addrinfo *ai);
/* Same as ares_gethostbyaddr() except does not take a channel lock. Use this
* if a channel lock is already held */
void ares_gethostbyaddr_nolock(ares_channel_t *channel, const void *addr,
int addrlen, int family,
ares_host_callback callback, void *arg);
/*! Parse a compressed DNS name as defined in RFC1035 starting at the current /*! Parse a compressed DNS name as defined in RFC1035 starting at the current
* offset within the buffer. * offset within the buffer.
@ -539,7 +544,9 @@ ares_status_t ares__dns_name_write(ares__buf_t *buf, ares__llist_t **list,
#define SOCK_STATE_CALLBACK(c, s, r, w) \ #define SOCK_STATE_CALLBACK(c, s, r, w) \
do { \ do { \
if ((c)->sock_state_cb) { \ if ((c)->sock_state_cb) { \
ares__channel_unlock((c)); \
(c)->sock_state_cb((c)->sock_state_cb_data, (s), (r), (w)); \ (c)->sock_state_cb((c)->sock_state_cb_data, (s), (r), (w)); \
ares__channel_lock((c)); \
} \ } \
} while (0) } while (0)

@ -82,7 +82,7 @@ static void ares_search_int(ares_channel_t *channel, const char *name,
return; return;
} }
if (s) { if (s) {
ares_query(channel, s, dnsclass, type, callback, arg); ares_query_qid(channel, s, dnsclass, type, callback, arg, NULL);
ares_free(s); ares_free(s);
return; return;
} }
@ -140,14 +140,14 @@ static void ares_search_int(ares_channel_t *channel, const char *name,
/* Try the name as-is first. */ /* Try the name as-is first. */
squery->next_domain = 0; squery->next_domain = 0;
squery->trying_as_is = ARES_TRUE; squery->trying_as_is = ARES_TRUE;
ares_query(channel, name, dnsclass, type, search_callback, squery); ares_query_qid(channel, name, dnsclass, type, search_callback, squery, NULL);
} else { } else {
/* Try the name as-is last; start with the first search domain. */ /* Try the name as-is last; start with the first search domain. */
squery->next_domain = 1; squery->next_domain = 1;
squery->trying_as_is = ARES_FALSE; squery->trying_as_is = ARES_FALSE;
status = ares__cat_domain(name, squery->domains[0], &s); status = ares__cat_domain(name, squery->domains[0], &s);
if (status == ARES_SUCCESS) { if (status == ARES_SUCCESS) {
ares_query(channel, s, dnsclass, type, search_callback, squery); ares_query_qid(channel, s, dnsclass, type, search_callback, squery, NULL);
ares_free(s); ares_free(s);
} else { } else {
/* failed, free the malloc()ed memory */ /* failed, free the malloc()ed memory */
@ -209,15 +209,15 @@ static void search_callback(void *arg, int status, int timeouts,
} else { } else {
squery->trying_as_is = ARES_FALSE; squery->trying_as_is = ARES_FALSE;
squery->next_domain++; squery->next_domain++;
ares_query(channel, s, squery->dnsclass, squery->type, search_callback, ares_query_qid(channel, s, squery->dnsclass, squery->type,
squery); search_callback, squery, NULL);
ares_free(s); ares_free(s);
} }
} else if (squery->status_as_is == -1) { } else if (squery->status_as_is == -1) {
/* Try the name as-is at the end. */ /* Try the name as-is at the end. */
squery->trying_as_is = ARES_TRUE; squery->trying_as_is = ARES_TRUE;
ares_query(channel, squery->name, squery->dnsclass, squery->type, ares_query_qid(channel, squery->name, squery->dnsclass, squery->type,
search_callback, squery); search_callback, squery, NULL);
} else { } else {
if (squery->status_as_is == ARES_ENOTFOUND && squery->ever_got_nodata) { if (squery->status_as_is == ARES_ENOTFOUND && squery->ever_got_nodata) {
end_squery(squery, ARES_ENODATA, NULL, 0); end_squery(squery, ARES_ENODATA, NULL, 0);

@ -703,8 +703,6 @@ ares_status_t ares__servers_update(ares_channel_t *channel,
return ARES_EFORMERR; return ARES_EFORMERR;
} }
ares__channel_lock(channel);
/* NOTE: a NULL or zero entry server list is considered valid due to /* NOTE: a NULL or zero entry server list is considered valid due to
* real-world people needing support for this for their test harnesses * real-world people needing support for this for their test harnesses
*/ */
@ -773,7 +771,6 @@ ares_status_t ares__servers_update(ares_channel_t *channel,
status = ARES_SUCCESS; status = ARES_SUCCESS;
done: done:
ares__channel_unlock(channel);
return status; return status;
} }
@ -1044,8 +1041,9 @@ int ares_set_servers(ares_channel_t *channel,
return (int)status; return (int)status;
} }
/* NOTE: lock is in ares__servers_update() */ ares__channel_lock(channel);
status = ares__servers_update(channel, slist, ARES_TRUE); status = ares__servers_update(channel, slist, ARES_TRUE);
ares__channel_unlock(channel);
ares__llist_destroy(slist); ares__llist_destroy(slist);
@ -1067,8 +1065,9 @@ int ares_set_servers_ports(ares_channel_t *channel,
return (int)status; return (int)status;
} }
/* NOTE: lock is in ares__servers_update() */ ares__channel_lock(channel);
status = ares__servers_update(channel, slist, ARES_TRUE); status = ares__servers_update(channel, slist, ARES_TRUE);
ares__channel_unlock(channel);
ares__llist_destroy(slist); ares__llist_destroy(slist);
@ -1086,8 +1085,6 @@ static ares_status_t set_servers_csv(ares_channel_t *channel, const char *_csv)
return ARES_ENODATA; return ARES_ENODATA;
} }
/* NOTE: lock is in ares__servers_update() */
if (ares_strlen(_csv) == 0) { if (ares_strlen(_csv) == 0) {
/* blank all servers */ /* blank all servers */
return (ares_status_t)ares_set_servers_ports(channel, NULL); return (ares_status_t)ares_set_servers_ports(channel, NULL);
@ -1099,8 +1096,9 @@ static ares_status_t set_servers_csv(ares_channel_t *channel, const char *_csv)
return status; return status;
} }
/* NOTE: lock is in ares__servers_update() */ ares__channel_lock(channel);
status = ares__servers_update(channel, slist, ARES_TRUE); status = ares__servers_update(channel, slist, ARES_TRUE);
ares__channel_unlock(channel);
ares__llist_destroy(slist); ares__llist_destroy(slist);
@ -1110,13 +1108,11 @@ static ares_status_t set_servers_csv(ares_channel_t *channel, const char *_csv)
/* We'll go ahead and honor ports anyhow */ /* We'll go ahead and honor ports anyhow */
int ares_set_servers_csv(ares_channel_t *channel, const char *_csv) int ares_set_servers_csv(ares_channel_t *channel, const char *_csv)
{ {
/* NOTE: lock is in ares__servers_update() */
return (int)set_servers_csv(channel, _csv); return (int)set_servers_csv(channel, _csv);
} }
int ares_set_servers_ports_csv(ares_channel_t *channel, const char *_csv) int ares_set_servers_ports_csv(ares_channel_t *channel, const char *_csv)
{ {
/* NOTE: lock is in ares__servers_update() */
return (int)set_servers_csv(channel, _csv); return (int)set_servers_csv(channel, _csv);
} }

Loading…
Cancel
Save