sysmocom
/
asterisk
11
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bundled pjproject: Crashes while resolving DNS names. 

PJPROJECT 2.5.5 introduced a race condition with the -r5349 IPv6 DNS
patch.

The patches below fix the DNS lookup race condition crash caused by
attempting to send the same message twice for the single DNS lookup.

0006-r5471-svn-backport-Various-fixes-for-DNS-IPv6.patch
0006-r5473-svn-backport-Fix-pending-query.patch

The patch below removes a cached DNS response from the hash table when
another thread is referencing the old entry.  The table still contained
the entry when it was destroyed which can result in inexplicable crashes.

0006-r5475-svn-backport-Remove-DNS-cache-entry.patch

ASTERISK-26344 #close
Reported by: Ian Gilmour

ASTERISK-26387 #close
Reported by: Harley Peters

Change-Id: I17fde80359e66f65a91341ceca58d914d0f61cc4
This commit is contained in:
Richard Mudgett 2016-10-28 14:55:08 -05:00
parent 2995b31d18
commit 6feee22e09
3 changed files with 232 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

134
third-party/pjproject/patches/0006-r5471-svn-backport-Various-fixes-for-DNS-IPv6.patch vendored Normal file
View File

@ -0,0 +1,134 @@
From 2ab7a9f67caf73be3f2215473f72882cfaef4972 Mon Sep 17 00:00:00 2001
From: Richard Mudgett <rmudgett@digium.com>
Date: Fri, 28 Oct 2016 12:11:30 -0500
Subject: [PATCH 1/3] r5471 svn backport Various fixes for DNS IPv6
Fixed #1974: Various fixes for DNS IPv6
---
pjlib-util/src/pjlib-util/resolver.c | 11 +++++------
pjlib-util/src/pjlib-util/srv_resolver.c | 17 +++++++++++++++--
pjsip/src/pjsip/sip_resolve.c | 14 +++++++-------
3 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/pjlib-util/src/pjlib-util/resolver.c b/pjlib-util/src/pjlib-util/resolver.c
index e5e1bed..d24ef9d 100644
--- a/pjlib-util/src/pjlib-util/resolver.c
+++ b/pjlib-util/src/pjlib-util/resolver.c
@@ -835,7 +835,7 @@ PJ_DEF(pj_status_t) pj_dns_resolver_start_query( pj_dns_resolver *resolver,
pj_time_val now;
struct res_key key;
struct cached_res *cache;
- pj_dns_async_query *q;
+ pj_dns_async_query *q, *p_q = NULL;
pj_uint32_t hval;
pj_status_t status = PJ_SUCCESS;
@@ -849,9 +849,6 @@ PJ_DEF(pj_status_t) pj_dns_resolver_start_query( pj_dns_resolver *resolver,
/* Check type */
PJ_ASSERT_RETURN(type > 0 && type < 0xFFFF, PJ_EINVAL);
- if (p_query)
- *p_query = NULL;
-
/* Build resource key for looking up hash tables */
init_res_key(&key, type, name);
@@ -970,10 +967,12 @@ PJ_DEF(pj_status_t) pj_dns_resolver_start_query( pj_dns_resolver *resolver,
pj_hash_set_np(resolver->hquerybyres, &q->key, sizeof(q->key),
0, q->hbufkey, q);
- if (p_query)
- *p_query = q;
+ p_q = q;
on_return:
+ if (p_query)
+ *p_query = p_q;
+
pj_mutex_unlock(resolver->mutex);
return status;
}
diff --git a/pjlib-util/src/pjlib-util/srv_resolver.c b/pjlib-util/src/pjlib-util/srv_resolver.c
index 02672aa..ff9c979 100644
--- a/pjlib-util/src/pjlib-util/srv_resolver.c
+++ b/pjlib-util/src/pjlib-util/srv_resolver.c
@@ -187,9 +187,12 @@ PJ_DEF(pj_status_t) pj_dns_srv_cancel_query(pj_dns_srv_async_query *query,
has_pending = PJ_TRUE;
}
if (srv->q_aaaa) {
- pj_dns_resolver_cancel_query(srv->q_aaaa, PJ_FALSE);
+ /* Check if it is a dummy query. */
+ if (srv->q_aaaa != (pj_dns_async_query*)0x1) {
+ pj_dns_resolver_cancel_query(srv->q_aaaa, PJ_FALSE);
+ has_pending = PJ_TRUE;
+ }
srv->q_aaaa = NULL;
- has_pending = PJ_TRUE;
}
}
@@ -485,12 +488,22 @@ static pj_status_t resolve_hostnames(pj_dns_srv_async_query *query_job)
srv->common.type = PJ_DNS_TYPE_A;
srv->common_aaaa.type = PJ_DNS_TYPE_AAAA;
srv->parent = query_job;
+ srv->q_a = NULL;
+ srv->q_aaaa = NULL;
status = PJ_SUCCESS;
/* Start DNA A record query */
if ((query_job->option & PJ_DNS_SRV_RESOLVE_AAAA_ONLY) == 0)
{
+ if ((query_job->option & PJ_DNS_SRV_RESOLVE_AAAA) != 0) {
+ /* If there will be DNS AAAA query too, let's setup
+ * a dummy one here, otherwise app callback may be called
+ * immediately (before DNS AAAA query is sent) when
+ * DNS A record is available in the cache.
+ */
+ srv->q_aaaa = (pj_dns_async_query*)0x1;
+ }
status = pj_dns_resolver_start_query(query_job->resolver,
&srv->target_name,
PJ_DNS_TYPE_A, 0,
diff --git a/pjsip/src/pjsip/sip_resolve.c b/pjsip/src/pjsip/sip_resolve.c
index ed326ba..3f3654d 100644
--- a/pjsip/src/pjsip/sip_resolve.c
+++ b/pjsip/src/pjsip/sip_resolve.c
@@ -452,7 +452,7 @@ PJ_DEF(void) pjsip_resolve( pjsip_resolver_t *resolver,
}
/* Resolve DNS AAAA record if address family is not fixed to IPv4 */
- if (af != pj_AF_INET()) {
+ if (af != pj_AF_INET() && status == PJ_SUCCESS) {
status = pj_dns_resolver_start_query(resolver->res,
&query->naptr[0].name,
PJ_DNS_TYPE_AAAA, 0,
@@ -530,9 +530,9 @@ static void dns_a_callback(void *user_data,
++srv->count;
}
-
- } else {
-
+ }
+
+ if (status != PJ_SUCCESS) {
char errmsg[PJ_ERR_MSG_SIZE];
/* Log error */
@@ -593,9 +593,9 @@ static void dns_aaaa_callback(void *user_data,
++srv->count;
}
-
- } else {
-
+ }
+
+ if (status != PJ_SUCCESS) {
char errmsg[PJ_ERR_MSG_SIZE];
/* Log error */
--
1.7.9.5

28
third-party/pjproject/patches/0006-r5473-svn-backport-Fix-pending-query.patch vendored Normal file
View File

@ -0,0 +1,28 @@
From 509d4339747f11cfbde3a0acc447ef5d521eea93 Mon Sep 17 00:00:00 2001
From: Richard Mudgett <rmudgett@digium.com>
Date: Fri, 28 Oct 2016 12:12:28 -0500
Subject: [PATCH 2/3] r5473 svn backport Fix pending query
Re #1974:
If there is a pending query, set the return value to that query (instead of NULL)
Thanks to Richard Mudgett for the patch.
---
pjlib-util/src/pjlib-util/resolver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/pjlib-util/src/pjlib-util/resolver.c b/pjlib-util/src/pjlib-util/resolver.c
index d24ef9d..fe687b7 100644
--- a/pjlib-util/src/pjlib-util/resolver.c
+++ b/pjlib-util/src/pjlib-util/resolver.c
@@ -940,6 +940,7 @@ PJ_DEF(pj_status_t) pj_dns_resolver_start_query( pj_dns_resolver *resolver,
/* Done. This child query will be notified once the "parent"
* query completes.
*/
+ p_q = nq;
status = PJ_SUCCESS;
goto on_return;
}
--
1.7.9.5

70
third-party/pjproject/patches/0006-r5475-svn-backport-Remove-DNS-cache-entry.patch vendored Normal file
View File

@ -0,0 +1,70 @@
From 46e1cfa18853a38b7fcdebad782710c5db676657 Mon Sep 17 00:00:00 2001
From: Richard Mudgett <rmudgett@digium.com>
Date: Fri, 28 Oct 2016 12:15:44 -0500
Subject: [PATCH 3/3] r5475 svn backport Remove DNS cache entry
Re #1974: Remove DNS cache entry from resolver's hash table when app callback has a reference.
Thanks to Richard Mudgett for the patch.
---
pjlib-util/src/pjlib-util/resolver.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/pjlib-util/src/pjlib-util/resolver.c b/pjlib-util/src/pjlib-util/resolver.c
index fe687b7..52b7655 100644
--- a/pjlib-util/src/pjlib-util/resolver.c
+++ b/pjlib-util/src/pjlib-util/resolver.c
@@ -1444,10 +1444,12 @@ static void update_res_cache(pj_dns_resolver *resolver,
if (ttl > resolver->settings.cache_max_ttl)
ttl = resolver->settings.cache_max_ttl;
+ /* Get a cache response entry */
+ cache = (struct cached_res *) pj_hash_get(resolver->hrescache, key,
+ sizeof(*key), &hval);
+
/* If TTL is zero, clear the same entry in the hash table */
if (ttl == 0) {
- cache = (struct cached_res *) pj_hash_get(resolver->hrescache, key,
- sizeof(*key), &hval);
/* Remove the entry before releasing its pool (see ticket #1710) */
pj_hash_set(NULL, resolver->hrescache, key, sizeof(*key), hval, NULL);
@@ -1457,24 +1459,23 @@ static void update_res_cache(pj_dns_resolver *resolver,
return;
}
- /* Get a cache response entry */
- cache = (struct cached_res *) pj_hash_get(resolver->hrescache, key,
- sizeof(*key), &hval);
if (cache == NULL) {
cache = alloc_entry(resolver);
- } else if (cache->ref_cnt > 1) {
- /* When cache entry is being used by callback (to app), just decrement
- * ref_cnt so it will be freed after the callback returns and allocate
- * new entry.
- */
- cache->ref_cnt--;
- cache = alloc_entry(resolver);
} else {
/* Remove the entry before resetting its pool (see ticket #1710) */
pj_hash_set(NULL, resolver->hrescache, key, sizeof(*key), hval, NULL);
- /* Reset cache to avoid bloated cache pool */
- reset_entry(&cache);
+ if (cache->ref_cnt > 1) {
+ /* When cache entry is being used by callback (to app),
+ * just decrement ref_cnt so it will be freed after
+ * the callback returns and allocate new entry.
+ */
+ cache->ref_cnt--;
+ cache = alloc_entry(resolver);
+ } else {
+ /* Reset cache to avoid bloated cache pool */
+ reset_entry(&cache);
+ }
}
/* Duplicate the packet.
--
1.7.9.5