res_config_pgsql: Fix thread safety problems

* A missing AST_LIST_UNLOCK() in find_table()

* The ESCAPE_STRING() macro uses pgsqlConn under the hood and we were
  not consistently locking before calling it.

* There were a handful of other places where pgsqlConn was accessed
  directly without appropriate locking.

Change-Id: Iea63f0728f76985a01e95b9912c3c5c6065836ed
This commit is contained in:
Sean Bright
2017-02-23 15:48:53 -05:00
parent fcd4fa8dff
commit 3de68bb492

View File

@@ -270,6 +270,7 @@ static struct tables *find_table(const char *database, const char *orig_tablenam
}
if (database == NULL) {
AST_LIST_UNLOCK(&psql_tables);
return NULL;
}
@@ -411,6 +412,16 @@ static struct ast_variable *realtime_pgsql(const char *database, const char *tab
return NULL;
}
/*
* Must connect to the server before anything else as ESCAPE_STRING()
* uses pgsqlConn
*/
ast_mutex_lock(&pgsql_lock);
if (!pgsql_reconnect(database)) {
ast_mutex_unlock(&pgsql_lock);
return NULL;
}
/* Get the first parameter and first value in our list of passed paramater/value pairs */
if (!field) {
ast_log(LOG_WARNING,
@@ -419,6 +430,7 @@ static struct ast_variable *realtime_pgsql(const char *database, const char *tab
PQfinish(pgsqlConn);
pgsqlConn = NULL;
}
ast_mutex_unlock(&pgsql_lock);
return NULL;
}
@@ -436,6 +448,7 @@ static struct ast_variable *realtime_pgsql(const char *database, const char *tab
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
return NULL;
}
@@ -454,6 +467,7 @@ static struct ast_variable *realtime_pgsql(const char *database, const char *tab
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
return NULL;
}
@@ -461,8 +475,6 @@ static struct ast_variable *realtime_pgsql(const char *database, const char *tab
}
/* We now have our complete statement; Lets connect to the server and execute it. */
ast_mutex_lock(&pgsql_lock);
if (pgsql_exec(database, tablename, ast_str_buffer(sql), &result) != 0) {
ast_mutex_unlock(&pgsql_lock);
return NULL;
@@ -542,6 +554,16 @@ static struct ast_config *realtime_multi_pgsql(const char *database, const char
if (!(cfg = ast_config_new()))
return NULL;
/*
* Must connect to the server before anything else as ESCAPE_STRING()
* uses pgsqlConn
*/
ast_mutex_lock(&pgsql_lock);
if (!pgsql_reconnect(database)) {
ast_mutex_unlock(&pgsql_lock);
return NULL;
}
/* Get the first parameter and first value in our list of passed paramater/value pairs */
if (!field) {
ast_log(LOG_WARNING,
@@ -550,6 +572,7 @@ static struct ast_config *realtime_multi_pgsql(const char *database, const char
PQfinish(pgsqlConn);
pgsqlConn = NULL;
}
ast_mutex_unlock(&pgsql_lock);
ast_config_destroy(cfg);
return NULL;
}
@@ -575,6 +598,7 @@ static struct ast_config *realtime_multi_pgsql(const char *database, const char
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
ast_config_destroy(cfg);
return NULL;
}
@@ -595,6 +619,7 @@ static struct ast_config *realtime_multi_pgsql(const char *database, const char
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
ast_config_destroy(cfg);
return NULL;
}
@@ -606,10 +631,7 @@ static struct ast_config *realtime_multi_pgsql(const char *database, const char
ast_str_append(&sql, 0, " ORDER BY %s", initfield);
}
/* We now have our complete statement; Lets connect to the server and execute it. */
ast_mutex_lock(&pgsql_lock);
if (pgsql_exec(database, table, ast_str_buffer(sql), &result) != 0) {
ast_mutex_unlock(&pgsql_lock);
ast_config_destroy(cfg);
@@ -706,6 +728,16 @@ static int update_pgsql(const char *database, const char *tablename, const char
return -1;
}
/*
* Must connect to the server before anything else as ESCAPE_STRING()
* uses pgsqlConn
*/
ast_mutex_lock(&pgsql_lock);
if (!pgsql_reconnect(database)) {
ast_mutex_unlock(&pgsql_lock);
return -1;
}
/* Get the first parameter and first value in our list of passed paramater/value pairs */
if (!field) {
ast_log(LOG_WARNING,
@@ -714,6 +746,7 @@ static int update_pgsql(const char *database, const char *tablename, const char
PQfinish(pgsqlConn);
pgsqlConn = NULL;
}
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -727,6 +760,7 @@ static int update_pgsql(const char *database, const char *tablename, const char
if (!column) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: Updating on column '%s', but that column does not exist within the table '%s'!\n", field->name, tablename);
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -737,6 +771,7 @@ static int update_pgsql(const char *database, const char *tablename, const char
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -751,6 +786,7 @@ static int update_pgsql(const char *database, const char *tablename, const char
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -762,6 +798,7 @@ static int update_pgsql(const char *database, const char *tablename, const char
ESCAPE_STRING(escapebuf, lookup);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", lookup);
ast_mutex_unlock(&pgsql_lock);
return -1;
}
@@ -770,8 +807,6 @@ static int update_pgsql(const char *database, const char *tablename, const char
ast_debug(1, "PostgreSQL RealTime: Update SQL: %s\n", ast_str_buffer(sql));
/* We now have our complete statement; Lets connect to the server and execute it. */
ast_mutex_lock(&pgsql_lock);
if (pgsql_exec(database, tablename, ast_str_buffer(sql), &result) != 0) {
ast_mutex_unlock(&pgsql_lock);
return -1;
@@ -838,12 +873,23 @@ static int update2_pgsql(const char *database, const char *tablename, const stru
return -1;
}
/*
* Must connect to the server before anything else as ESCAPE_STRING()
* uses pgsqlConn
*/
ast_mutex_lock(&pgsql_lock);
if (!pgsql_reconnect(database)) {
ast_mutex_unlock(&pgsql_lock);
return -1;
}
ast_str_set(&sql, 0, "UPDATE %s SET", tablename);
ast_str_set(&where, 0, " WHERE");
for (field = lookup_fields; field; field = field->next) {
if (!find_column(table, field->name)) {
ast_log(LOG_ERROR, "Attempted to update based on criteria column '%s' (%s@%s), but that column does not exist!\n", field->name, tablename, database);
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -851,6 +897,7 @@ static int update2_pgsql(const char *database, const char *tablename, const stru
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -865,6 +912,7 @@ static int update2_pgsql(const char *database, const char *tablename, const stru
PQfinish(pgsqlConn);
pgsqlConn = NULL;
}
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -881,6 +929,7 @@ static int update2_pgsql(const char *database, const char *tablename, const stru
ESCAPE_STRING(escapebuf, field->value);
if (pgresult) {
ast_log(LOG_ERROR, "PostgreSQL RealTime: detected invalid input: '%s'\n", field->value);
ast_mutex_unlock(&pgsql_lock);
release_table(table);
return -1;
}
@@ -894,7 +943,7 @@ static int update2_pgsql(const char *database, const char *tablename, const stru
ast_debug(1, "PostgreSQL RealTime: Update SQL: %s\n", ast_str_buffer(sql));
/* We now have our complete statement; connect to the server and execute it. */
/* We now have our complete statement; Lets connect to the server and execute it. */
if (pgsql_exec(database, tablename, ast_str_buffer(sql), &result) != 0) {
ast_mutex_unlock(&pgsql_lock);
return -1;
@@ -939,6 +988,16 @@ static int store_pgsql(const char *database, const char *table, const struct ast
return -1;
}
/*
* Must connect to the server before anything else as ESCAPE_STRING()
* uses pgsqlConn
*/
ast_mutex_lock(&pgsql_lock);
if (!pgsql_reconnect(database)) {
ast_mutex_unlock(&pgsql_lock);
return -1;
}
/* Get the first parameter and first value in our list of passed paramater/value pairs */
if (!field) {
ast_log(LOG_WARNING,
@@ -947,12 +1006,6 @@ static int store_pgsql(const char *database, const char *table, const struct ast
PQfinish(pgsqlConn);
pgsqlConn = NULL;
}
return -1;
}
/* Must connect to the server before anything else, as the escape function requires the connection handle.. */
ast_mutex_lock(&pgsql_lock);
if (!pgsql_reconnect(database)) {
ast_mutex_unlock(&pgsql_lock);
return -1;
}
@@ -973,6 +1026,7 @@ static int store_pgsql(const char *database, const char *table, const struct ast
ast_debug(1, "PostgreSQL RealTime: Insert SQL: %s\n", ast_str_buffer(sql1));
/* We now have our complete statement; Lets connect to the server and execute it. */
if (pgsql_exec(database, table, ast_str_buffer(sql1), &result) != 0) {
ast_mutex_unlock(&pgsql_lock);
return -1;
@@ -1016,27 +1070,27 @@ static int destroy_pgsql(const char *database, const char *table, const char *ke
return -1;
}
/* Get the first parameter and first value in our list of passed paramater/value pairs */
/*newparam = va_arg(ap, const char *);
newval = va_arg(ap, const char *);
if (!newparam || !newval) {*/
if (ast_strlen_zero(keyfield) || ast_strlen_zero(lookup)) {
ast_log(LOG_WARNING,
"PostgreSQL RealTime: Realtime destroy requires at least 1 parameter and 1 value to search on.\n");
if (pgsqlConn) {
PQfinish(pgsqlConn);
pgsqlConn = NULL;
};
return -1;
}
/* Must connect to the server before anything else, as the escape function requires the connection handle.. */
/*
* Must connect to the server before anything else as ESCAPE_STRING()
* uses pgsqlConn
*/
ast_mutex_lock(&pgsql_lock);
if (!pgsql_reconnect(database)) {
ast_mutex_unlock(&pgsql_lock);
return -1;
}
/* Get the first parameter and first value in our list of passed paramater/value pairs */
if (ast_strlen_zero(keyfield) || ast_strlen_zero(lookup)) {
ast_log(LOG_WARNING,
"PostgreSQL RealTime: Realtime destroy requires at least 1 parameter and 1 value to search on.\n");
if (pgsqlConn) {
PQfinish(pgsqlConn);
pgsqlConn = NULL;
}
ast_mutex_unlock(&pgsql_lock);
return -1;
}
/* Create the first part of the query using the first parameter/value pairs we just extracted
If there is only 1 set, then we have our query. Otherwise, loop thru the list and concat */
@@ -1052,10 +1106,11 @@ static int destroy_pgsql(const char *database, const char *table, const char *ke
ast_debug(1, "PostgreSQL RealTime: Delete SQL: %s\n", ast_str_buffer(sql));
if (pgsql_exec(database, table, ast_str_buffer(sql), &result) != 0) {
/* We now have our complete statement; Lets connect to the server and execute it. */
if (pgsql_exec(database, table, ast_str_buffer(sql), &result) != 0) {
ast_mutex_unlock(&pgsql_lock);
return -1;
}
return -1;
}
numrows = atoi(PQcmdTuples(result));
ast_mutex_unlock(&pgsql_lock);
@@ -1268,7 +1323,7 @@ static int require_pgsql(const char *database, const char *tablename, va_list ap
ast_debug(1, "About to run ALTER query on table '%s' to add column '%s'\n", tablename, elm);
if (pgsql_exec(database, tablename, ast_str_buffer(sql), &result) != 0) {
ast_mutex_unlock(&pgsql_lock);
ast_mutex_unlock(&pgsql_lock);
return -1;
}
@@ -1395,6 +1450,7 @@ static int parse_config(int is_reload)
ast_mutex_lock(&pgsql_lock);
/* XXX: Why would we do this before we're ready to establish a new connection? */
if (pgsqlConn) {
PQfinish(pgsqlConn);
pgsqlConn = NULL;
@@ -1502,13 +1558,18 @@ static int pgsql_reconnect(const char *database)
/* mutex lock should have been locked before calling this function. */
if (pgsqlConn && PQstatus(pgsqlConn) != CONNECTION_OK) {
if (pgsqlConn) {
if (PQstatus(pgsqlConn) == CONNECTION_OK) {
/* We're good? */
return 1;
}
PQfinish(pgsqlConn);
pgsqlConn = NULL;
}
/* DB password can legitimately be 0-length */
if ((!pgsqlConn) && (!ast_strlen_zero(dbhost) || !ast_strlen_zero(dbsock)) && !ast_strlen_zero(dbuser) && !ast_strlen_zero(my_database)) {
if ((!ast_strlen_zero(dbhost) || !ast_strlen_zero(dbsock)) && !ast_strlen_zero(dbuser) && !ast_strlen_zero(my_database)) {
struct ast_str *conn_info = ast_str_create(128);
if (!conn_info) {
@@ -1608,7 +1669,7 @@ static char *handle_cli_realtime_pgsql_status(struct ast_cli_entry *e, int cmd,
char connection_info[256];
char credentials[100] = "";
char buf[376]; /* 256+100+"Connected to "+" for "+NULL */
int ctimesec = time(NULL) - connect_time;
int is_connected = 0, ctimesec = time(NULL) - connect_time;
switch (cmd) {
case CLI_INIT:
@@ -1634,8 +1695,11 @@ static char *handle_cli_realtime_pgsql_status(struct ast_cli_entry *e, int cmd,
if (!ast_strlen_zero(dbuser))
snprintf(credentials, sizeof(credentials), " with username %s", dbuser);
ast_mutex_lock(&pgsql_lock);
is_connected = (pgsqlConn && PQstatus(pgsqlConn) == CONNECTION_OK);
ast_mutex_unlock(&pgsql_lock);
if (pgsqlConn && PQstatus(pgsqlConn) == CONNECTION_OK) {
if (is_connected) {
snprintf(buf, sizeof(buf), "Connected to %s%s for ", connection_info, credentials);
ast_cli_print_timestr_fromseconds(a->fd, ctimesec, buf);
return CLI_SUCCESS;