mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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:44 +09:00
parent bc3c8db8ae
commit 01eca6a913
1 changed files with 42 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

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