Fix race condition with threaded applications where SQLGetPrivateProf…

…ileString can fail due to collision with loading driver library during connection
lurcher · Oct 23, 2024 · e72afd6 · e72afd6
1 parent b3ce35a
commit e72afd6
Show file tree

Hide file tree

Showing 10 changed files with 157 additions and 17 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -14,6 +14,8 @@
     * Added some missing error logging in SQLSpecialColumns[W]
     * Set driver_name in SQLBrowseConnect(W) to "" to prevent seg fault if lib not found
     * Various mem buffers fixes. Thanks chipitsine
+    * Fix race condition with threaded applications where SQLGetPrivateProfileString can fail
+      due to collision with loading driver library during connection
 
 8-Aug-2023
 2.3.12

diff --git a/DriverManager/__connection.c b/DriverManager/__connection.c
@@ -114,14 +114,23 @@ char *__find_lib_name( char *dsn, char *lib_name, char *driver_name )
 {
     char driver[ INI_MAX_PROPERTY_VALUE + 1 ];
     char driver_lib[ INI_MAX_PROPERTY_VALUE + 1 ];
+    int mode;
 
-    SQLSetConfigMode( ODBC_USER_DSN );
+    /*
+     * this cound mess up threaded programs by changing the mode
+     */
+
+    __lock_config_mode();
+
+    mode = __get_config_mode();
+
+    __set_config_mode( ODBC_USER_DSN );
 
 	/*
 	 * GET DRIVER FROM ODBC.INI
 	 */
 
-    SQLGetPrivateProfileString( dsn, "Driver", "",
+    __SQLGetPrivateProfileStringNL( dsn, "Driver", "",
             driver_lib, sizeof( driver_lib ), "ODBC.INI" );
 
     if ( driver_lib[ 0 ] == 0 )
@@ -130,14 +139,18 @@ char *__find_lib_name( char *dsn, char *lib_name, char *driver_name )
          * if not found look in system DSN
          */
 
-        SQLSetConfigMode( ODBC_SYSTEM_DSN );
+        __set_config_mode( ODBC_SYSTEM_DSN );
 
-        SQLGetPrivateProfileString( dsn, "Driver", "",
+        __SQLGetPrivateProfileStringNL( dsn, "Driver", "",
                 driver_lib, sizeof( driver_lib ), "ODBC.INI" );
 
-        SQLSetConfigMode( ODBC_BOTH_DSN );
-        if ( driver_lib[ 0 ] == 0 )
+        if ( driver_lib[ 0 ] == 0 ) {
+            __set_config_mode( mode );
+            __unlock_config_mode();
             return NULL;
+        }
+
+        __set_config_mode( ODBC_BOTH_DSN );
     }
 
 	/*
@@ -151,32 +164,38 @@ char *__find_lib_name( char *dsn, char *lib_name, char *driver_name )
         strcpy( driver, driver_lib );
 
 		/*
-		 * allow the use of "User odbcinst files
+		 * allow the use of User odbcinst files, use no lock version as its 
+         * protected by mutex
 		 */
 
 #ifdef PLATFORM64
-		SQLGetPrivateProfileString( driver, "Driver64", "",
+		__SQLGetPrivateProfileStringNL( driver, "Driver64", "",
 				driver_lib, sizeof( driver_lib ), "ODBCINST.INI" );
 
 		if ( driver_lib[ 0 ] == '\0' )
 		{
-			SQLGetPrivateProfileString( driver, "Driver", "",
+			__SQLGetPrivateProfileStringNL( driver, "Driver", "",
 					driver_lib, sizeof( driver_lib ), "ODBCINST.INI" );
 		}
 #else
-		SQLGetPrivateProfileString( driver, "Driver", "",
+		__SQLGetPrivateProfileStringNL( driver, "Driver", "",
 				driver_lib, sizeof( driver_lib ), "ODBCINST.INI" );
 #endif
 
                 strcpy( driver_name, driver );
 
 		if ( driver_lib[ 0 ] == 0 ) {
+            __set_config_mode( mode );
+            __unlock_config_mode();
 		    return NULL;
 		}
 	}
 
 	strcpy( lib_name, driver_lib );
 
+    __set_config_mode( mode );
+    __unlock_config_mode();
+
     return lib_name;
 }
 

diff --git a/include/odbcinstext.h b/include/odbcinstext.h
@@ -208,6 +208,8 @@ BOOL _SQLDriverConnectPromptW(
 
 void __set_config_mode( int mode );
 int __get_config_mode( void );
+void __lock_config_mode( void );
+void __unlock_config_mode( void );
 
 int inst_logPushMsg( 
         char *pszModule, 
@@ -220,6 +222,13 @@ int inst_logPushMsg(
 int inst_logPeekMsg( long nMsg, HLOGMSG *phMsg );
 int inst_logClear();
 
+int __SQLGetPrivateProfileStringNL( LPCSTR  pszSection,
+                                LPCSTR  pszEntry,
+                                LPCSTR  pszDefault,
+                                LPSTR   pRetBuffer,
+                                int     nRetBuffer,
+                                LPCSTR  pszFileName
+                              );
 
 /*
  * we should look at caching this info, the calls can become expensive

diff --git a/include/sql.h b/include/sql.h
@@ -9,7 +9,7 @@
 
 
 /****************************
- * default to 3.51 declare something else before here and you get a whole new ball of wax
+ * default to 3.8 declare something else before here and you get a whole new ball of wax
  ***************************/
 #ifndef ODBCVER
 #define ODBCVER 0x0380

diff --git a/odbcinst/SQLConfigDataSource.c b/odbcinst/SQLConfigDataSource.c
@@ -35,6 +35,7 @@ static BOOL SQLConfigDataSourceWide(	HWND	hWnd,
 	char	szDriverSetup[INI_MAX_PROPERTY_VALUE+1];
     char    szIniName[ ODBC_FILENAME_MAX * 2 + 3 ];
 	char	b1[ ODBC_FILENAME_MAX + 1 ], b2[ ODBC_FILENAME_MAX + 1 ];
+    int     config_mode;
 
 	/* SANITY CHECKS */
 	if ( pszDriver == NULL || pszAttributes == NULL )
@@ -69,6 +70,9 @@ static BOOL SQLConfigDataSourceWide(	HWND	hWnd,
     sprintf( szIniName, "%s/%s", odbcinst_system_file_path( b1 ), odbcinst_system_file_name( b2 ) );
 #endif
 
+    __lock_config_mode();
+    config_mode = __get_config_mode();
+
 	/* OK */
 #ifdef __OS2__
 	if ( iniOpen( &hIni, szIniName, "#;", '[', ']', '=', TRUE, 1L ) != INI_SUCCESS )
@@ -77,6 +81,8 @@ static BOOL SQLConfigDataSourceWide(	HWND	hWnd,
 #endif
 	{
 		inst_logPushMsg( __FILE__, __FILE__, __LINE__, LOG_CRITICAL, ODBC_ERROR_GENERAL_ERR, "" );
+        __set_config_mode( config_mode );
+        __unlock_config_mode();
 		return FALSE;
 	}
 
@@ -105,7 +111,8 @@ static BOOL SQLConfigDataSourceWide(	HWND	hWnd,
 			char szError[ 512 ];
 			sprintf( szError, "Could not find Setup property for (%.400s) in system information", pszDriver );
 			inst_logPushMsg( __FILE__, __FILE__, __LINE__, LOG_CRITICAL, ODBC_ERROR_GENERAL_ERR, szError );
-        	__set_config_mode( ODBC_BOTH_DSN );
+            __set_config_mode( config_mode );
+            __unlock_config_mode();
 			return FALSE;
 		}
 
@@ -184,15 +191,18 @@ static BOOL SQLConfigDataSourceWide(	HWND	hWnd,
 		else
 			inst_logPushMsg( __FILE__, __FILE__, __LINE__, LOG_CRITICAL, ODBC_ERROR_GENERAL_ERR, "" );
 
-        __set_config_mode( ODBC_BOTH_DSN );
+        __set_config_mode( config_mode );
+        __unlock_config_mode();
+
 		return nReturn;
 
 	}
 
 	inst_logPushMsg( __FILE__, __FILE__, __LINE__, LOG_CRITICAL, ODBC_ERROR_GENERAL_ERR, "" );
 	iniClose( hIni );
 
-    __set_config_mode( ODBC_BOTH_DSN );
+    __set_config_mode( config_mode );
+    __unlock_config_mode();
 
 	return FALSE;
 }

diff --git a/odbcinst/SQLGetConfigMode.c b/odbcinst/SQLGetConfigMode.c
@@ -18,7 +18,9 @@ BOOL SQLGetConfigMode( UWORD *pnConfigMode )
 {
     inst_logClear();
 
+    __lock_config_mode();
     *pnConfigMode = __get_config_mode();
+    __unlock_config_mode();
 
     return TRUE;
 }

diff --git a/odbcinst/SQLGetPrivateProfileString.c b/odbcinst/SQLGetPrivateProfileString.c
@@ -365,7 +365,7 @@ void __clear_ini_cache( void )
 
 #endif
 
-int SQLGetPrivateProfileString( LPCSTR  pszSection,
+int __SQLGetPrivateProfileStringNL( LPCSTR  pszSection,
                                 LPCSTR  pszEntry,
                                 LPCSTR  pszDefault,
                                 LPSTR   pRetBuffer,
@@ -605,6 +605,23 @@ int SQLGetPrivateProfileString( LPCSTR  pszSection,
     return ret;
 }
 
+int SQLGetPrivateProfileString( LPCSTR  pszSection,
+                                LPCSTR  pszEntry,
+                                LPCSTR  pszDefault,
+                                LPSTR   pRetBuffer,
+                                int     nRetBuffer,
+                                LPCSTR  pszFileName
+                              )
+{
+int ret;
+
+    __lock_config_mode();
+    ret = __SQLGetPrivateProfileStringNL( pszSection, pszEntry, pszDefault, pRetBuffer, nRetBuffer, pszFileName );
+    __unlock_config_mode();
+
+    return ret;
+}
+
 int  INSTAPI SQLGetPrivateProfileStringW( LPCWSTR lpszSection,
                                         LPCWSTR lpszEntry,
                                         LPCWSTR lpszDefault,

diff --git a/odbcinst/SQLSetConfigMode.c b/odbcinst/SQLSetConfigMode.c
@@ -15,12 +15,85 @@
 #include <odbcinstext.h>
 
 /*
- * This avoids all sorts of problems with using putenv, we need to check 
+ * This avoids all sorts of problems with using putenv, we need to check
  * that drivers can see this as well though....
+ * It has to mess with threads and locking to prevent the calls in connect where
+ * its getting the lib info from messing what ini file is being read
  */
 
 static int __config_mode = ODBC_BOTH_DSN;
 
+#ifdef HAVE_LIBPTH
+
+#include <pth.h>
+
+static pth_mutex_t mutex_config = PTH_MUTEX_INIT;
+static int pth_init_called = 0;
+
+static int mutex_entry( pth_mutex_t *mutex )
+{
+    if ( !pth_init_called )
+    {
+        pth_init();
+        pth_init_called = 1;
+    }
+    return pth_mutex_acquire( mutex, 0, NULL );
+}
+
+static int mutex_exit( pth_mutex_t *mutex )
+{
+    return pth_mutex_release( mutex );
+}
+
+#elif HAVE_LIBPTHREAD
+
+#include <pthread.h>
+
+static pthread_mutex_t mutex_config = PTHREAD_MUTEX_INITIALIZER;
+
+static int mutex_entry( pthread_mutex_t *mutex )
+{
+    return pthread_mutex_lock( mutex );
+}
+
+static int mutex_exit( pthread_mutex_t *mutex )
+{
+    return pthread_mutex_unlock( mutex );
+}
+
+#elif HAVE_LIBTHREAD
+
+#include <thread.h>
+
+static mutex_t mutex_config;
+
+static int mutex_entry( mutex_t *mutex )
+{
+    return mutex_lock( mutex );
+}
+
+static int mutex_exit( mutex_t *mutex )
+{
+    return mutex_unlock( mutex );
+}
+
+#else
+
+#define mutex_entry(x)
+#define mutex_exit(x)
+
+#endif
+
+void __lock_config_mode( void )
+{
+    mutex_entry( &mutex_config );
+}
+
+void __unlock_config_mode( void )
+{
+    mutex_exit( &mutex_config );
+}
+
 void __set_config_mode( int mode )
 {
     __config_mode = mode;
@@ -58,6 +131,9 @@ BOOL SQLSetConfigMode( UWORD nConfigMode )
 {
     inst_logClear();
 
+    __lock_config_mode();
     __set_config_mode( nConfigMode );
+    __unlock_config_mode();
+
     return TRUE;
 }
diff --git a/odbcinst/_odbcinst_ConfigModeINI.c b/odbcinst/_odbcinst_ConfigModeINI.c
@@ -25,7 +25,11 @@
 
 BOOL _odbcinst_ConfigModeINI( char *pszFileName )
 {
-	UWORD nConfigMode = __get_config_mode();
+	UWORD nConfigMode;
+
+    __lock_config_mode();
+	nConfigMode = __get_config_mode();
+    __unlock_config_mode();
 
 	pszFileName[0] = '\0';
 

diff --git a/odbcinst/odbcinst.exp b/odbcinst/odbcinst.exp
@@ -13,6 +13,7 @@ SQLRemoveDSNFromIni
 SQLValidDSN
 SQLWritePrivateProfileString
 SQLGetPrivateProfileString
+__SQLGetPrivateProfileStringNL
 SQLRemoveDriverManager
 SQLInstallTranslator
 SQLRemoveTranslator