diff --git a/ChangeLog b/ChangeLog index e63b36a..94bedb0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 0.8.3dev ======== + 10-Jun-2020: - fix: eliminated a possible deadlock in logging routines. + 04-Jun-2020: - fix: better handling of strange usernames in SIP URIs + when comparing. 09-Nov-2019: - fix: Repetitions (T1 timer) during INVITE could cause loss of audio 10-May-2019: - fix: TCP fragment reassembly fails diff --git a/src/log.c b/src/log.c index b6b25bd..197a6a6 100644 --- a/src/log.c +++ b/src/log.c @@ -74,10 +74,17 @@ static int silence_level=1; * * use a 'fast' mutex for synchronizing - as these are portable... */ -static pthread_mutex_t log_mutex = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutex_t log_mutex; //&&& = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutexattr_t log_mutex_attr; void log_init(void) { openlog("siproxd", LOG_NDELAY|LOG_PID, LOG_DAEMON); + pthread_mutexattr_init(&log_mutex_attr); + // need a recussive mutex, as logging itself does call ERROR/WARN/INFO while + // the mutex is locked by this thread. + // Otherwise we may end in a deadlock. + pthread_mutexattr_settype(&log_mutex_attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init(&log_mutex, &log_mutex_attr); } void log_end(void) { @@ -187,9 +194,10 @@ void log_tcp_connect(void) { sts=select(debug_listen_fd+1, &fdset, NULL, NULL, &timeout); if (sts > 0) { if (debug_fd != 0) { - tmpfd=accept(debug_listen_fd, NULL, NULL); - close(tmpfd); - INFO("Rejected DEBUG TCP connection"); + while ((tmpfd=accept(debug_listen_fd, NULL, NULL)>=0)) { + close(tmpfd); + INFO("Rejected DEBUG TCP connection"); + } } else { debug_fd=accept(debug_listen_fd, NULL, NULL); INFO("Accepted DEBUG TCP connection [fd=%i], debugpattern=%i", @@ -281,12 +289,27 @@ static void output_to_TCP(const char *label, va_list ap, char *file, tim->tm_hour, tim->tm_min, tim->tm_sec, (int)(tv.tv_usec/1000), label, file, line); sts=write(debug_fd, outbuf, strlen(outbuf)); + if (sts < 0) { + // assume TCP debug connection has been aborted in some bad way + close(debug_fd); + debug_fd=0; + } va_copy(ap_copy, ap); vsnprintf(outbuf, sizeof(outbuf), format, ap_copy); va_end(ap_copy); sts=write(debug_fd, outbuf, strlen(outbuf)); + if (sts < 0) { + // assume TCP debug connection has been aborted in some bad way + close(debug_fd); + debug_fd=0; + } snprintf(outbuf, sizeof(outbuf), "\n"); sts=write(debug_fd, outbuf, strlen(outbuf)); + if (sts < 0) { + // assume TCP debug connection has been aborted in some bad way + close(debug_fd); + debug_fd=0; + } return; } @@ -385,6 +408,9 @@ void log_dump_buffer(unsigned int class, char *file, int line, sts=write(debug_fd, outbuf, strlen(outbuf)); if (sts < 0) { ERROR("write returned error [%i:%s]",errno, strerror(errno)); + // assume TCP debug connection has been aborted in some bad way + close(debug_fd); + debug_fd=0; } } @@ -406,6 +432,9 @@ void log_dump_buffer(unsigned int class, char *file, int line, sts=write(debug_fd, outbuf, strlen(outbuf)); if (sts < 0) { ERROR("write returned error [%i:%s]",errno, strerror(errno)); + // assume TCP debug connection has been aborted in some bad way + close(debug_fd); + debug_fd=0; } } } @@ -419,6 +448,9 @@ void log_dump_buffer(unsigned int class, char *file, int line, sts=write(debug_fd, outbuf, strlen(outbuf)); if (sts < 0) { ERROR("write returned error [%i:%s]",errno, strerror(errno)); + // assume TCP debug connection has been aborted in some bad way + close(debug_fd); + debug_fd=0; } } pthread_mutex_unlock(&log_mutex); diff --git a/src/plugin_stats.c b/src/plugin_stats.c index 2156999..6d7b810 100644 --- a/src/plugin_stats.c +++ b/src/plugin_stats.c @@ -56,6 +56,11 @@ static char desc[]="Upon receiving SIGUSR1, dump some call statistics"; /* global configuration storage - required for config file location */ extern struct siproxd_config configuration; /* need access to the proxy table */ +/*&&& should not do that. Instead, should get a copy/clone of the proxytable + (lock mutex, clone, unlock mutex) and work with this copy. + Avoids a possible race condition if RTP thread starts/stops + a stream during stats dump. +*/ extern rtp_proxytable_t rtp_proxytable[]; extern struct urlmap_s urlmap[]; diff --git a/src/sip_utils.c b/src/sip_utils.c index 0351b9f..cd58540 100644 --- a/src/sip_utils.c +++ b/src/sip_utils.c @@ -290,19 +290,28 @@ int compare_url(osip_uri_t *url1, osip_uri_t *url2) { WARN("compare_url: NULL scheme - ignoring"); } - /* compare username (if present) case sensitive */ + /* + * first compare the username, avoid unneccesaery DNS lookups + * compare username + * - if present on both: must match case sensitive + * - if present only on one side -> failure (mismatch) + * - if not present (NULL) on both sides: -> match + */ if (url1->username && url2->username) { if (strcmp(url1->username, url2->username) != 0) { -// DEBUGC(DBCLASS_BABBLE, "compare_url: username mismatch"); +// DEBUGC(DBCLASS_BABBLE, "compare_url: username mismatch [%s <-> %s]", +// url1->username, url2->username); return STS_FAILURE; } + } else if (! ((url1->username==NULL) && (url2->username==NULL))) { +// DEBUGC(DBCLASS_BABBLE, "compare_url: username mismatch (NULL/not-NULL"); + return STS_FAILURE; } else { -// DEBUGC(DBCLASS_BABBLE, "compare_url: NULL username - ignoring"); +// DEBUGC(DBCLASS_BABBLE, "compare_url: both NULL username - consider it a match"); } - /* - * now, try to resolve the host. If resolveable, compare + * finally, try to resolve the host. If resolveable, compare * IP addresses - if not resolveable, compare the host names * itselfes */ @@ -323,13 +332,13 @@ int compare_url(osip_uri_t *url1, osip_uri_t *url2) { if ((sts1 == STS_SUCCESS) && (sts2 == STS_SUCCESS)) { /* compare IP addresses */ if (memcmp(&addr1, &addr2, sizeof(addr1))!=0) { - DEBUGC(DBCLASS_BABBLE, "compare_url: IP mismatch"); +// DEBUGC(DBCLASS_BABBLE, "compare_url: IP mismatch"); return STS_FAILURE; } } else { /* compare hostname strings case INsensitive */ if (osip_strcasecmp(url1->host, url2->host) != 0) { - DEBUGC(DBCLASS_BABBLE, "compare_url: host name mismatch"); +// DEBUGC(DBCLASS_BABBLE, "compare_url: host name mismatch"); return STS_FAILURE; } } @@ -1063,6 +1072,9 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { } else { type=RESTYP_OUTGOING; } + DEBUGC(DBCLASS_SIP, "sip_find_direction: found internal client, " + "type=%i, ip=%s", + type, utils_inet_ntoa(from->sin_addr)); break; } } @@ -1104,6 +1116,7 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { (!MSG_IS_REGISTER(request) && (compare_url(request->to->url, urlmap[i].reg_url)==STS_SUCCESS))) { type=REQTYP_INCOMING; + DEBUGC(DBCLASS_SIP, "sip_find_direction: found - incoming request"); break; } } else { @@ -1112,13 +1125,14 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { if ((compare_url(response->from->url, urlmap[i].reg_url)==STS_SUCCESS) || (compare_url(response->from->url, urlmap[i].masq_url)==STS_SUCCESS)) { type=RESTYP_INCOMING; + DEBUGC(DBCLASS_SIP, "sip_find_direction: found - incoming response"); break; } } /* is request */ } /* for i */ } /* if type == DIRTYP_UNKNOWN */ if (type == DIRTYP_UNKNOWN) { - DEBUGC(DBCLASS_SIP, "sip_find_direction: no INCOMING (To:) found"); + DEBUGC(DBCLASS_SIP, "sip_find_direction: no INCOMING (To:/From:) found"); } @@ -1265,11 +1279,17 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { if (i < URLMAP_SIZE) { if (urlidx) *urlidx=i; DEBUGC(DBCLASS_SIP, "sip_find_direction: dir=%i, urlmap %i, " - "trueurl [%s:%s] / masqurl [%s:%s] / regurl [%s:%s]", + "trueurl [%s@%s:%s] / masqurl [%s@%s:%s] / regurl [%s@%s:%s]", type, i, - urlmap[i].true_url->host, urlmap[i].true_url->port, - urlmap[i].masq_url->host, urlmap[i].masq_url->port, - urlmap[i].reg_url->host, urlmap[i].reg_url->port); + (urlmap[i].true_url->username)? urlmap[i].true_url->username : "(null)", + (urlmap[i].true_url->host)? urlmap[i].true_url->host : "(null)", + urlmap[i].true_url->port, + (urlmap[i].masq_url->username)? urlmap[i].masq_url->username : "(null)", + (urlmap[i].masq_url->host)? urlmap[i].masq_url->host : "(null)", + urlmap[i].masq_url->port, + (urlmap[i].reg_url->username)? urlmap[i].reg_url->username : "(null)", + (urlmap[i].reg_url->host)? urlmap[i].reg_url->host : "(null)", + urlmap[i].reg_url->port); } else { if (urlidx) *urlidx=-1; DEBUGC(DBCLASS_SIP, "sip_find_direction: dir=%i, not found in URLMAP", type); diff --git a/src/utils.c b/src/utils.c index d284e4c..55bae75 100644 --- a/src/utils.c +++ b/src/utils.c @@ -138,8 +138,10 @@ int get_ip_by_host(char *hostname, struct in_addr *addr) { idx=i; break; } - DEBUGC(DBCLASS_BABBLE, "DNS lookup - from cache: %s -> %s", - hostname, utils_inet_ntoa(*addr)); +// avoid: causes tremendous logging during URL lookups through +// the urlmap... +// DEBUGC(DBCLASS_BABBLE, "DNS lookup - from cache: %s -> %s", +// hostname, utils_inet_ntoa(*addr)); return STS_SUCCESS; } }