postgres.git / summary / log / commit / refs

commit    cc057fb315e24b78552f5b7ac05792418042e71b
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Mon Sep 13 04:24:04 2021 +0000

    Fix error handling with threads on OOM in ECPG connection logic
    
    An out-of-memory failure happening when allocating the structures to
    store the connection parameter keywords and values would mess up with
    the set of connections saved, as on failure the pthread mutex would
    still be hold with the new connection object listed but free()'d.
    
    Rather than just unlocking the mutex, which would leave the static list
    of connections into an inconsistent state, move the allocation for the
    structures of the connection parameters before beginning the test
    manipulation.  This ensures that the list of connections and the
    connection mutex remain consistent all the time in this code path.
    
    This error is unlikely going to happen, but this could mess up badly
    with ECPG clients in surprising ways, so backpatch all the way down.
    
    Reported-by: ryancaicse
    Discussion: https://postgr.es/m/17186-b4cfd8f0eb4d1dee@postgresql.org
    Backpatch-through: 9.6


 src/interfaces/ecpg/ecpglib/connect.c | 71 +++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 60564b176c9..a2166b1e12f 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -458,37 +458,10 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 	else
 		realname = NULL;
 
-	/* add connection to our list */
-#ifdef ENABLE_THREAD_SAFETY
-	pthread_mutex_lock(&connections_mutex);
-#endif
-	if (connection_name != NULL)
-		this->name = ecpg_strdup(connection_name, lineno);
-	else
-		this->name = ecpg_strdup(realname, lineno);
-
-	this->cache_head = NULL;
-	this->prep_stmts = NULL;
-
-	if (all_connections == NULL)
-		this->next = NULL;
-	else
-		this->next = all_connections;
-
-	all_connections = this;
-#ifdef ENABLE_THREAD_SAFETY
-	pthread_setspecific(actual_connection_key, all_connections);
-#endif
-	actual_connection = all_connections;
-
-	ecpg_log("ECPGconnect: opening database %s on %s port %s %s%s %s%s\n",
-			 realname ? realname : "<DEFAULT>",
-			 host ? host : "<DEFAULT>",
-			 port ? (ecpg_internal_regression_mode ? "<REGRESSION_PORT>" : port) : "<DEFAULT>",
-			 options ? "with options " : "", options ? options : "",
-			 (user && strlen(user) > 0) ? "for user " : "", user ? user : "");
-
-	/* count options (this may produce an overestimate, it's ok) */
+	/*
+	 * Count options for the allocation done below (this may produce an
+	 * overestimate, it's ok).
+	 */
 	if (options)
 		for (i = 0; options[i]; i++)
 			if (options[i] == '=')
@@ -499,7 +472,11 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 	if (passwd && strlen(passwd) > 0)
 		connect_params++;
 
-	/* allocate enough space for all connection parameters */
+	/*
+	 * Allocate enough space for all connection parameters.  These allocations
+	 * are done before manipulating the list of connections to ease the error
+	 * handling on failure.
+	 */
 	conn_keywords = (const char **) ecpg_alloc((connect_params + 1) * sizeof(char *), lineno);
 	conn_values = (const char **) ecpg_alloc(connect_params * sizeof(char *), lineno);
 	if (conn_keywords == NULL || conn_values == NULL)
@@ -522,6 +499,36 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 		return false;
 	}
 
+	/* add connection to our list */
+#ifdef ENABLE_THREAD_SAFETY
+	pthread_mutex_lock(&connections_mutex);
+#endif
+	if (connection_name != NULL)
+		this->name = ecpg_strdup(connection_name, lineno);
+	else
+		this->name = ecpg_strdup(realname, lineno);
+
+	this->cache_head = NULL;
+	this->prep_stmts = NULL;
+
+	if (all_connections == NULL)
+		this->next = NULL;
+	else
+		this->next = all_connections;
+
+	all_connections = this;
+#ifdef ENABLE_THREAD_SAFETY
+	pthread_setspecific(actual_connection_key, all_connections);
+#endif
+	actual_connection = all_connections;
+
+	ecpg_log("ECPGconnect: opening database %s on %s port %s %s%s %s%s\n",
+			 realname ? realname : "<DEFAULT>",
+			 host ? host : "<DEFAULT>",
+			 port ? (ecpg_internal_regression_mode ? "<REGRESSION_PORT>" : port) : "<DEFAULT>",
+			 options ? "with options " : "", options ? options : "",
+			 (user && strlen(user) > 0) ? "for user " : "", user ? user : "");
+
 	i = 0;
 	if (realname)
 	{


[parent: b33283cbd336]