[Asterisk-code-review] pjproject: Fixes to resolve DNS SRV crashes. (asterisk[14])
Joshua Colp
asteriskteam at digium.com
Wed Feb 22 05:53:58 CST 2017
Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/5034 )
Change subject: pjproject: Fixes to resolve DNS SRV crashes.
......................................................................
pjproject: Fixes to resolve DNS SRV crashes.
* Re #1945 (misc): Don't trigger SRV complete callback when there is a
parse error.
* srv_resolver.c: Don't try to send query if already considered resolved.
** In resolve_hostnames() don't try to resolve a query that is already
considered resolved.
** In resolve_hostnames() fix DNS typo in comments.
** In build_server_entries() move a common expression assigning to cnt
earlier.
* sip_transport.c: Fix tdata object name to actually contain the pointer.
It helps if the logs referencing a tdata object buffer actually have a
name that includes the correct pointer as part of the name. Also since
the tdata has its own pool it helps if any logs referencing the pool have
the same name as the tdata object. This change brings tdata logging in
line with how tsx objects are named.
ASTERISK-26669 #close
ASTERISK-26738 #close
Change-Id: I56af2ded25476b3e870ca586ee69ed6954ef75af
---
A third-party/pjproject/patches/0012-Re-1945-misc-Don-t-trigger-SRV-complete-callback-whe.patch
A third-party/pjproject/patches/0013-r5559-svn-backport-Fix-to-resolve-DNS-SRV-crashes.patch
2 files changed, 171 insertions(+), 0 deletions(-)
Approvals:
Mark Michelson: Looks good to me, approved
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, but someone else must approve
diff --git a/third-party/pjproject/patches/0012-Re-1945-misc-Don-t-trigger-SRV-complete-callback-whe.patch b/third-party/pjproject/patches/0012-Re-1945-misc-Don-t-trigger-SRV-complete-callback-whe.patch
new file mode 100644
index 0000000..e65556f
--- /dev/null
+++ b/third-party/pjproject/patches/0012-Re-1945-misc-Don-t-trigger-SRV-complete-callback-whe.patch
@@ -0,0 +1,59 @@
+From 783de8956190c47a70ffefed56a1a2b21a62b235 Mon Sep 17 00:00:00 2001
+From: Riza Sulistyo <riza at teluu.com>
+Date: Mon, 23 Jan 2017 01:34:12 +0000
+Subject: [PATCH 2/5] Re #1945 (misc): Don't trigger SRV complete callback when
+ there is a parse error.
+
+git-svn-id: https://svn.pjsip.org/repos/pjproject/trunk@5536 74dad513-b988-da41-8d7b-12977e46ad98
+---
+ pjlib-util/src/pjlib-util/srv_resolver.c | 24 ++++++++++++++++++------
+ 1 file changed, 18 insertions(+), 6 deletions(-)
+
+diff --git a/pjlib-util/src/pjlib-util/srv_resolver.c b/pjlib-util/src/pjlib-util/srv_resolver.c
+index 8a4a599..8a2f7e1 100644
+--- a/pjlib-util/src/pjlib-util/srv_resolver.c
++++ b/pjlib-util/src/pjlib-util/srv_resolver.c
+@@ -652,6 +652,7 @@ static void dns_callback(void *user_data,
+
+ } else if (query_job->dns_state == PJ_DNS_TYPE_A) {
+ pj_bool_t is_type_a, srv_completed;
++ pj_dns_addr_record rec;
+
+ /* Clear outstanding job */
+ if (common->type == PJ_DNS_TYPE_A) {
+@@ -668,15 +669,26 @@ static void dns_callback(void *user_data,
+
+ is_type_a = (common->type == PJ_DNS_TYPE_A);
+
++ /* Parse response */
++ if (status==PJ_SUCCESS && pkt->hdr.anscount != 0) {
++ status = pj_dns_parse_addr_response(pkt, &rec);
++ if (status!=PJ_SUCCESS) {
++ char errmsg[PJ_ERR_MSG_SIZE];
++
++ PJ_LOG(4,(query_job->objname,
++ "DNS %s record parse error for '%.*s'."
++ " Err=%d (%s)",
++ (is_type_a ? "A" : "AAAA"),
++ (int)query_job->domain_part.slen,
++ query_job->domain_part.ptr,
++ status,
++ pj_strerror(status,errmsg,sizeof(errmsg)).ptr));
++ }
++ }
++
+ /* Check that we really have answer */
+ if (status==PJ_SUCCESS && pkt->hdr.anscount != 0) {
+ char addr[PJ_INET6_ADDRSTRLEN];
+- pj_dns_addr_record rec;
+-
+- /* Parse response */
+- status = pj_dns_parse_addr_response(pkt, &rec);
+- if (status != PJ_SUCCESS)
+- goto on_error;
+
+ pj_assert(rec.addr_count != 0);
+
+--
+2.7.4
+
diff --git a/third-party/pjproject/patches/0013-r5559-svn-backport-Fix-to-resolve-DNS-SRV-crashes.patch b/third-party/pjproject/patches/0013-r5559-svn-backport-Fix-to-resolve-DNS-SRV-crashes.patch
new file mode 100644
index 0000000..dc03cbc
--- /dev/null
+++ b/third-party/pjproject/patches/0013-r5559-svn-backport-Fix-to-resolve-DNS-SRV-crashes.patch
@@ -0,0 +1,112 @@
+From d9d52f005f6d0242ea84e7c59ad6b25f052c8485 Mon Sep 17 00:00:00 2001
+From: Richard Mudgett <rmudgett at digium.com>
+Date: Mon, 20 Feb 2017 12:05:32 -0600
+Subject: [PATCH 3/5] r5559 svn backport Fix to resolve DNS SRV crashes.
+
+Re #1994 (misc): Don't try to resolve a DNS SRV query that is already considered resolved.
+Thanks to Richard Mudgett for the patch.
+
+srv_resolver.c: Don't try to send query if already considered resolved.
+
+* In resolve_hostnames() don't try to resolve a query that is already
+considered resolved.
+
+* In resolve_hostnames() fix DNS typo in comments.
+
+* In build_server_entries() move a common expression assigning to cnt
+earlier.
+
+sip_transport.c: Fix tdata object name to actually contain the pointer.
+
+It helps if the logs referencing a tdata object buffer actually have
+a name that includes the correct pointer as part of the name. Also
+since the tdata has its own pool it helps if any logs referencing the
+pool have the same name as the tdata object. This change brings tdata
+logging in line with how tsx objects are named.
+---
+ pjlib-util/src/pjlib-util/srv_resolver.c | 18 +++++++++++++-----
+ pjsip/src/pjsip/sip_transport.c | 3 ++-
+ 2 files changed, 15 insertions(+), 6 deletions(-)
+
+diff --git a/pjlib-util/src/pjlib-util/srv_resolver.c b/pjlib-util/src/pjlib-util/srv_resolver.c
+index 8a2f7e1..84ad3f6 100644
+--- a/pjlib-util/src/pjlib-util/srv_resolver.c
++++ b/pjlib-util/src/pjlib-util/srv_resolver.c
+@@ -407,8 +407,9 @@ static void build_server_entries(pj_dns_srv_async_query *query_job,
+ for (i=0; i<query_job->srv_cnt; ++i) {
+ pj_in_addr addr;
+ pj_in6_addr addr6;
++ unsigned cnt = query_job->srv[i].addr_cnt;
+
+- if (query_job->srv[i].addr_cnt != 0) {
++ if (cnt != 0) {
+ /* IP address already resolved */
+ continue;
+ }
+@@ -417,7 +418,6 @@ static void build_server_entries(pj_dns_srv_async_query *query_job,
+ pj_inet_pton(pj_AF_INET(), &query_job->srv[i].target_name,
+ &addr) == PJ_SUCCESS)
+ {
+- unsigned cnt = query_job->srv[i].addr_cnt;
+ pj_sockaddr_init(pj_AF_INET(), &query_job->srv[i].addr[cnt],
+ NULL, query_job->srv[i].port);
+ query_job->srv[i].addr[cnt].ipv4.sin_addr = addr;
+@@ -427,7 +427,6 @@ static void build_server_entries(pj_dns_srv_async_query *query_job,
+ pj_inet_pton(pj_AF_INET6(), &query_job->srv[i].target_name,
+ &addr6) == PJ_SUCCESS)
+ {
+- unsigned cnt = query_job->srv[i].addr_cnt;
+ pj_sockaddr_init(pj_AF_INET6(), &query_job->srv[i].addr[cnt],
+ NULL, query_job->srv[i].port);
+ query_job->srv[i].addr[cnt].ipv6.sin6_addr = addr6;
+@@ -480,6 +479,15 @@ static pj_status_t resolve_hostnames(pj_dns_srv_async_query *query_job)
+ for (i=0; i<query_job->srv_cnt; ++i) {
+ struct srv_target *srv = &query_job->srv[i];
+
++ if (srv->addr_cnt != 0) {
++ /*
++ * This query is already counted as resolved because of the
++ * additional records in the SRV response or the target name
++ * is an IP address exception in build_server_entries().
++ */
++ continue;
++ }
++
+ PJ_LOG(5, (query_job->objname,
+ "Starting async DNS A query_job for %.*s",
+ (int)srv->target_name.slen,
+@@ -493,7 +501,7 @@ static pj_status_t resolve_hostnames(pj_dns_srv_async_query *query_job)
+
+ status = PJ_SUCCESS;
+
+- /* Start DNA A record query */
++ /* Start DNS A record query */
+ if ((query_job->option & PJ_DNS_SRV_RESOLVE_AAAA_ONLY) == 0)
+ {
+ if ((query_job->option & PJ_DNS_SRV_RESOLVE_AAAA) != 0) {
+@@ -511,7 +519,7 @@ static pj_status_t resolve_hostnames(pj_dns_srv_async_query *query_job)
+ &srv->common, &srv->q_a);
+ }
+
+- /* Start DNA AAAA record query */
++ /* Start DNS AAAA record query */
+ if (status == PJ_SUCCESS &&
+ (query_job->option & PJ_DNS_SRV_RESOLVE_AAAA) != 0)
+ {
+diff --git a/pjsip/src/pjsip/sip_transport.c b/pjsip/src/pjsip/sip_transport.c
+index d672a6d..6dd14d1 100644
+--- a/pjsip/src/pjsip/sip_transport.c
++++ b/pjsip/src/pjsip/sip_transport.c
+@@ -422,7 +422,8 @@ PJ_DEF(pj_status_t) pjsip_tx_data_create( pjsip_tpmgr *mgr,
+ tdata = PJ_POOL_ZALLOC_T(pool, pjsip_tx_data);
+ tdata->pool = pool;
+ tdata->mgr = mgr;
+- pj_memcpy(tdata->obj_name, pool->obj_name, PJ_MAX_OBJ_NAME);
++ pj_ansi_snprintf(tdata->obj_name, sizeof(tdata->obj_name), "tdta%p", tdata);
++ pj_memcpy(pool->obj_name, tdata->obj_name, sizeof(pool->obj_name));
+
+ status = pj_atomic_create(tdata->pool, 0, &tdata->ref_cnt);
+ if (status != PJ_SUCCESS) {
+--
+2.7.4
+
--
To view, visit https://gerrit.asterisk.org/5034
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I56af2ded25476b3e870ca586ee69ed6954ef75af
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list