From a58d3959a7ba248437ef2cd8de736013d675ba73 Mon Sep 17 00:00:00 2001 From: Davide Beatrici Date: Sat, 27 Jun 2026 00:53:18 +0200 Subject: [PATCH] net.openssl: fix double-free on shutdown() (#27482) --- vlib/net/openssl/openssl.c.v | 34 ++++++++++++++- vlib/net/openssl/ssl_connection.c.v | 68 ++++++++++++++++++++--------- 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/vlib/net/openssl/openssl.c.v b/vlib/net/openssl/openssl.c.v index b87b0ceab..f51411739 100644 --- a/vlib/net/openssl/openssl.c.v +++ b/vlib/net/openssl/openssl.c.v @@ -127,6 +127,10 @@ fn C.X509_free(const_cert &C.X509) fn C.ERR_clear_error() +fn C.ERR_get_error() u64 + +fn C.ERR_error_string_n(e u64, buf &char, len usize) + fn C.SSL_get_error(ssl &C.SSL, ret i32) i32 fn C.SSL_get_verify_result(ssl &C.SSL) i32 @@ -163,6 +167,27 @@ fn init() { C.v_net_openssl_init_ssl() } +// ssl_get_error_queue drains the current thread's OpenSSL error queue and +// returns a human-readable, semicolon-separated description of every queued +// entry (oldest first), or an empty string if the queue is empty. It always +// leaves the queue empty afterwards: this both turns the otherwise opaque +// `SSL_ERROR_SSL`/`SSL_ERROR_SYSCALL` codes into actionable messages, and +// prevents a leftover entry from making a later SSL_get_error() misreport the +// status of the next, unrelated I/O operation. +fn ssl_get_error_queue() string { + mut reasons := []string{} + for { + code := C.ERR_get_error() + if code == 0 { + break + } + mut buf := [256]char{} + C.ERR_error_string_n(code, &buf[0], usize(buf.len)) + reasons << unsafe { cstring_to_vstring(&buf[0]) } + } + return reasons.join('; ') +} + // ssl_error returns non error ssl code or error if unrecoverable and we should panic fn ssl_error(ret int, ssl voidptr) !SSLError { res := C.SSL_get_error(ssl, ret) @@ -171,10 +196,15 @@ fn ssl_error(ret int, ssl voidptr) !SSLError { } match unsafe { SSLError(res) } { .ssl_error_syscall { - return error_with_code('net.openssl unrecoverable syscall (${res})', res) + details := ssl_get_error_queue() + suffix := if details == '' { '' } else { ': ${details}' } + return error_with_code('net.openssl unrecoverable syscall (${res})${suffix}', res) } .ssl_error_ssl { - return error_with_code('net.openssl unrecoverable ssl protocol error (${res})', res) + details := ssl_get_error_queue() + suffix := if details == '' { '' } else { ': ${details}' } + return error_with_code('net.openssl unrecoverable ssl protocol error (${res})${suffix}', + res) } else { return unsafe { SSLError(res) } diff --git a/vlib/net/openssl/ssl_connection.c.v b/vlib/net/openssl/ssl_connection.c.v index 5fa43a353..91914a7e9 100644 --- a/vlib/net/openssl/ssl_connection.c.v +++ b/vlib/net/openssl/ssl_connection.c.v @@ -98,37 +98,55 @@ pub fn (mut s SSLConn) shutdown() ! { eprintln(@METHOD) } - if s.ssl != 0 { + if s.ssl != unsafe { nil } { deadline := ssl_timeout_deadline(s.duration) - for { - mut res := C.SSL_shutdown(voidptr(s.ssl)) + mut shutdown_done := false + + for !shutdown_done { + // Clear the thread's OpenSSL error queue so ssl_error()/SSL_get_error() + // below reflect only this call and not a stale entry from an earlier + // operation, which could be misread as a fatal SSL_ERROR_SSL. + C.ERR_clear_error() + res := C.SSL_shutdown(voidptr(s.ssl)) if res == 1 { + shutdown_done = true break } - err_res := ssl_error(res, s.ssl) or { - break // We break to free rest of resources - } - if err_res == .ssl_error_want_read { - s.wait_for_read(ssl_remaining_timeout(deadline))! - continue - } else if err_res == .ssl_error_want_write { - s.wait_for_write(ssl_remaining_timeout(deadline))! - continue - } - if s.ssl != 0 { - unsafe { C.SSL_free(voidptr(s.ssl)) } - } - if s.sslctx != 0 { - C.SSL_CTX_free(s.sslctx) + // res == 0 means our close_notify was sent, but the peer's has not + // been received yet, so another SSL_shutdown() call is needed to + // finish the bidirectional shutdown; res < 0 signals an error or a + // retryable condition. In both cases SSL_get_error() tells us whether + // the socket must first become readable/writable before retrying. + // Routing res == 0 through ssl_error() instead of looping immediately + // avoids busy-spinning on non-blocking sockets. + err_res := ssl_error(res, s.ssl) or { break } + + match err_res { + .ssl_error_want_read { + s.wait_for_read(ssl_remaining_timeout(deadline)) or { break } + } + .ssl_error_want_write { + s.wait_for_write(ssl_remaining_timeout(deadline)) or { break } + } + else { + break + } } - return error('net.openssl Could not connect using SSL. (${err_res}),err') } - C.SSL_free(voidptr(s.ssl)) + + // Always free SSL object first. + if s.ssl != unsafe { nil } { + unsafe { C.SSL_free(voidptr(s.ssl)) } + s.ssl = unsafe { nil } + } } - if s.sslctx != 0 { + + if s.sslctx != unsafe { nil } { C.SSL_CTX_free(s.sslctx) + s.sslctx = unsafe { nil } } + if s.owns_socket { net.shutdown(s.handle) net.close(s.handle)! @@ -274,6 +292,8 @@ fn (mut s SSLConn) complete_connect() ! { deadline := ssl_timeout_deadline(s.duration) for { + // Clear the error queue so SSL_get_error() reflects only this call. + C.ERR_clear_error() mut res := C.SSL_connect(voidptr(s.ssl)) if res == 1 { break @@ -294,6 +314,8 @@ fn (mut s SSLConn) complete_connect() ! { if s.config.validate { mut pcert := &C.X509(unsafe { nil }) for { + // Clear the error queue so SSL_get_error() reflects only this call. + C.ERR_clear_error() mut res := C.SSL_do_handshake(voidptr(s.ssl)) if res == 1 { break @@ -356,6 +378,8 @@ pub fn (mut s SSLConn) socket_read_into_ptr(buf_ptr &u8, len int) !int { deadline := ssl_timeout_deadline(s.duration) // s.wait_for_read(deadline - time.now())! for { + // Clear the error queue so SSL_get_error() reflects only this call. + C.ERR_clear_error() res = C.SSL_read(voidptr(s.ssl), buf_ptr, len) if res > 0 { return res @@ -426,6 +450,8 @@ pub fn (mut s SSLConn) write_ptr(bytes &u8, len int) !int { for total_sent < len { ptr := ptr_base + total_sent remaining := len - total_sent + // Clear the error queue so SSL_get_error() reflects only this call. + C.ERR_clear_error() mut sent := C.SSL_write(voidptr(s.ssl), ptr, remaining) if sent <= 0 { // SSL_write did not fully complete: OpenSSL may already have -- 2.39.5