Your patch has been added to the PostgreSQL unapplied patches list at:You are too fast: the patch was a proof of concept, not really tested (actually quite buggy).
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
Attached are two patches:
- ready-sigpipe: check_sigpipe_handler skips pthread_create_key if a signal handler was installed. This is wrong - the key is always required.
- ready-locking: locking around kerberos and openssl.
The patches pass the regression tests on i386 linux. Kerberos is untested, ssl only partially tested due to the lack of a test setup.
I'm still not sure if the new code is the right thing for the openssl initialization: libpq calls SSL_library_init() unconditionally. If the calling app uses ssl, too, this might confuse openssl.
Could you replace my initial proposal with these two patches?
Btw, is it intentional that THREAD_SUPPORT is not set in src/template/linux?
-- Manfred
Index: src/backend/libpq/md5.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/libpq/md5.c,v
retrieving revision 1.22
diff -c -r1.22 md5.c
*** src/backend/libpq/md5.c 29 Nov 2003 19:51:49 -0000 1.22
--- src/backend/libpq/md5.c 14 Mar 2004 10:46:54 -0000
***************
*** 271,277 ****
static void
bytesToHex(uint8 b[16], char *s)
{
! static char *hex = "0123456789abcdef";
int q,
w;
--- 271,277 ----
static void
bytesToHex(uint8 b[16], char *s)
{
! static const char *hex = "0123456789abcdef";
int q,
w;
Index: src/interfaces/libpq/fe-auth.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-auth.c,v
retrieving revision 1.89
diff -c -r1.89 fe-auth.c
*** src/interfaces/libpq/fe-auth.c 7 Jan 2004 18:56:29 -0000 1.89
--- src/interfaces/libpq/fe-auth.c 14 Mar 2004 10:46:55 -0000
***************
*** 590,595 ****
--- 590,596 ----
case AUTH_REQ_KRB4:
#ifdef KRB4
+ pglock_thread();
if (pg_krb4_sendauth(PQerrormsg, conn->sock,
(struct sockaddr_in *) &
conn->laddr.addr,
(struct sockaddr_in *) &
conn->raddr.addr,
***************
*** 597,604 ****
--- 598,607 ----
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
libpq_gettext("Kerberos 4 authentication
failed\n"));
+ pgunlock_thread();
return STATUS_ERROR;
}
+ pgunlock_thread();
break;
#else
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
***************
*** 608,620 ****
--- 611,626 ----
case AUTH_REQ_KRB5:
#ifdef KRB5
+ pglock_thread();
if (pg_krb5_sendauth(PQerrormsg, conn->sock,
hostname) !=
STATUS_OK)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
libpq_gettext("Kerberos 5 authentication
failed\n"));
+ pgunlock_thread();
return STATUS_ERROR;
}
+ pgunlock_thread();
break;
#else
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
***************
*** 722,727 ****
--- 728,734 ----
if (authsvc == 0)
return NULL; /* leave original error message in
place */
+ pglock_thread();
#ifdef KRB4
if (authsvc == STARTUP_KRB4_MSG)
name = pg_krb4_authname(PQerrormsg);
***************
*** 759,763 ****
--- 766,771 ----
if (name && (authn = (char *) malloc(strlen(name) + 1)))
strcpy(authn, name);
+ pgunlock_thread();
return authn;
}
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.268
diff -c -r1.268 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 10 Mar 2004 21:12:47 -0000 1.268
--- src/interfaces/libpq/fe-connect.c 14 Mar 2004 10:46:56 -0000
***************
*** 2902,2908 ****
PQsetClientEncoding(PGconn *conn, const char *encoding)
{
char qbuf[128];
! static char query[] = "set client_encoding to '%s'";
PGresult *res;
int status;
--- 2902,2908 ----
PQsetClientEncoding(PGconn *conn, const char *encoding)
{
char qbuf[128];
! static const char query[] = "set client_encoding to '%s'";
PGresult *res;
int status;
***************
*** 3162,3166 ****
--- 3162,3207 ----
return NULL;
#undef LINELEN
+ }
+
+ /*
+ * To keep the API consistent, the locking stubs are always provided, even
+ * if they are not required.
+ */
+
+ void
+ PQenableSSLLocks(int enable)
+ {
+ #if defined(ENABLE_THREAD_SAFETY) && defined(USE_SSL)
+ pq_usessllocks = enable;
+ #endif
+ }
+
+ static pgthreadlock_t default_threadlock;
+ static void
+ default_threadlock(int acquire)
+ {
+ #if defined(ENABLE_THREAD_SAFETY)
+ static pthread_mutex_t singlethread_lock = PTHREAD_MUTEX_INITIALIZER;
+ if (acquire)
+ pthread_mutex_lock(&singlethread_lock);
+ else
+ pthread_mutex_unlock(&singlethread_lock);
+ #endif
+ }
+
+ pgthreadlock_t *g_threadlock = default_threadlock;
+
+ pgthreadlock_t *
+ PQregisterThreadLock(pgthreadlock_t *newhandler)
+ {
+ pgthreadlock_t *prev;
+
+ prev = g_threadlock;
+ if (newhandler)
+ g_threadlock = newhandler;
+ else
+ g_threadlock = default_threadlock;
+ return prev;
}
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.37
diff -c -r1.37 fe-secure.c
*** src/interfaces/libpq/fe-secure.c 10 Feb 2004 15:21:24 -0000 1.37
--- src/interfaces/libpq/fe-secure.c 14 Mar 2004 10:46:56 -0000
***************
*** 135,145 ****
static DH *load_dh_buffer(const char *, size_t);
static DH *tmp_dh_cb(SSL *s, int is_export, int keylength);
static int client_cert_cb(SSL *, X509 **, EVP_PKEY **);
static int initialize_SSL(PGconn *);
static void destroy_SSL(void);
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
! static const char *SSLerrmessage(void);
#endif
#ifdef USE_SSL
--- 135,147 ----
static DH *load_dh_buffer(const char *, size_t);
static DH *tmp_dh_cb(SSL *s, int is_export, int keylength);
static int client_cert_cb(SSL *, X509 **, EVP_PKEY **);
+ static int init_ssl_system(PGconn *conn);
static int initialize_SSL(PGconn *);
static void destroy_SSL(void);
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
! static char *SSLerrmessage(void);
! static void SSLerrfree(char *buf);
#endif
#ifdef USE_SSL
***************
*** 251,259 ****
!SSL_set_app_data(conn->ssl, conn) ||
!SSL_set_fd(conn->ssl, conn->sock))
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not establish SSL connection: %s\n"),
! SSLerrmessage());
close_SSL(conn);
return PGRES_POLLING_FAILED;
}
--- 253,263 ----
!SSL_set_app_data(conn->ssl, conn) ||
!SSL_set_fd(conn->ssl, conn->sock))
{
+ char *err = SSLerrmessage();
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not establish SSL connection: %s\n"),
! err);
! SSLerrfree(err);
close_SSL(conn);
return PGRES_POLLING_FAILED;
}
***************
*** 327,334 ****
break;
}
case SSL_ERROR_SSL:
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("SSL error: %s\n"),
SSLerrmessage());
/* fall through */
case SSL_ERROR_ZERO_RETURN:
SOCK_ERRNO_SET(ECONNRESET);
--- 331,342 ----
break;
}
case SSL_ERROR_SSL:
! {
! char *err = SSLerrmessage();
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("SSL error: %s\n"),
err);
! SSLerrfree(err);
! }
/* fall through */
case SSL_ERROR_ZERO_RETURN:
SOCK_ERRNO_SET(ECONNRESET);
***************
*** 402,409 ****
break;
}
case SSL_ERROR_SSL:
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("SSL error: %s\n"),
SSLerrmessage());
/* fall through */
case SSL_ERROR_ZERO_RETURN:
SOCK_ERRNO_SET(ECONNRESET);
--- 410,421 ----
break;
}
case SSL_ERROR_SSL:
! {
! char *err = SSLerrmessage();
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("SSL error: %s\n"),
err);
! SSLerrfree(err);
! }
/* fall through */
case SSL_ERROR_ZERO_RETURN:
SOCK_ERRNO_SET(ECONNRESET);
***************
*** 750,758 ****
}
if (PEM_read_X509(fp, x509, NULL, NULL) == NULL)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read certificate (%s):
%s\n"),
! fnbuf, SSLerrmessage());
fclose(fp);
return -1;
}
--- 762,772 ----
}
if (PEM_read_X509(fp, x509, NULL, NULL) == NULL)
{
+ char *err = SSLerrmessage();
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read certificate (%s):
%s\n"),
! fnbuf, err);
! SSLerrfree(err);
fclose(fp);
return -1;
}
***************
*** 795,803 ****
}
if (PEM_read_PrivateKey(fp, pkey, cb, NULL) == NULL)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read private key (%s):
%s\n"),
! fnbuf, SSLerrmessage());
X509_free(*x509);
fclose(fp);
return -1;
--- 809,819 ----
}
if (PEM_read_PrivateKey(fp, pkey, cb, NULL) == NULL)
{
+ char *err = SSLerrmessage();
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read private key (%s):
%s\n"),
! fnbuf, err);
! SSLerrfree(err);
X509_free(*x509);
fclose(fp);
return -1;
***************
*** 807,815 ****
/* verify that the cert and key go together */
if (!X509_check_private_key(*x509, *pkey))
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate/private key mismatch (%s): %s\n"),
! fnbuf, SSLerrmessage());
X509_free(*x509);
EVP_PKEY_free(*pkey);
return -1;
--- 823,833 ----
/* verify that the cert and key go together */
if (!X509_check_private_key(*x509, *pkey))
{
+ char *err = SSLerrmessage();
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate/private key mismatch (%s): %s\n"),
! fnbuf, err);
! SSLerrfree(err);
X509_free(*x509);
EVP_PKEY_free(*pkey);
return -1;
***************
*** 819,838 ****
#endif
}
! /*
! * Initialize global SSL context.
! */
static int
! initialize_SSL(PGconn *conn)
{
! #ifndef WIN32
! struct stat buf;
! char pwdbuf[BUFSIZ];
! struct passwd pwdstr;
! struct passwd *pwd = NULL;
! char fnbuf[2048];
! #endif
if (!SSL_context)
{
SSL_library_init();
--- 837,888 ----
#endif
}
! #ifdef ENABLE_THREAD_SAFETY
!
! static unsigned long
! pq_threadidcallback(void)
! {
! return (unsigned long)pthread_self();
! }
!
! static pthread_rwlock_t *pq_lockarray;
! static void
! pq_lockingcallback(int mode, int n, const char *file, int line)
! {
! if (mode & CRYPTO_LOCK) {
! pthread_rwlock_wrlock(&pq_lockarray[n]);
! } else {
! pthread_rwlock_unlock(&pq_lockarray[n]);
! }
! }
!
! bool pq_usessllocks = true;
!
! #endif /* ENABLE_THRAD_SAFETY */
!
static int
! init_ssl_system(PGconn *conn)
{
! #ifdef ENABLE_THREAD_SAFETY
! static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
!
! pthread_mutex_lock(&init_mutex);
!
! if (pq_usessllocks && pq_lockarray == NULL) {
! int i;
! CRYPTO_set_id_callback(pq_threadidcallback);
!
! pq_lockarray = malloc(sizeof(pthread_rwlock_t)*CRYPTO_num_locks());
! if (!pq_lockarray) {
! pthread_mutex_unlock(&init_mutex);
! return -1;
! }
! for (i=0;i<CRYPTO_num_locks();i++)
! pthread_rwlock_init(&pq_lockarray[i], NULL);
+ CRYPTO_set_locking_callback(pq_lockingcallback);
+ }
+ #endif
if (!SSL_context)
{
SSL_library_init();
***************
*** 840,851 ****
SSL_context = SSL_CTX_new(TLSv1_method());
if (!SSL_context)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not create SSL context:
%s\n"),
! SSLerrmessage());
return -1;
}
}
#ifndef WIN32
if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) == 0)
--- 890,927 ----
SSL_context = SSL_CTX_new(TLSv1_method());
if (!SSL_context)
{
+ char *err = SSLerrmessage();
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not create SSL context:
%s\n"),
! err);
! SSLerrfree(err);
! #ifdef ENABLE_THREAD_SAFETY
! pthread_mutex_unlock(&init_mutex);
! #endif
return -1;
}
}
+ #ifdef ENABLE_THREAD_SAFETY
+ pthread_mutex_unlock(&init_mutex);
+ #endif
+ return 0;
+ }
+ /*
+ * Initialize global SSL context.
+ */
+ static int
+ initialize_SSL(PGconn *conn)
+ {
+ #ifndef WIN32
+ struct stat buf;
+ char pwdbuf[BUFSIZ];
+ struct passwd pwdstr;
+ struct passwd *pwd = NULL;
+ char fnbuf[2048];
+ #endif
+
+ if(!init_ssl_system(conn))
+ return -1;
#ifndef WIN32
if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) == 0)
***************
*** 867,875 ****
}
if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, 0))
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not
read root certificate list (%s): %s\n"),
! fnbuf, SSLerrmessage());
return -1;
}
}
--- 943,953 ----
}
if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, 0))
{
+ char *err = SSLerrmessage();
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not
read root certificate list (%s): %s\n"),
! fnbuf, err);
! SSLerrfree(err);
return -1;
}
}
***************
*** 936,945 ****
return PGRES_POLLING_FAILED;
}
case SSL_ERROR_SSL:
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("SSL error: %s\n"),
SSLerrmessage());
! close_SSL(conn);
! return PGRES_POLLING_FAILED;
default:
printfPQExpBuffer(&conn->errorMessage,
--- 1014,1027 ----
return PGRES_POLLING_FAILED;
}
case SSL_ERROR_SSL:
! {
! char *err = SSLerrmessage();
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("SSL error: %s\n"),
err);
! SSLerrfree(err);
! close_SSL(conn);
! return PGRES_POLLING_FAILED;
! }
default:
printfPQExpBuffer(&conn->errorMessage,
***************
*** 973,981 ****
conn->peer = SSL_get_peer_certificate(conn->ssl);
if (conn->peer == NULL)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate could not be obtained:
%s\n"),
! SSLerrmessage());
close_SSL(conn);
return PGRES_POLLING_FAILED;
}
--- 1055,1065 ----
conn->peer = SSL_get_peer_certificate(conn->ssl);
if (conn->peer == NULL)
{
+ char *err = SSLerrmessage();
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate could not be obtained:
%s\n"),
! err);
! SSLerrfree(err);
close_SSL(conn);
return PGRES_POLLING_FAILED;
}
***************
*** 1036,1058 ****
* return NULL if it doesn't recognize the error code. We don't
* want to return NULL ever.
*/
! static const char *
SSLerrmessage(void)
{
unsigned long errcode;
const char *errreason;
! static char errbuf[32];
errcode = ERR_get_error();
! if (errcode == 0)
! return "No SSL error reported";
errreason = ERR_reason_error_string(errcode);
! if (errreason != NULL)
! return errreason;
! snprintf(errbuf, sizeof(errbuf), "SSL error code %lu", errcode);
return errbuf;
}
/*
* Return pointer to SSL object.
*/
--- 1120,1159 ----
* return NULL if it doesn't recognize the error code. We don't
* want to return NULL ever.
*/
! static char ssl_nomem[] = "Out of memory allocating error description";
! #define SSL_ERR_LEN 128
!
! static char *
SSLerrmessage(void)
{
unsigned long errcode;
const char *errreason;
! char *errbuf;
+ errbuf = malloc(SSL_ERR_LEN);
+ if (!errbuf)
+ return ssl_nomem;
errcode = ERR_get_error();
! if (errcode == 0) {
! strcpy(errbuf, "No SSL error reported");
! return errbuf;
! }
errreason = ERR_reason_error_string(errcode);
! if (errreason != NULL) {
! strncpy(errbuf, errreason, SSL_ERR_LEN-1);
! errbuf[SSL_ERR_LEN-1] = '\0';
! return errbuf;
! }
! snprintf(errbuf, SSL_ERR_LEN, "SSL error code %lu", errcode);
return errbuf;
}
+ static void
+ SSLerrfree(char *buf)
+ {
+ if (buf != ssl_nomem)
+ free(buf);
+ }
/*
* Return pointer to SSL object.
*/
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.102
diff -c -r1.102 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h 9 Jan 2004 02:02:43 -0000 1.102
--- src/interfaces/libpq/libpq-fe.h 14 Mar 2004 10:46:57 -0000
***************
*** 274,279 ****
--- 274,293 ----
PQnoticeProcessor proc,
void *arg);
+ /*
+ * Used to set callback that prevents concurrent access to
+ * non-thread safe functions that libpq needs.
+ * The default implementation uses a libpq internal mutex.
+ * Only required for multithreaded apps that use kerberos
+ * both within their app and for postgresql connections.
+ */
+ typedef void (pgthreadlock_t)(int acquire);
+
+ extern pgthreadlock_t * PQregisterThreadLock(pgthreadlock_t *newhandler);
+
+ void
+ PQenableSSLLocks(int enable);
+
/* === in fe-exec.c === */
/* Simple synchronous query */
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.85
diff -c -r1.85 libpq-int.h
*** src/interfaces/libpq/libpq-int.h 5 Mar 2004 01:53:59 -0000 1.85
--- src/interfaces/libpq/libpq-int.h 14 Mar 2004 10:46:57 -0000
***************
*** 359,364 ****
--- 359,374 ----
extern int pqPacketSend(PGconn *conn, char pack_type,
const void *buf, size_t buf_len);
+ #ifdef ENABLE_THREAD_SAFETY
+ extern pgthreadlock_t *g_threadlock;
+ #define pglock_thread() g_threadlock(true);
+ #define pgunlock_thread() g_threadlock(false);
+ #else
+ #define pglock_thread() ((void)0)
+ #define pgunlock_thread() ((void)0)
+ #endif
+
+
/* === in fe-exec.c === */
extern void pqSetResultError(PGresult *res, const char *msg);
***************
*** 448,453 ****
--- 458,464 ----
#ifdef ENABLE_THREAD_SAFETY
extern void check_sigpipe_handler(void);
extern pthread_key_t thread_in_send;
+ extern bool pq_usessllocks;
#endif
/*
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.37
diff -c -r1.37 fe-secure.c
*** src/interfaces/libpq/fe-secure.c 10 Feb 2004 15:21:24 -0000 1.37
--- src/interfaces/libpq/fe-secure.c 14 Mar 2004 08:31:48 -0000
***************
*** 1077,1096 ****
pqsigfunc pipehandler;
/*
* If the app hasn't set a SIGPIPE handler, define our own
* that ignores SIGPIPE on libpq send() and does SIG_DFL
* for other SIGPIPE cases.
*/
pipehandler = pqsignalinquire(SIGPIPE);
if (pipehandler == SIG_DFL) /* not set by application */
- {
- /*
- * Create key first because the signal handler might be called
- * right after being installed.
- */
- pthread_key_create(&thread_in_send, NULL);
pqsignal(SIGPIPE, sigpipe_handler_ignore_send);
- }
}
/*
--- 1077,1096 ----
pqsigfunc pipehandler;
/*
+ * Always create the key for SIGPIPE handling - PQinSend needs
+ * it. Create it first because the signal handler might be called
+ * right after being installed.
+ */
+ pthread_key_create(&thread_in_send, NULL);
+
+ /*
* If the app hasn't set a SIGPIPE handler, define our own
* that ignores SIGPIPE on libpq send() and does SIG_DFL
* for other SIGPIPE cases.
*/
pipehandler = pqsignalinquire(SIGPIPE);
if (pipehandler == SIG_DFL) /* not set by application */
pqsignal(SIGPIPE, sigpipe_handler_ignore_send);
}
/*
---------------------------(end of broadcast)--------------------------- TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
