[Asterisk-code-review] chan sip: Fix segfault during module unload (asterisk[14])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4495 )

Change subject: chan_sip: Fix segfault during module unload
......................................................................


chan_sip: Fix segfault during module unload

If a TCP/TLS connection was pending (not accepted and not timed out) during
unload of chan_sip, Asterisk would segfault when trying to send a signal to
a thread whose thread ID hadn't been recorded yet. This commit fixes that by
recording the thread ID before calling the blocking connect() syscall.
This was a regression introduced by 776a14386a55b5425c7e9617eff8af8b45427144.

The above wasn't enough to fix the segfault, which was now delayed to the
point where connect() timed out. Therefore, it was necessary to also remove
the SA_RESTART flag from the SIGURG sigaction so that pthread_kill() could be
used to interruput the connect() syscall.
This was a regression introduced by 5d313f51b982a18f7321adcf7c7a4e822d8b2714.

ASTERISK-26586 #close

Change-Id: I76fd9d47d56e4264e2629bce8ec15fecba673e7b
---
M channels/chan_sip.c
M main/asterisk.c
2 files changed, 8 insertions(+), 4 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 297981a..21828c5 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -2964,6 +2964,7 @@
 		if (!(me = sip_threadinfo_create(tcptls_session, tcptls_session->ssl ? AST_TRANSPORT_TLS : AST_TRANSPORT_TCP))) {
 			goto cleanup;
 		}
+		me->threadid = pthread_self();
 		ao2_t_ref(me, +1, "Adding threadinfo ref for tcp_helper_thread");
 	} else {
 		struct sip_threadinfo tmp = {
@@ -2971,8 +2972,13 @@
 		};
 
 		if ((!(ca = tcptls_session->parent)) ||
-			(!(me = ao2_t_find(threadt, &tmp, OBJ_POINTER, "ao2_find, getting sip_threadinfo in tcp helper thread"))) ||
-			(!(tcptls_session = ast_tcptls_client_start(tcptls_session)))) {
+			(!(me = ao2_t_find(threadt, &tmp, OBJ_POINTER, "ao2_find, getting sip_threadinfo in tcp helper thread")))) {
+			goto cleanup;
+		}
+
+		me->threadid = pthread_self();
+
+		if (!(tcptls_session = ast_tcptls_client_start(tcptls_session))) {
 			goto cleanup;
 		}
 	}
@@ -2983,7 +2989,6 @@
 		goto cleanup;
 	}
 
-	me->threadid = pthread_self();
 	ast_debug(2, "Starting thread for %s server\n", tcptls_session->ssl ? "TLS" : "TCP");
 
 	/* set up pollfd to watch for reads on both the socket and the alert_pipe */
diff --git a/main/asterisk.c b/main/asterisk.c
index 54f988c..6c7a57f 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -1714,7 +1714,6 @@
 
 static struct sigaction urg_handler = {
 	.sa_handler = _urg_handler,
-	.sa_flags = SA_RESTART,
 };
 
 static void _hup_handler(int num)

-- 
To view, visit https://gerrit.asterisk.org/4495
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I76fd9d47d56e4264e2629bce8ec15fecba673e7b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Michael Kuron <m.kuron at gmx.de>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>