Move channel mutex out of QUIC_CHANNEL for init/teardown flexibility

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20348)
This commit is contained in:
Hugo Landau 2023-02-21 10:18:58 +00:00
parent d7b1faddab
commit 4847599b54
5 changed files with 70 additions and 46 deletions

View File

@ -54,9 +54,11 @@
* To support thread assisted mode, QUIC_CHANNEL can be used by multiple
* threads. **It is the caller's responsibility to ensure that the QUIC_CHANNEL
* is only accessed (whether via its methods or via direct access to its state)
* while the QUIC_CHANNEL mutex is held**, except for methods explicitly marked
* as not requiring prior locking. See ossl_quic_channel_get_mutex() for more
* information. This is an unchecked precondition.
* while the channel mutex is held**, except for methods explicitly marked as
* not requiring prior locking. This is an unchecked precondition.
*
* The instantiator of the channel is responsible for providing a suitable
* mutex which then serves as the channel mutex; see QUIC_CHANNEL_ARGS.
*/
# define QUIC_NEEDS_LOCK
@ -70,10 +72,18 @@
# define QUIC_CHANNEL_STATE_TERMINATED 4
typedef struct quic_channel_args_st {
OSSL_LIB_CTX *libctx;
const char *propq;
int is_server;
SSL *tls;
OSSL_LIB_CTX *libctx;
const char *propq;
int is_server;
SSL *tls;
/*
* This must be a mutex the lifetime of which will exceed that of the
* channel. The instantiator of the channel is responsible for providing a
* mutex as this makes it easier to handle instantiation and teardown of
* channels in situations potentially requiring locking.
*/
CRYPTO_RWLOCK *mutex;
} QUIC_CHANNEL_ARGS;
typedef struct quic_channel_st QUIC_CHANNEL;
@ -213,31 +223,20 @@ QUIC_DEMUX *ossl_quic_channel_get0_demux(QUIC_CHANNEL *ch);
SSL *ossl_quic_channel_get0_ssl(QUIC_CHANNEL *ch);
/*
* Retreves the channel mutex, which can be used to synchronise access to
* channel functions and internal data. In order to allow locks to be acquired
* and released with the correct granularity, it is the caller's responsibility
* to ensure this lock is held for write while calling any QUIC_CHANNEL method.
* Retrieves a pointer to the channel mutex which was provided at the time the
* channel was instantiated. In order to allow locks to be acquired and released
* with the correct granularity, it is the caller's responsibility to ensure
* this lock is held for write while calling any QUIC_CHANNEL method, except for
* methods explicitly designed otherwise.
*
* This method is thread safe and does not require prior locking. It can also be
* called while the lock is already held.
* called while the lock is already held. Note that this is simply a convenience
* function to access the mutex which was passed to the channel at instantiation
* time; it does not belong to the channel but rather is presumed to belong to
* the owner of the channel.
*/
CRYPTO_RWLOCK *ossl_quic_channel_get_mutex(QUIC_CHANNEL *ch);
/*
* Locks the channel mutex. It is roughly analagous to locking the mutex
* returned by ossl_quic_channel_get_mutex() but might be able to avoid locking
* where thread assisted mode is not being used, thus it is recommended that
* these methods are used uniformly rather than locking the channel mutex
* directly.
*
* This method is (obviously) thread safe and does not require prior locking. It
* must not be called while the lock is already held.
*/
int ossl_quic_channel_lock(QUIC_CHANNEL *ch);
/* Unlocks the channel mutex. */
void ossl_quic_channel_unlock(QUIC_CHANNEL *ch);
# endif
#endif

View File

@ -115,10 +115,6 @@ static int ch_init(QUIC_CHANNEL *ch)
qtx_args.mdpl = QUIC_MIN_INITIAL_DGRAM_LEN;
ch->rx_max_udp_payload_size = qtx_args.mdpl;
ch->mutex = CRYPTO_THREAD_lock_new();
if (ch->mutex == NULL)
goto err;
ch->qtx = ossl_qtx_new(&qtx_args);
if (ch->qtx == NULL)
goto err;
@ -322,7 +318,6 @@ static void ch_cleanup(QUIC_CHANNEL *ch)
ossl_qrx_free(ch->qrx);
ossl_quic_demux_free(ch->demux);
OPENSSL_free(ch->local_transport_params);
CRYPTO_THREAD_lock_free(ch->mutex);
}
QUIC_CHANNEL *ossl_quic_channel_new(const QUIC_CHANNEL_ARGS *args)
@ -336,6 +331,7 @@ QUIC_CHANNEL *ossl_quic_channel_new(const QUIC_CHANNEL_ARGS *args)
ch->propq = args->propq;
ch->is_server = args->is_server;
ch->tls = args->tls;
ch->mutex = args->mutex;
if (!ch_init(ch)) {
OPENSSL_free(ch);
@ -453,16 +449,6 @@ CRYPTO_MUTEX *ossl_quic_channel_get_mutex(QUIC_CHANNEL *ch)
return ch->mutex;
}
int ossl_quic_channel_lock(QUIC_CHANNEL *ch)
{
return ossl_crypto_mutex_lock(ch->mutex);
}
void ossl_quic_channel_unlock(QUIC_CHANNEL *ch)
{
ossl_crypto_mutex_unlock(ch->mutex);
}
/*
* QUIC Channel: Callbacks from Miscellaneous Subsidiary Components
* ================================================================

View File

@ -27,7 +27,9 @@ struct quic_channel_st {
/*
* Master synchronisation mutex used for thread assisted mode
* synchronisation.
* synchronisation. We don't own this; the instantiator of the channel
* passes it to us and is responsible for freeing it after channel
* destruction.
*/
CRYPTO_RWLOCK *mutex;

View File

@ -42,7 +42,7 @@ static int block_until_pred(QUIC_CONNECTION *qc,
rtor = ossl_quic_channel_get_reactor(qc->ch);
return ossl_quic_reactor_block_until_pred(rtor, pred, pred_arg, flags,
ossl_quic_channel_get_mutex(qc->ch));
qc->mutex);
}
/*
@ -104,6 +104,25 @@ static ossl_inline int expect_quic_conn(const QUIC_CONNECTION *qc)
}
/*
* Ensures that the channel mutex is held for a method which touches channel
* state.
*
* Precondition: Channel mutex is not held (unchecked)
*/
static int quic_lock(QUIC_CONNECTION *qc)
{
return CRYPTO_THREAD_write_lock(qc->mutex);
}
/* Precondition: Channel mutex is held (unchecked) */
QUIC_NEEDS_LOCK
static void quic_unlock(QUIC_CONNECTION *qc)
{
CRYPTO_THREAD_unlock(qc->mutex);
}
/*
* QUIC Front-End I/O API: Initialization
* ======================================
@ -139,6 +158,9 @@ SSL *ossl_quic_new(SSL_CTX *ctx)
if (qc->tls == NULL || (sc = SSL_CONNECTION_FROM_SSL(qc->tls)) == NULL)
goto err;
if ((qc->mutex = CRYPTO_THREAD_lock_new()) == NULL)
goto err;
/* Channel is not created yet. */
qc->ssl_mode = qc->ssl.ctx->mode;
qc->last_error = SSL_ERROR_NONE;
@ -147,6 +169,7 @@ SSL *ossl_quic_new(SSL_CTX *ctx)
return ssl_base;
err:
SSL_free(qc->tls);
OPENSSL_free(qc);
return NULL;
}
@ -169,6 +192,7 @@ void ossl_quic_free(SSL *s)
/* Note: SSL_free calls OPENSSL_free(qc) for us */
SSL_free(qc->tls);
CRYPTO_THREAD_lock_free(qc->mutex);
}
/* SSL method init */
@ -601,7 +625,7 @@ static int configure_channel(QUIC_CONNECTION *qc)
return 1;
}
QUIC_TODO_LOCK
QUIC_NEEDS_LOCK
static int ensure_channel(QUIC_CONNECTION *qc)
{
QUIC_CHANNEL_ARGS args = {0};
@ -626,7 +650,7 @@ static int ensure_channel(QUIC_CONNECTION *qc)
* via calls made to us from the application prior to starting a handshake
* attempt.
*/
QUIC_TODO_LOCK
QUIC_NEEDS_LOCK
static int ensure_channel_and_start(QUIC_CONNECTION *qc)
{
if (!ensure_channel(qc))
@ -793,6 +817,7 @@ int ossl_quic_get_error(const QUIC_CONNECTION *qc, int i)
* - SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER.
*
*/
QUIC_NEEDS_LOCK
static void quic_post_write(QUIC_CONNECTION *qc, int did_append, int do_tick)
{
/*
@ -820,6 +845,7 @@ struct quic_write_again_args {
size_t total_written;
};
QUIC_NEEDS_LOCK
static int quic_write_again(void *arg)
{
struct quic_write_again_args *args = arg;
@ -847,6 +873,7 @@ static int quic_write_again(void *arg)
return 0;
}
QUIC_NEEDS_LOCK
static int quic_write_blocking(QUIC_CONNECTION *qc, const void *buf, size_t len,
size_t *written)
{
@ -915,6 +942,7 @@ static void aon_write_finish(QUIC_CONNECTION *qc)
qc->aon_buf_len = 0;
}
QUIC_NEEDS_LOCK
static int quic_write_nonblocking_aon(QUIC_CONNECTION *qc, const void *buf,
size_t len, size_t *written)
{
@ -1001,6 +1029,7 @@ static int quic_write_nonblocking_aon(QUIC_CONNECTION *qc, const void *buf,
return QUIC_RAISE_NORMAL_ERROR(qc, SSL_ERROR_WANT_WRITE);
}
QUIC_NEEDS_LOCK
static int quic_write_nonblocking_epw(QUIC_CONNECTION *qc, const void *buf, size_t len,
size_t *written)
{
@ -1060,6 +1089,7 @@ struct quic_read_again_args {
int peek;
};
QUIC_NEEDS_LOCK
static int quic_read_actual(QUIC_CONNECTION *qc,
QUIC_STREAM *stream,
void *buf, size_t buf_len,
@ -1114,6 +1144,7 @@ static int quic_read_actual(QUIC_CONNECTION *qc,
return 1;
}
QUIC_NEEDS_LOCK
static int quic_read_again(void *arg)
{
struct quic_read_again_args *args = arg;

View File

@ -50,6 +50,12 @@ struct quic_conn_st {
*/
QUIC_CHANNEL *ch;
/*
* The mutex used to synchronise access to the QUIC_CHANNEL. We own this but
* provide it to the channel.
*/
CRYPTO_RWLOCK *mutex;
/* Our single bidirectional application data stream. */
QUIC_STREAM *stream0;