database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-03 22:24:49 +03:00
Code

Fix race condition with BIO methods initialization in libpq with threads

Browse Source 
The libpq code in charge of creating per-connection SSL objects was
prone to a race condition when loading the custom BIO methods needed by
my_SSL_set_fd().  As BIO methods are stored as a static variable, the
initialization of a connection could fail because it could be possible
to have one thread refer to my_bio_methods while it is being manipulated
by a second concurrent thread.

This error has been introduced by 8bb14cdd33de, that has removed
ssl_config_mutex around the call of my_SSL_set_fd(), that itself sets
the custom BIO methods used in libpq.  Like previously, the BIO method
initialization is now protected by the existing ssl_config_mutex, itself
initialized earlier for WIN32.

While on it, document that my_bio_methods is protected by
ssl_config_mutex, as this can be easy to miss.

Reported-by: Willi Mann
Author: Willi Mann, Michael Paquier
Discussion: https://postgr.es/m/e77abc4c-4d03-4058-a9d7-ef0035657e04@celonis.com
Backpatch-through: 12
This commit is contained in:
Michael Paquier 2023-11-27 09:40:52 +09:00
parent 648fc6a1c4
commit c3b79223f3
1 changed files with 51 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
src/interfaces/libpq/fe-secure-openssl.c
View File

@ -1666,6 +1666,7 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
#define BIO_set_data(bio, data) (bio->ptr = data) #define BIO_set_data(bio, data) (bio->ptr = data)
#endif #endif
/* protected by ssl_config_mutex */
static BIO_METHOD *my_bio_methods; static BIO_METHOD *my_bio_methods;
static int static int
@ -1731,6 +1732,15 @@ my_sock_write(BIO *h, const char *buf, int size)
static BIO_METHOD * static BIO_METHOD *
my_BIO_s_socket(void) my_BIO_s_socket(void)
{ {
BIO_METHOD *res;
#ifdef ENABLE_THREAD_SAFETY
if (pthread_mutex_lock(&ssl_config_mutex))
return NULL;
#endif
res = my_bio_methods;
if (!my_bio_methods) if (!my_bio_methods)
{ {
BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket(); BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
@ -1739,39 +1749,58 @@ my_BIO_s_socket(void)
my_bio_index = BIO_get_new_index(); my_bio_index = BIO_get_new_index();
if (my_bio_index == -1) if (my_bio_index == -1)
return NULL; goto err;
my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK); my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
my_bio_methods = BIO_meth_new(my_bio_index, "libpq socket"); res = BIO_meth_new(my_bio_index, "libpq socket");
if (!my_bio_methods) if (!res)
return NULL; goto err;
/* /*
* As of this writing, these functions never fail. But check anyway, * As of this writing, these functions never fail. But check anyway,
* like OpenSSL's own examples do. * like OpenSSL's own examples do.
*/ */
if (!BIO_meth_set_write(my_bio_methods, my_sock_write) || if (!BIO_meth_set_write(res, my_sock_write) ||
!BIO_meth_set_read(my_bio_methods, my_sock_read) || !BIO_meth_set_read(res, my_sock_read) ||
!BIO_meth_set_gets(my_bio_methods, BIO_meth_get_gets(biom)) || !BIO_meth_set_gets(res, BIO_meth_get_gets(biom)) ||
!BIO_meth_set_puts(my_bio_methods, BIO_meth_get_puts(biom)) || !BIO_meth_set_puts(res, BIO_meth_get_puts(biom)) ||
!BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom)) || !BIO_meth_set_ctrl(res, BIO_meth_get_ctrl(biom)) ||
!BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom)) || !BIO_meth_set_create(res, BIO_meth_get_create(biom)) ||
!BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom)) || !BIO_meth_set_destroy(res, BIO_meth_get_destroy(biom)) ||
!BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom))) !BIO_meth_set_callback_ctrl(res, BIO_meth_get_callback_ctrl(biom)))
{ {
BIO_meth_free(my_bio_methods); goto err;
my_bio_methods = NULL;
return NULL;
} }
#else #else
my_bio_methods = malloc(sizeof(BIO_METHOD)); res = malloc(sizeof(BIO_METHOD));
if (!my_bio_methods) if (!res)
return NULL; goto err;
memcpy(my_bio_methods, biom, sizeof(BIO_METHOD)); memcpy(res, biom, sizeof(BIO_METHOD));
my_bio_methods->bread = my_sock_read; res->bread = my_sock_read;
my_bio_methods->bwrite = my_sock_write; res->bwrite = my_sock_write;
#endif #endif
} }
return my_bio_methods;
my_bio_methods = res;
#ifdef ENABLE_THREAD_SAFETY
pthread_mutex_unlock(&ssl_config_mutex);
#endif
return res;
err:
#ifdef HAVE_BIO_METH_NEW
if (res)
BIO_meth_free(res);
#else
if (res)
free(res);
#endif
#ifdef ENABLE_THREAD_SAFETY
pthread_mutex_unlock(&ssl_config_mutex);
#endif
return NULL;
} }
/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */ /* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */