PR #24633 opened by Martin Storsjö (mstorsjo) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24633 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24633.patch
OpenSSL uses a thread specific queue for errors. We mostly do handle them and drain them, but on shutdown we can trigger an error we don't handle. Don't let old queued errors trigger spurious failures (either caused by our own code, or other uses of OpenSSL in the same thread), and avoid enqueueing errors from calling SSL_shutdown in cases when it shouldn't be called. This is mostly a cleanliness fix, I don't have a concrete case where this would cause problems - but it's plausible if new URLContexts/AVIOContexts are used on the same thread as earlier URLContexts/AVIOContexts or other use of OpenSSL. From defbd70bfbc363a19b1d92343da1cdb4f13a3665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <[email protected]> Date: Mon, 21 Sep 2026 15:49:04 +0300 Subject: [PATCH 1/2] tls_openssl: Call ERR_clear_error before OpenSSL IO functions OpenSSL stores errors in a thread specific queue. If an earlier function call has set an error that we haven't observed/consumed, then this can affect the output of SSL_get_error on the same thread. (This is an issue in particular for nonblocking IO, where a SSL_WANT_WRITE case instead can return an old queued fatal error.) Such fatal errors can be set if SSL_shutdown is called on a connection that isn't in the right state for doing a proper shutdown. The documentation for SSL_get_error, https://docs.openssl.org/3.4/man3/SSL_get_error/, explicitly says: > The current thread's error queue must be empty before the TLS/SSL > I/O operation is attempted, or SSL_get_error() will not work reliably. --- libavformat/tls_openssl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c index 48b4a2226a..f417953404 100644 --- a/libavformat/tls_openssl.c +++ b/libavformat/tls_openssl.c @@ -649,6 +649,7 @@ static int dtls_handshake(URLContext *h) goto end; } + ERR_clear_error(); ret = SSL_do_handshake(c->ssl); if (ret == 1) { av_log(c, AV_LOG_TRACE, "Handshake success\n"); @@ -907,6 +908,7 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op } av_log(c, AV_LOG_VERBOSE, "Setup ok, MTU=%d\n", c->tls_shared.mtu); } else { + ERR_clear_error(); ret = s->listen ? SSL_accept(c->ssl) : SSL_connect(c->ssl); if (ret == 0) { av_log(h, AV_LOG_ERROR, "Unable to negotiate TLS/SSL session\n"); @@ -941,6 +943,7 @@ static int tls_read(URLContext *h, uint8_t *buf, int size) // Set or clear the AVIO_FLAG_NONBLOCK on the underlying socket uc->flags &= ~AVIO_FLAG_NONBLOCK; uc->flags |= h->flags & AVIO_FLAG_NONBLOCK; + ERR_clear_error(); ret = SSL_read(c->ssl, buf, size); if (ret > 0) return ret; @@ -965,6 +968,7 @@ static int tls_write(URLContext *h, const uint8_t *buf, int size) size = FFMIN(size, mtu_size); } + ERR_clear_error(); ret = SSL_write(c->ssl, buf, size); if (ret > 0) return ret; -- 2.52.0 From 3654b34ddb4ad43a51b8ebb3e014f29ed6f5495d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <[email protected]> Date: Mon, 21 Sep 2026 15:56:51 +0300 Subject: [PATCH 2/2] tls_openssl: Only do SSL_shutdown if the connection was healthy If SSL_connect or SSL_accept haven't completed, or if we've run into a fatal error, we aren't supposed to call this function. This should avoid accumulating errors in the OpenSSL thread specific error queues, when we clean up after an unsuccessful connection. The documentation for SSL_get_error, https://docs.openssl.org/3.4/man3/SSL_get_error/, explicitly says: > SSL_ERROR_SYSCALL > > [...] If this error occurs then no further I/O operations should > be performed on the connection and SSL_shutdown() must not be called. > > SSL_ERROR_SSL > > [...] If this error occurs then no further I/O operations should > be performed on the connection and SSL_shutdown() must not be called. --- libavformat/tls_openssl.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c index f417953404..789dae48a0 100644 --- a/libavformat/tls_openssl.c +++ b/libavformat/tls_openssl.c @@ -425,6 +425,7 @@ typedef struct TLSContext { TLSShared tls_shared; SSL_CTX *ctx; SSL *ssl; + int do_shutdown; BIO_METHOD* url_bio_method; int io_err; char error_message[256]; @@ -483,11 +484,17 @@ static int print_ssl_error(URLContext *h, int ret) { TLSContext *c = h->priv_data; int printed = 0, e, averr = AVERROR(EIO); + int err = SSL_get_error(c->ssl, ret); if (h->flags & AVIO_FLAG_NONBLOCK) { - int err = SSL_get_error(c->ssl, ret); if (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE) return AVERROR(EAGAIN); } + switch (err) { + case SSL_ERROR_SSL: + case SSL_ERROR_SYSCALL: + c->do_shutdown = 0; + break; + } while ((e = ERR_get_error()) != 0) { av_log(h, AV_LOG_ERROR, "%s\n", ERR_error_string(e, NULL)); printed = 1; @@ -507,7 +514,8 @@ static int tls_close(URLContext *h) { TLSContext *c = h->priv_data; if (c->ssl) { - SSL_shutdown(c->ssl); + if (c->do_shutdown) + SSL_shutdown(c->ssl); SSL_free(c->ssl); } if (c->ctx) @@ -653,6 +661,7 @@ static int dtls_handshake(URLContext *h) ret = SSL_do_handshake(c->ssl); if (ret == 1) { av_log(c, AV_LOG_TRACE, "Handshake success\n"); + c->do_shutdown = 1; break; } err = SSL_get_error(c->ssl, ret); @@ -918,6 +927,7 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op ret = print_ssl_error(h, ret); goto fail; } + c->do_shutdown = 1; } return 0; -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
