Update for certified/13.13-cert8

AST-2017-013: chan_skinny: Call pthread_detach when sess threads end
chan_skinny creates a new thread for each new session. In trying to be a good cleanup citizen, the threads are joinable and the unload_module function does a pthread_cancel() and a pthread_join() on any sessions that are active at that time. This has an unintended side effect though. Since you can call pthread_join on a thread that's already terminated, pthreads keeps the thread's storage around until you explicitly call pthread_join (or pthread_detach()). Since only the module_unload function was calling pthread_join, and even then only on the ones active at the tme, the storage for every thread/session ever created sticks around until asterisk exits. * A thread can detach itself so the session_destroy() function now calls pthread_detach() just before it frees the session memory allocation. The module_unload function still takes care of the ones that are still active should the module be unloaded. ASTERISK-27452 Reported by: Juan Sacco Change-Id: I9af7268eba14bf76960566f891320f97b974e6dd
2025-11-07 10:28:32 +00:00 · 2017-12-01 14:42:21 -05:00 · 2017-12-01 13:12:35 -06:00 · 2017-11-14 06:31:09 -04:00
7 changed files with 121 additions and 73 deletions
--- a/.version
+++ b/.version
@@ -1 +1 @@
-certified/13.13-cert7
+certified/13.13-cert8
--- a/52
+++ b/52
@@ -1,3 +1,55 @@
+2017-12-01 19:42 +0000  Asterisk Development Team <asteriskteam@digium.com>
+
+	* asterisk certified/13.13-cert8 Released.
+
+2017-11-30 14:38 +0000 [efeb9da0e7]  George Joseph <gjoseph@digium.com>
+
+	* AST-2017-013: chan_skinny: Call pthread_detach when sess threads end
+
+	  chan_skinny creates a new thread for each new session.  In trying
+	  to be a good cleanup citizen, the threads are joinable and the
+	  unload_module function does a pthread_cancel() and a pthread_join()
+	  on any sessions that are active at that time.  This has an
+	  unintended side effect though. Since you can call pthread_join on a
+	  thread that's already terminated, pthreads keeps the thread's
+	  storage around until you explicitly call pthread_join (or
+	  pthread_detach()).   Since only the module_unload function was
+	  calling pthread_join, and even then only on the ones active at the
+	  tme, the storage for every thread/session ever created sticks
+	  around until asterisk exits.
+
+	  * A thread can detach itself so the session_destroy() function
+	    now calls pthread_detach() just before it frees the session
+	    memory allocation.  The module_unload function still takes care
+	    of the ones that are still active should the module be unloaded.
+
+	  ASTERISK-27452
+	  Reported by: Juan Sacco
+
+	  Change-Id: I9af7268eba14bf76960566f891320f97b974e6dd
+
+2017-11-10 07:06 +0000 [191190a982]  Joshua Colp <jcolp@digium.com>
+
+	* pjsip: Add patch to allow all transports to be destroyed.
+
+	  If a transport is created with the same transport type, source
+	  IP address, and source port as one that already exists the old
+	  transport is moved into a linked list called "tp_list".
+
+	  If this old transport is later shutdown it will not be destroyed
+	  as the process checks whether the transport is valid or not. This
+	  check does not look at the "tp_list" when making the determination
+	  causing the transport to not be destroyed.
+
+	  This change updates the logic to query not just the main storage
+	  method for transports but also the "tp_list".
+
+	  Upstream issue https://trac.pjsip.org/repos/ticket/2061
+
+	  ASTERISK-27411
+
+	  Change-Id: Ic5c2bb60226df0ef1c8851359ed8d4cd64469429
+
 2017-11-08 16:59 +0000  Asterisk Development Team <asteriskteam@digium.com>

 	* asterisk certified/13.13-cert7 Released.
--- a/asterisk-certified-13.13-cert7-summary.html
+++ b/asterisk-certified-13.13-cert7-summary.html
@@ -1,24 +0,0 @@
-<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html xmlns="http://www.w3.org/1999/xhtml"><title>Release Summary - asterisk-certified/13.13-cert7</title><h1 align="center"><a name="top">Release Summary</a></h1><h3 align="center">asterisk-certified/13.13-cert7</h3><h3 align="center">Date: 2017-11-08</h3><h3 align="center">&lt;asteriskteam@digium.com&gt;</h3><hr><h2 align="center">Table of Contents</h2><ol>
-<li><a href="#summary">Summary</a></li>
-<li><a href="#contributors">Contributors</a></li>
-<li><a href="#closed_issues">Closed Issues</a></li>
-<li><a href="#diffstat">Diffstat</a></li>
-</ol><hr><a name="summary"><h2 align="center">Summary</h2></a><center><a href="#top">[Back to Top]</a></center><p>This release has been made to address one or more security vulnerabilities that have been identified. A security advisory document has been published for each vulnerability that includes additional information. Users of versions of Asterisk that are affected are strongly encouraged to review the advisories and determine what action they should take to protect their systems from these issues.</p><p>Security Advisories:</p><ul>
-<li><a href="http://downloads.asterisk.org/pub/security/AST-2017-009,AST-2017-010,AST-2017-011.html">AST-2017-009,AST-2017-010,AST-2017-011</a></li>
-</ul><p>The data in this summary reflects changes that have been made since the previous release, asterisk-certified/13.13-cert6.</p><hr><a name="contributors"><h2 align="center">Contributors</h2></a><center><a href="#top">[Back to Top]</a></center><p>This table lists the people who have submitted code, those that have tested patches, as well as those that reported issues on the issue tracker that were resolved in this release. For coders, the number is how many of their patches (of any size) were committed into this release. For testers, the number is the number of times their name was listed as assisting with testing a patch. Finally, for reporters, the number is the number of issues that they reported that were affected by commits that went into this release.</p><table width="100%" border="0">
-<tr><th width="33%">Coders</th><th width="33%">Testers</th><th width="33%">Reporters</th></tr>
-<tr valign="top"><td width="33%">1 Richard Mudgett <rmudgett@digium.com><br/>1 Kevin Harwell <kharwell@digium.com><br/>1 George Joseph <gjoseph@digium.com><br/></td><td width="33%"><td width="33%">1 Youngsung Kim at LINE Corporation<br/>1 Richard Mudgett <rmudgett@digium.com><br/>1 Kim youngsung <youngsung.kim@linecorp.com><br/>1 Corey Farrell <git@cfware.com><br/></td></tr>
-</table><hr><a name="closed_issues"><h2 align="center">Closed Issues</h2></a><center><a href="#top">[Back to Top]</a></center><p>This is a list of all issues from the issue tracker that were closed by changes that went into this release.</p><h3>Bug</h3><h4>Category: General</h4><a href="https://issues.asterisk.org/jira/browse/ASTERISK-27319">ASTERISK-27319</a>: (Security) Function in PJSIP 2.7 miscalculates the length of an unsigned long variable in 64bit machines<br/>Reported by: Kim youngsung<ul>
-<li><a href="https://code.asterisk.org/code/changelog/asterisk?cs=44f3d85cde0625765e0fac24c2065f5af2402bbf">[44f3d85cde]</a> George Joseph -- AST-2017-009: pjproject: Add validation of numeric header values</li>
-</ul><a href="https://issues.asterisk.org/jira/browse/ASTERISK-27337">ASTERISK-27337</a>: chan_sip: Security vulnerability with client code header (revisited)<br/>Reported by: Richard Mudgett<ul>
-<li><a href="https://code.asterisk.org/code/changelog/asterisk?cs=178b372019b9a324034f0e3d6a34b67d616bd284">[178b372019]</a> Richard Mudgett -- AST-2017-010: Fix cdr_object_update_party_b_userfield_cb() buf overrun</li>
-</ul><br><h4>Category: Resources/res_pjsip</h4><a href="https://issues.asterisk.org/jira/browse/ASTERISK-27345">ASTERISK-27345</a>: res_pjsip_session: RTP instances leak on 488 responses.<br/>Reported by: Corey Farrell<ul>
-<li><a href="https://code.asterisk.org/code/changelog/asterisk?cs=1b31e3c3bd6cf18d6f4a4dbfc2f8c2b28ba6f71c">[1b31e3c3bd]</a> Kevin Harwell -- AST-2017-011 - res_pjsip_session: session leak when a call is rejected</li>
-</ul><br><h4>Category: Resources/res_pjsip_sdp_rtp</h4><a href="https://issues.asterisk.org/jira/browse/ASTERISK-27345">ASTERISK-27345</a>: res_pjsip_session: RTP instances leak on 488 responses.<br/>Reported by: Corey Farrell<ul>
-<li><a href="https://code.asterisk.org/code/changelog/asterisk?cs=1b31e3c3bd6cf18d6f4a4dbfc2f8c2b28ba6f71c">[1b31e3c3bd]</a> Kevin Harwell -- AST-2017-011 - res_pjsip_session: session leak when a call is rejected</li>
-</ul><br><h4>Category: Resources/res_pjsip_session</h4><a href="https://issues.asterisk.org/jira/browse/ASTERISK-27345">ASTERISK-27345</a>: res_pjsip_session: RTP instances leak on 488 responses.<br/>Reported by: Corey Farrell<ul>
-<li><a href="https://code.asterisk.org/code/changelog/asterisk?cs=1b31e3c3bd6cf18d6f4a4dbfc2f8c2b28ba6f71c">[1b31e3c3bd]</a> Kevin Harwell -- AST-2017-011 - res_pjsip_session: session leak when a call is rejected</li>
-</ul><br><hr><a name="diffstat"><h2 align="center">Diffstat Results</h2></a><center><a href="#top">[Back to Top]</a></center><p>This is a summary of the changes to the source code that went into this release that was generated using the diffstat utility.</p><pre>main/cdr.c                                                                                    |    6
-res/res_pjsip_session.c                                                                       |   80
-third-party/pjproject/patches/0090-sip_parser-Add-validity-checking-for-numeric-header-.patch |  834 ++++++++++
-3 files changed, 880 insertions(+), 40 deletions(-)</pre><br></html>
--- a/asterisk-certified-13.13-cert8-summary.html
+++ b/asterisk-certified-13.13-cert8-summary.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html xmlns="http://www.w3.org/1999/xhtml"><title>Release Summary - asterisk-certified/13.13-cert8</title><h1 align="center"><a name="top">Release Summary</a></h1><h3 align="center">asterisk-certified/13.13-cert8</h3><h3 align="center">Date: 2017-12-01</h3><h3 align="center">&lt;asteriskteam@digium.com&gt;</h3><hr><h2 align="center">Table of Contents</h2><ol>
+<li><a href="#summary">Summary</a></li>
+<li><a href="#contributors">Contributors</a></li>
+<li><a href="#closed_issues">Closed Issues</a></li>
+<li><a href="#diffstat">Diffstat</a></li>
+</ol><hr><a name="summary"><h2 align="center">Summary</h2></a><center><a href="#top">[Back to Top]</a></center><p>This release has been made to address one or more security vulnerabilities that have been identified. A security advisory document has been published for each vulnerability that includes additional information. Users of versions of Asterisk that are affected are strongly encouraged to review the advisories and determine what action they should take to protect their systems from these issues.</p><p>Security Advisories:</p><ul>
+<li><a href="http://downloads.asterisk.org/pub/security/AST-2017-013.html">AST-2017-013</a></li>
+</ul><p>The data in this summary reflects changes that have been made since the previous release, asterisk-certified/13.13-cert7.</p><hr><a name="contributors"><h2 align="center">Contributors</h2></a><center><a href="#top">[Back to Top]</a></center><p>This table lists the people who have submitted code, those that have tested patches, as well as those that reported issues on the issue tracker that were resolved in this release. For coders, the number is how many of their patches (of any size) were committed into this release. For testers, the number is the number of times their name was listed as assisting with testing a patch. Finally, for reporters, the number is the number of issues that they reported that were affected by commits that went into this release.</p><table width="100%" border="0">
+<tr><th width="33%">Coders</th><th width="33%">Testers</th><th width="33%">Reporters</th></tr>
+<tr valign="top"><td width="33%">1 Joshua Colp <jcolp@digium.com><br/>1 George Joseph <gjoseph@digium.com><br/></td><td width="33%"><td width="33%">1 Joshua Colp <jcolp@digium.com><br/>1 Juan Sacco<br/>1 George Joseph <gjoseph@digium.com><br/></td></tr>
+</table><hr><a name="closed_issues"><h2 align="center">Closed Issues</h2></a><center><a href="#top">[Back to Top]</a></center><p>This is a list of all issues from the issue tracker that were closed by changes that went into this release.</p><h3>Bug</h3><h4>Category: Channels/chan_skinny</h4><a href="https://issues.asterisk.org/jira/browse/ASTERISK-27452">ASTERISK-27452</a>: Security: chan_skinny:  Memory exhaustion if flooded with unauthenticated requests<br/>Reported by: George Joseph<ul>
+<li><a href="https://code.asterisk.org/code/changelog/asterisk?cs=efeb9da0e762be0782d7b420ead90e03422f548b">[efeb9da0e7]</a> George Joseph -- AST-2017-013: chan_skinny: Call pthread_detach when sess threads end</li>
+</ul><br><h4>Category: Resources/res_pjsip/Bundling</h4><a href="https://issues.asterisk.org/jira/browse/ASTERISK-27411">ASTERISK-27411</a>: pjsip: TCP connections may not be destroyed<br/>Reported by: Joshua Colp<ul>
+<li><a href="https://code.asterisk.org/code/changelog/asterisk?cs=191190a9821e99aaedac9c8e0e4929179329f771">[191190a982]</a> Joshua Colp -- pjsip: Add patch to allow all transports to be destroyed.</li>
+</ul><br><hr><a name="diffstat"><h2 align="center">Diffstat Results</h2></a><center><a href="#top">[Back to Top]</a></center><p>This is a summary of the changes to the source code that went into this release that was generated using the diffstat utility.</p><pre>0 files changed</pre><br></html>
--- a/asterisk-certified-13.13-cert8-summary.txt
+++ b/asterisk-certified-13.13-cert8-summary.txt
@@ -1,8 +1,8 @@
                                Release Summary

-                         asterisk-certified/13.13-cert7
+                         asterisk-certified/13.13-cert8

-                                Date: 2017-11-08
+                                Date: 2017-12-01

                           <asteriskteam@digium.com>

@@ -30,10 +30,10 @@

   Security Advisories:

-     * AST-2017-009,AST-2017-010,AST-2017-011
+     * AST-2017-013

   The data in this summary reflects changes that have been made since the
-   previous release, asterisk-certified/13.13-cert6.
+   previous release, asterisk-certified/13.13-cert7.

     ----------------------------------------------------------------------

@@ -50,11 +50,10 @@
   issues that they reported that were affected by commits that went into
   this release.

-   Coders                   Testers       Reporters                           
-   1 Richard Mudgett                      1 Youngsung Kim at LINE Corporation 
-   1 Kevin Harwell                        1 Richard Mudgett                   
-   1 George Joseph                        1 Kim youngsung                     
-                                          1 Corey Farrell                     
+   Coders                   Testers                  Reporters                
+   1 Joshua Colp                                     1 Joshua Colp            
+   1 George Joseph                                   1 Juan Sacco             
+                                                     1 George Joseph          

     ----------------------------------------------------------------------

@@ -67,39 +66,20 @@

  Bug

-    Category: General
+    Category: Channels/chan_skinny

-   ASTERISK-27319: (Security) Function in PJSIP 2.7 miscalculates the length
-   of an unsigned long variable in 64bit machines
-   Reported by: Kim youngsung
-     * [44f3d85cde] George Joseph -- AST-2017-009: pjproject: Add validation
-       of numeric header values
-   ASTERISK-27337: chan_sip: Security vulnerability with client code header
-   (revisited)
-   Reported by: Richard Mudgett
-     * [178b372019] Richard Mudgett -- AST-2017-010: Fix
-       cdr_object_update_party_b_userfield_cb() buf overrun
+   ASTERISK-27452: Security: chan_skinny: Memory exhaustion if flooded with
+   unauthenticated requests
+   Reported by: George Joseph
+     * [efeb9da0e7] George Joseph -- AST-2017-013: chan_skinny: Call
+       pthread_detach when sess threads end

-    Category: Resources/res_pjsip
+    Category: Resources/res_pjsip/Bundling

-   ASTERISK-27345: res_pjsip_session: RTP instances leak on 488 responses.
-   Reported by: Corey Farrell
-     * [1b31e3c3bd] Kevin Harwell -- AST-2017-011 - res_pjsip_session:
-       session leak when a call is rejected
-
-    Category: Resources/res_pjsip_sdp_rtp
-
-   ASTERISK-27345: res_pjsip_session: RTP instances leak on 488 responses.
-   Reported by: Corey Farrell
-     * [1b31e3c3bd] Kevin Harwell -- AST-2017-011 - res_pjsip_session:
-       session leak when a call is rejected
-
-    Category: Resources/res_pjsip_session
-
-   ASTERISK-27345: res_pjsip_session: RTP instances leak on 488 responses.
-   Reported by: Corey Farrell
-     * [1b31e3c3bd] Kevin Harwell -- AST-2017-011 - res_pjsip_session:
-       session leak when a call is rejected
+   ASTERISK-27411: pjsip: TCP connections may not be destroyed
+   Reported by: Joshua Colp
+     * [191190a982] Joshua Colp -- pjsip: Add patch to allow all transports
+       to be destroyed.

     ----------------------------------------------------------------------

@@ -110,7 +90,4 @@
   This is a summary of the changes to the source code that went into this
   release that was generated using the diffstat utility.

- main/cdr.c                                                                                    |    6
- res/res_pjsip_session.c                                                                       |   80
- third-party/pjproject/patches/0090-sip_parser-Add-validity-checking-for-numeric-header-.patch |  834 ++++++++++
- 3 files changed, 880 insertions(+), 40 deletions(-)
+ 0 files changed
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -7414,6 +7414,11 @@ static void destroy_session(struct skinnysession *s)
 	}
 	ast_mutex_unlock(&s->lock);
 	ast_mutex_destroy(&s->lock);
+
+	if (s->t != AST_PTHREADT_NULL) {
+		pthread_detach(s->t);
+	}
+
 	ast_free(s);
 }

@@ -7500,11 +7505,6 @@ static void *skinny_session(void *data)
 	int eventmessage = 0;
 	struct pollfd fds[1];

-	if (!s) {
-		ast_log(LOG_WARNING, "Bad Skinny Session\n");
-		return 0;
-	}
-
 	ast_log(LOG_NOTICE, "Starting Skinny session from %s\n", ast_inet_ntoa(s->sin.sin_addr));

 	pthread_cleanup_push(skinny_session_cleanup, s);
@@ -7673,6 +7673,7 @@ static void *accept_thread(void *ignore)
 		s->keepalive_timeout_sched = -1;

 		if (ast_pthread_create(&s->t, NULL, skinny_session, s)) {
+			s->t = AST_PTHREADT_NULL;
 			destroy_session(s);
 		}
 	}
--- a/third-party/pjproject/patches/0100-sip_transport-Destroy-transports-not-in-hash.patch
+++ b/third-party/pjproject/patches/0100-sip_transport-Destroy-transports-not-in-hash.patch
@@ -0,0 +1,27 @@
+diff --git a/pjsip/src/pjsip/sip_transport.c b/pjsip/src/pjsip/sip_transport.c
+index e4bec24..a39b56e 100644
+--- a/pjsip/src/pjsip/sip_transport.c
+++ b/pjsip/src/pjsip/sip_transport.c
+@@ -957,7 +957,21 @@ static pj_bool_t is_transport_valid(pjsip_tpmgr *tpmgr, pjsip_transport *tp,
+ 				    const pjsip_transport_key *key,
+ 				    int key_len)
+ {
+-    return (pj_hash_get(tpmgr->table, key, key_len, NULL) == (void*)tp);
+    transport *tp_iter;
+
+    if (pj_hash_get(tpmgr->table, key, key_len, NULL) == (void*)tp) {
+        return PJ_TRUE;
+    }
+
+    tp_iter = tpmgr->tp_list.next;
+    while (tp_iter != &tpmgr->tp_list) {
+        if (tp_iter->tp == tp) {
+            return PJ_TRUE;
+        }
+        tp_iter = tp_iter->next;
+    }
+
+    return PJ_FALSE;
+ }
+ 
+ /*