diff options
author | Sean Bright <sean.bright@gmail.com> | 2017-02-23 15:48:53 -0500 |
---|---|---|
committer | Sean Bright <sean.bright@gmail.com> | 2017-02-23 14:56:53 -0600 |
commit | 0f4b349d371141316d7237488b57d58c2276ee42 (patch) | |
tree | cbaff75d89ad52c48188736d26d5e58f3d1aa945 | |
parent | 25688ab79743dd31a1db2e0d7eeb3dc9443bf94b (diff) |
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
-rw-r--r-- | res/res_config_pgsql.c | 130 |
1 files changed, 97 insertions, 33 deletions
diff --git a/res/res_config_pgsql.c b/res/res_config_pgsql.c index b5f24b0f4..93126c6a5 100644 --- a/res/res_config_pgsql.c +++ b/res/res_config_pgsql.c @@ -268,6 +268,7 @@ static struct tables *find_table(const char *database, const char *orig_tablenam } if (database == NULL) { + AST_LIST_UNLOCK(&psql_tables); return NULL; } @@ -409,6 +410,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, @@ -417,6 +428,7 @@ static struct ast_variable *realtime_pgsql(const char *database, const char *tab PQfinish(pgsqlConn); pgsqlConn = NULL; } + ast_mutex_unlock(&pgsql_lock); return NULL; } @@ -434,6 +446,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; } @@ -452,6 +465,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; } @@ -459,8 +473,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; @@ -540,6 +552,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, @@ -548,6 +570,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; } @@ -573,6 +596,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; } @@ -593,6 +617,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; } @@ -604,10 +629,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); @@ -704,6 +726,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, @@ -712,6 +744,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; } @@ -725,6 +758,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; } @@ -735,6 +769,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; } @@ -749,6 +784,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; } @@ -760,6 +796,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; } @@ -768,8 +805,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; @@ -836,12 +871,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; } @@ -849,6 +895,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; } @@ -863,6 +910,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; } @@ -879,6 +927,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; } @@ -892,7 +941,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; @@ -937,6 +986,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, @@ -945,12 +1004,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; } @@ -971,6 +1024,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; @@ -1014,28 +1068,28 @@ static int destroy_pgsql(const char *database, const char *table, const char *ke 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 */ - /*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.. */ - ast_mutex_lock(&pgsql_lock); - if (!pgsql_reconnect(database)) { + } 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 */ @@ -1050,10 +1104,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); @@ -1266,7 +1321,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; } @@ -1393,6 +1448,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; @@ -1500,13 +1556,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) { @@ -1606,7 +1667,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: @@ -1632,8 +1693,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; |