Also, add fixes for CVE-2026-4271 and CVE-2026-5119. Resolves: RHEL-151634 Resolves: RHEL-159788 Resolves: RHEL-167775
362 lines
13 KiB
Diff
362 lines
13 KiB
Diff
From f99a814cadc4c19c5b70a42476f05e62ac2d5bab Mon Sep 17 00:00:00 2001
|
|
From: Carlos Garcia Campos <cgarcia@igalia.com>
|
|
Date: Mon, 16 Feb 2026 12:09:08 +0100
|
|
Subject: [PATCH] server: protect message io while reading and writing
|
|
|
|
Ensure the nghttp2 session is not destroyed while being used.
|
|
|
|
Closes #496
|
|
---
|
|
.../http2/soup-server-message-io-http2.c | 117 +++++++++++++-----
|
|
tests/http2-test.c | 54 ++++++++
|
|
2 files changed, 141 insertions(+), 30 deletions(-)
|
|
|
|
diff --git a/libsoup/server/http2/soup-server-message-io-http2.c b/libsoup/server/http2/soup-server-message-io-http2.c
|
|
index 913afb46..6f8d1bb6 100644
|
|
--- a/libsoup/server/http2/soup-server-message-io-http2.c
|
|
+++ b/libsoup/server/http2/soup-server-message-io-http2.c
|
|
@@ -69,6 +69,8 @@ typedef struct {
|
|
GHashTable *messages;
|
|
|
|
guint in_callback;
|
|
+ guint protected;
|
|
+ gboolean destroyed;
|
|
} SoupServerMessageIOHTTP2;
|
|
|
|
static void soup_server_message_io_http2_send_response (SoupServerMessageIOHTTP2 *io,
|
|
@@ -146,6 +148,8 @@ soup_server_message_io_http2_destroy (SoupServerMessageIO *iface)
|
|
{
|
|
SoupServerMessageIOHTTP2 *io = (SoupServerMessageIOHTTP2 *)iface;
|
|
|
|
+ io->destroyed = TRUE;
|
|
+
|
|
if (io->read_source) {
|
|
g_source_destroy (io->read_source);
|
|
g_source_unref (io->read_source);
|
|
@@ -160,10 +164,14 @@ soup_server_message_io_http2_destroy (SoupServerMessageIO *iface)
|
|
}
|
|
|
|
g_clear_object (&io->iostream);
|
|
- g_clear_pointer (&io->session, nghttp2_session_del);
|
|
- g_clear_pointer (&io->messages, g_hash_table_unref);
|
|
+ io->istream = NULL;
|
|
+ io->ostream = NULL;
|
|
|
|
- g_free (io);
|
|
+ if (io->protected == 0) {
|
|
+ g_clear_pointer (&io->session, nghttp2_session_del);
|
|
+ g_clear_pointer (&io->messages, g_hash_table_unref);
|
|
+ g_free (io);
|
|
+ }
|
|
}
|
|
|
|
static void
|
|
@@ -321,7 +329,33 @@ static const SoupServerMessageIOFuncs io_funcs = {
|
|
soup_server_message_io_http2_is_paused
|
|
};
|
|
|
|
+static void
|
|
+soup_server_message_io_http2_protect (SoupServerMessageIOHTTP2 *io)
|
|
+{
|
|
+ io->protected++;
|
|
+ g_object_ref (io->conn);
|
|
+}
|
|
+
|
|
static gboolean
|
|
+soup_server_message_io_http2_unprotect (SoupServerMessageIOHTTP2 *io)
|
|
+{
|
|
+ g_object_unref (io->conn);
|
|
+
|
|
+ if (--io->protected > 0)
|
|
+ return FALSE;
|
|
+
|
|
+ if (io->destroyed) {
|
|
+ g_clear_pointer (&io->session, nghttp2_session_del);
|
|
+ g_clear_pointer (&io->messages, g_hash_table_unref);
|
|
+ g_free (io);
|
|
+
|
|
+ return TRUE;
|
|
+ }
|
|
+
|
|
+ return FALSE;
|
|
+}
|
|
+
|
|
+static void
|
|
io_write (SoupServerMessageIOHTTP2 *io,
|
|
GError **error)
|
|
{
|
|
@@ -336,51 +370,57 @@ io_write (SoupServerMessageIOHTTP2 *io,
|
|
if (io->write_buffer_size == 0) {
|
|
/* Done */
|
|
io->write_buffer = NULL;
|
|
- return TRUE;
|
|
+ return;
|
|
}
|
|
}
|
|
|
|
+ if (!io->ostream)
|
|
+ return;
|
|
+
|
|
gssize ret = g_pollable_stream_write (io->ostream,
|
|
io->write_buffer + io->written_bytes,
|
|
io->write_buffer_size - io->written_bytes,
|
|
FALSE, NULL, error);
|
|
- if (ret < 0)
|
|
- return FALSE;
|
|
-
|
|
- io->written_bytes += ret;
|
|
- return TRUE;
|
|
+ if (ret > 0)
|
|
+ io->written_bytes += ret;
|
|
}
|
|
|
|
static gboolean
|
|
io_write_ready (GObject *stream,
|
|
SoupServerMessageIOHTTP2 *io)
|
|
{
|
|
- SoupServerConnection *conn = io->conn;
|
|
GError *error = NULL;
|
|
|
|
- g_object_ref (conn);
|
|
+ soup_server_message_io_http2_protect (io);
|
|
+
|
|
+ while (!error) {
|
|
+ if (io->destroyed)
|
|
+ break;
|
|
+
|
|
+ if (!nghttp2_session_want_write (io->session))
|
|
+ break;
|
|
|
|
- while (!error && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && nghttp2_session_want_write (io->session))
|
|
io_write (io, &error);
|
|
+ }
|
|
|
|
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
|
|
g_error_free (error);
|
|
- g_object_unref (conn);
|
|
+ soup_server_message_io_http2_unprotect (io);
|
|
return G_SOURCE_CONTINUE;
|
|
}
|
|
|
|
- if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
|
|
+ if (!io->destroyed) {
|
|
if (error)
|
|
h2_debug (io, NULL, "[SESSION] IO error: %s", error->message);
|
|
|
|
g_clear_pointer (&io->write_source, g_source_unref);
|
|
|
|
if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session)))
|
|
- soup_server_connection_disconnect (conn);
|
|
+ soup_server_connection_disconnect (io->conn);
|
|
}
|
|
|
|
g_clear_error (&error);
|
|
- g_object_unref (conn);
|
|
+ soup_server_message_io_http2_unprotect (io);
|
|
|
|
return G_SOURCE_REMOVE;
|
|
}
|
|
@@ -390,13 +430,12 @@ static gboolean io_write_idle_cb (SoupServerMessageIOHTTP2* io);
|
|
static void
|
|
io_try_write (SoupServerMessageIOHTTP2 *io)
|
|
{
|
|
- SoupServerConnection *conn = io->conn;
|
|
GError *error = NULL;
|
|
|
|
if (io->write_source)
|
|
return;
|
|
|
|
- if (io->in_callback && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
|
|
+ if (io->in_callback && !io->destroyed) {
|
|
if (!nghttp2_session_want_write (io->session))
|
|
return;
|
|
|
|
@@ -416,12 +455,19 @@ io_try_write (SoupServerMessageIOHTTP2 *io)
|
|
g_clear_pointer (&io->write_idle_source, g_source_unref);
|
|
}
|
|
|
|
- g_object_ref (conn);
|
|
+ soup_server_message_io_http2_protect (io);
|
|
+
|
|
+ while (!error) {
|
|
+ if (io->destroyed)
|
|
+ break;
|
|
+
|
|
+ if (!nghttp2_session_want_write (io->session))
|
|
+ break;
|
|
|
|
- while (!error && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && !io->in_callback && nghttp2_session_want_write (io->session))
|
|
io_write (io, &error);
|
|
+ }
|
|
|
|
- if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
|
|
+ if (!io->destroyed) {
|
|
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
|
|
g_clear_error (&error);
|
|
io->write_source = g_pollable_output_stream_create_source (G_POLLABLE_OUTPUT_STREAM (io->ostream), NULL);
|
|
@@ -434,11 +480,11 @@ io_try_write (SoupServerMessageIOHTTP2 *io)
|
|
h2_debug (io, NULL, "[SESSION] IO error: %s", error->message);
|
|
|
|
if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session)))
|
|
- soup_server_connection_disconnect (conn);
|
|
+ soup_server_connection_disconnect (io->conn);
|
|
}
|
|
|
|
g_clear_error (&error);
|
|
- g_object_unref (conn);
|
|
+ soup_server_message_io_http2_unprotect (io);
|
|
}
|
|
|
|
static gboolean
|
|
@@ -481,31 +527,37 @@ static gboolean
|
|
io_read_ready (GObject *stream,
|
|
SoupServerMessageIOHTTP2 *io)
|
|
{
|
|
- SoupServerConnection *conn = io->conn;
|
|
gboolean progress = TRUE;
|
|
GError *error = NULL;
|
|
|
|
- g_object_ref (conn);
|
|
+ soup_server_message_io_http2_protect (io);
|
|
+
|
|
+ while (progress) {
|
|
+ if (io->destroyed)
|
|
+ break;
|
|
+
|
|
+ if (!nghttp2_session_want_read (io->session))
|
|
+ break;
|
|
|
|
- while (progress && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && nghttp2_session_want_read (io->session))
|
|
progress = io_read (io, &error);
|
|
+ }
|
|
|
|
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
|
|
g_error_free (error);
|
|
- g_object_unref (conn);
|
|
+ soup_server_message_io_http2_unprotect (io);
|
|
return G_SOURCE_CONTINUE;
|
|
}
|
|
|
|
- if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) {
|
|
+ if (!io->destroyed) {
|
|
if (error)
|
|
h2_debug (io, NULL, "[SESSION] IO error: %s", error->message);
|
|
|
|
if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session)))
|
|
- soup_server_connection_disconnect (conn);
|
|
+ soup_server_connection_disconnect (io->conn);
|
|
}
|
|
|
|
g_clear_error (&error);
|
|
- g_object_unref (conn);
|
|
+ soup_server_message_io_http2_unprotect (io);
|
|
|
|
return G_SOURCE_REMOVE;
|
|
}
|
|
@@ -931,5 +983,10 @@ soup_server_message_io_http2_new (SoupServerConnection *conn,
|
|
nghttp2_submit_settings (io->session, NGHTTP2_FLAG_NONE, settings, G_N_ELEMENTS (settings));
|
|
io_try_write (io);
|
|
|
|
+#ifdef __clang_analyzer__
|
|
+ // Suppress false positive about io being destroyed here, since at this point we have only
|
|
+ // send the initial settings and not callback is called.
|
|
+ [[clang::suppress]]
|
|
+#endif
|
|
return (SoupServerMessageIO *)io;
|
|
}
|
|
diff --git a/tests/http2-test.c b/tests/http2-test.c
|
|
index dafcb95a..ee15e072 100644
|
|
--- a/tests/http2-test.c
|
|
+++ b/tests/http2-test.c
|
|
@@ -1368,6 +1368,40 @@ do_broken_pseudo_header_test (Test *test, gconstpointer data)
|
|
g_uri_unref (uri);
|
|
}
|
|
|
|
+static void
|
|
+disconnect_on_got_headers (SoupServerMessage *msg, gpointer user_data)
|
|
+{
|
|
+ GUri *uri;
|
|
+ SoupServerConnection *conn;
|
|
+
|
|
+ uri = soup_server_message_get_uri (msg);
|
|
+ if (!g_str_equal (g_uri_get_path (uri), "/close-on-got-headers"))
|
|
+ return;
|
|
+
|
|
+ conn = soup_server_message_get_connection (msg);
|
|
+ soup_server_connection_disconnect (conn);
|
|
+}
|
|
+
|
|
+static void
|
|
+do_server_disconnect_on_got_headers_test (Test *test, gconstpointer data)
|
|
+{
|
|
+ SoupMessage *msg;
|
|
+ GUri *uri;
|
|
+ GBytes *response;
|
|
+ GError *error = NULL;
|
|
+
|
|
+ uri = g_uri_parse_relative (base_uri, "/close-on-got-headers", SOUP_HTTP_URI_FLAGS, NULL);
|
|
+ msg = soup_message_new_from_uri (SOUP_METHOD_GET, uri);
|
|
+
|
|
+ response = soup_test_session_async_send (test->session, msg, NULL, &error);
|
|
+ g_assert_error (error, G_IO_ERROR, G_IO_ERROR_PARTIAL_INPUT);
|
|
+
|
|
+ g_clear_error (&error);
|
|
+ g_bytes_unref (response);
|
|
+ g_object_unref (msg);
|
|
+ g_uri_unref (uri);
|
|
+}
|
|
+
|
|
static gboolean
|
|
unpause_message (SoupServerMessage *msg)
|
|
{
|
|
@@ -1505,12 +1539,26 @@ server_handler (SoupServer *server,
|
|
shutdown (fd, SHUT_WR);
|
|
#endif
|
|
|
|
+ soup_server_message_set_response (msg, "text/plain",
|
|
+ SOUP_MEMORY_STATIC,
|
|
+ "Success!", 8);
|
|
+ } else if (strcmp (path, "/close-on-got-headers") == 0) {
|
|
soup_server_message_set_response (msg, "text/plain",
|
|
SOUP_MEMORY_STATIC,
|
|
"Success!", 8);
|
|
}
|
|
}
|
|
|
|
+static void
|
|
+server_request_started (SoupServer *server,
|
|
+ SoupServerMessage *msg,
|
|
+ SoupServerConnection *conn,
|
|
+ gpointer user_data)
|
|
+{
|
|
+ g_signal_connect (msg, "got-headers",
|
|
+ G_CALLBACK (disconnect_on_got_headers), NULL);
|
|
+}
|
|
+
|
|
static gboolean
|
|
server_basic_auth_callback (SoupAuthDomain *auth_domain,
|
|
SoupServerMessage *msg,
|
|
@@ -1537,6 +1585,8 @@ main (int argc, char **argv)
|
|
return 0;
|
|
|
|
server = soup_test_server_new (SOUP_TEST_SERVER_IN_THREAD | SOUP_TEST_SERVER_HTTP2);
|
|
+ g_signal_connect (server, "request-started",
|
|
+ G_CALLBACK (server_request_started), NULL);
|
|
auth = soup_auth_domain_basic_new ("realm", "http2-test",
|
|
"auth-callback", server_basic_auth_callback,
|
|
NULL);
|
|
@@ -1697,6 +1747,10 @@ main (int argc, char **argv)
|
|
setup_session,
|
|
do_broken_pseudo_header_test,
|
|
teardown_session);
|
|
+ g_test_add ("/http2/server-disconnect-on-got-headers", Test, NULL,
|
|
+ setup_session,
|
|
+ do_server_disconnect_on_got_headers_test,
|
|
+ teardown_session);
|
|
|
|
ret = g_test_run ();
|
|
|
|
--
|
|
GitLab
|
|
|