fix: eliminated a possible deadlock in logging routines, fix: better handling of strange usernames in SIP URIs when comparing.

This commit is contained in:
Thomas Ries 2020-07-07 22:05:29 +00:00
parent 4ab0c81cbe
commit d5366ee8dd
5 changed files with 80 additions and 18 deletions

View File

@ -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

View File

@ -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);

View File

@ -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[];

View File

@ -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);

View File

@ -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;
}
}