r346525 | jrose | 2011-11-30 15:10:38 -0600 (Wed, 30 Nov 2011) | 18 lines
Cleaning up chan_sip/tcptls file descriptor closing. This patch attempts to eliminate various possible instances of undefined behavior caused by invoking close/fclose in situations where fclose may have already been issued on a tcptls_session_instance and/or closing file descriptors that don't have a valid index for fd (-1). Thanks for more than a little help from wdoekes. (closes issue ASTERISK-18700) Reported by: Erik Wallin (issue ASTERISK-18345) Reported by: Stephane Cazelas (issue ASTERISK-18342) Reported by: Stephane Chazelas Review: https://reviewboard.asterisk.org/r/1576/ ........ Merged revisions 346564 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 346565 from http://svn.asterisk.org/svn/asterisk/branches/10 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@346566 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
parent
fb4c483eb7
commit
9ef171ffe0
|
@ -2813,14 +2813,7 @@ cleanup:
|
||||||
|
|
||||||
if (tcptls_session) {
|
if (tcptls_session) {
|
||||||
ast_mutex_lock(&tcptls_session->lock);
|
ast_mutex_lock(&tcptls_session->lock);
|
||||||
if (tcptls_session->f) {
|
ast_tcptls_close_session_file(tcptls_session);
|
||||||
fclose(tcptls_session->f);
|
|
||||||
tcptls_session->f = NULL;
|
|
||||||
}
|
|
||||||
if (tcptls_session->fd != -1) {
|
|
||||||
close(tcptls_session->fd);
|
|
||||||
tcptls_session->fd = -1;
|
|
||||||
}
|
|
||||||
tcptls_session->parent = NULL;
|
tcptls_session->parent = NULL;
|
||||||
ast_mutex_unlock(&tcptls_session->lock);
|
ast_mutex_unlock(&tcptls_session->lock);
|
||||||
|
|
||||||
|
@ -26019,8 +26012,8 @@ create_tcptls_session_fail:
|
||||||
ao2_t_ref(ca, -1, "failed to create client, getting rid of client tcptls_session arguments");
|
ao2_t_ref(ca, -1, "failed to create client, getting rid of client tcptls_session arguments");
|
||||||
}
|
}
|
||||||
if (s->tcptls_session) {
|
if (s->tcptls_session) {
|
||||||
close(tcptls_session->fd);
|
ast_tcptls_close_session_file(tcptls_session);
|
||||||
s->fd = tcptls_session->fd = -1;
|
s->fd = -1;
|
||||||
ao2_ref(s->tcptls_session, -1);
|
ao2_ref(s->tcptls_session, -1);
|
||||||
s->tcptls_session = NULL;
|
s->tcptls_session = NULL;
|
||||||
}
|
}
|
||||||
|
|
|
@ -176,6 +176,13 @@ struct ast_tcptls_session_instance *ast_tcptls_client_create(struct ast_tcptls_s
|
||||||
|
|
||||||
void *ast_tcptls_server_root(void *);
|
void *ast_tcptls_server_root(void *);
|
||||||
|
|
||||||
|
/*!
|
||||||
|
* \brief Closes a tcptls session instance's file and/or file descriptor.
|
||||||
|
* The tcptls_session will be set to NULL and it's file descriptor will be set to -1
|
||||||
|
* by this function.
|
||||||
|
*/
|
||||||
|
void ast_tcptls_close_session_file(struct ast_tcptls_session_instance *tcptls_session);
|
||||||
|
|
||||||
/*!
|
/*!
|
||||||
* \brief This is a generic (re)start routine for a TCP server,
|
* \brief This is a generic (re)start routine for a TCP server,
|
||||||
* which does the socket/bind/listen and starts a thread for handling
|
* which does the socket/bind/listen and starts a thread for handling
|
||||||
|
|
|
@ -78,9 +78,23 @@ static HOOK_T ssl_write(void *cookie, const char *buf, LEN_T len)
|
||||||
|
|
||||||
static int ssl_close(void *cookie)
|
static int ssl_close(void *cookie)
|
||||||
{
|
{
|
||||||
close(SSL_get_fd(cookie));
|
int cookie_fd = SSL_get_fd(cookie);
|
||||||
SSL_shutdown(cookie);
|
int ret;
|
||||||
SSL_free(cookie);
|
if (cookie_fd > -1) {
|
||||||
|
/*
|
||||||
|
* According to the TLS standard, it is acceptable for an application to only send its shutdown
|
||||||
|
* alert and then close the underlying connection without waiting for the peer's response (this
|
||||||
|
* way resources can be saved, as the process can already terminate or serve another connection).
|
||||||
|
*/
|
||||||
|
if ((ret = SSL_shutdown(cookie)) < 0) {
|
||||||
|
ast_log(LOG_ERROR, "SSL_shutdown() failed: %d\n", SSL_get_error(cookie, ret));
|
||||||
|
}
|
||||||
|
SSL_free(cookie);
|
||||||
|
/* adding shutdown(2) here has no added benefit */
|
||||||
|
if (close(cookie_fd)) {
|
||||||
|
ast_log(LOG_ERROR, "close() failed: %s\n", strerror(errno));
|
||||||
|
}
|
||||||
|
}
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
#endif /* DO_SSL */
|
#endif /* DO_SSL */
|
||||||
|
@ -145,8 +159,7 @@ static void *handle_tcptls_connection(void *data)
|
||||||
if (!tcptls_session->parent->tls_cfg) {
|
if (!tcptls_session->parent->tls_cfg) {
|
||||||
if ((tcptls_session->f = fdopen(tcptls_session->fd, "w+"))) {
|
if ((tcptls_session->f = fdopen(tcptls_session->fd, "w+"))) {
|
||||||
if(setvbuf(tcptls_session->f, NULL, _IONBF, 0)) {
|
if(setvbuf(tcptls_session->f, NULL, _IONBF, 0)) {
|
||||||
fclose(tcptls_session->f);
|
ast_tcptls_close_session_file(tcptls_session);
|
||||||
tcptls_session->f = NULL;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -212,8 +225,7 @@ static void *handle_tcptls_connection(void *data)
|
||||||
if (peer) {
|
if (peer) {
|
||||||
X509_free(peer);
|
X509_free(peer);
|
||||||
}
|
}
|
||||||
close(tcptls_session->fd);
|
ast_tcptls_close_session_file(tcptls_session);
|
||||||
fclose(tcptls_session->f);
|
|
||||||
ao2_ref(tcptls_session, -1);
|
ao2_ref(tcptls_session, -1);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
@ -226,11 +238,11 @@ static void *handle_tcptls_connection(void *data)
|
||||||
if (!tcptls_session->f) { /* no success opening descriptor stacking */
|
if (!tcptls_session->f) { /* no success opening descriptor stacking */
|
||||||
SSL_free(tcptls_session->ssl);
|
SSL_free(tcptls_session->ssl);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
#endif /* DO_SSL */
|
#endif /* DO_SSL */
|
||||||
|
|
||||||
if (!tcptls_session->f) {
|
if (!tcptls_session->f) {
|
||||||
close(tcptls_session->fd);
|
ast_tcptls_close_session_file(tcptls_session);
|
||||||
ast_log(LOG_WARNING, "FILE * open failed!\n");
|
ast_log(LOG_WARNING, "FILE * open failed!\n");
|
||||||
#ifndef DO_SSL
|
#ifndef DO_SSL
|
||||||
if (tcptls_session->parent->tls_cfg) {
|
if (tcptls_session->parent->tls_cfg) {
|
||||||
|
@ -276,7 +288,9 @@ void *ast_tcptls_server_root(void *data)
|
||||||
tcptls_session = ao2_alloc(sizeof(*tcptls_session), session_instance_destructor);
|
tcptls_session = ao2_alloc(sizeof(*tcptls_session), session_instance_destructor);
|
||||||
if (!tcptls_session) {
|
if (!tcptls_session) {
|
||||||
ast_log(LOG_WARNING, "No memory for new session: %s\n", strerror(errno));
|
ast_log(LOG_WARNING, "No memory for new session: %s\n", strerror(errno));
|
||||||
close(fd);
|
if (close(fd)) {
|
||||||
|
ast_log(LOG_ERROR, "close() failed: %s\n", strerror(errno));
|
||||||
|
}
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -293,7 +307,7 @@ void *ast_tcptls_server_root(void *data)
|
||||||
/* This thread is now the only place that controls the single ref to tcptls_session */
|
/* This thread is now the only place that controls the single ref to tcptls_session */
|
||||||
if (ast_pthread_create_detached_background(&launched, NULL, handle_tcptls_connection, tcptls_session)) {
|
if (ast_pthread_create_detached_background(&launched, NULL, handle_tcptls_connection, tcptls_session)) {
|
||||||
ast_log(LOG_WARNING, "Unable to launch helper thread: %s\n", strerror(errno));
|
ast_log(LOG_WARNING, "Unable to launch helper thread: %s\n", strerror(errno));
|
||||||
close(tcptls_session->fd);
|
ast_tcptls_close_session_file(tcptls_session);
|
||||||
ao2_ref(tcptls_session, -1);
|
ao2_ref(tcptls_session, -1);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -558,6 +572,24 @@ error:
|
||||||
desc->accept_fd = -1;
|
desc->accept_fd = -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void ast_tcptls_close_session_file(struct ast_tcptls_session_instance *tcptls_session)
|
||||||
|
{
|
||||||
|
if (tcptls_session->f) {
|
||||||
|
if (fclose(tcptls_session->f)) {
|
||||||
|
ast_log(LOG_ERROR, "fclose() failed: %s\n", strerror(errno));
|
||||||
|
}
|
||||||
|
tcptls_session->f = NULL;
|
||||||
|
tcptls_session->fd = -1;
|
||||||
|
} else if (tcptls_session->fd != -1) {
|
||||||
|
if (close(tcptls_session->fd)) {
|
||||||
|
ast_log(LOG_ERROR, "close() failed: %s\n", strerror(errno));
|
||||||
|
}
|
||||||
|
tcptls_session->fd = -1;
|
||||||
|
} else {
|
||||||
|
ast_log(LOG_ERROR, "ast_tcptls_close_session_file invoked on session instance without file or file descriptor\n");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
void ast_tcptls_server_stop(struct ast_tcptls_session_args *desc)
|
void ast_tcptls_server_stop(struct ast_tcptls_session_args *desc)
|
||||||
{
|
{
|
||||||
if (desc->master != AST_PTHREADT_NULL) {
|
if (desc->master != AST_PTHREADT_NULL) {
|
||||||
|
|
Loading…
Reference in New Issue