qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/1 for 8.1] TLS crash fix
@ 2023-08-01 17:46 Daniel P. Berrangé
  2023-08-01 17:46 ` [PULL 1/1] io: remove io watch if TLS channel is closed during handshake Daniel P. Berrangé
  2023-08-02 15:14 ` [PULL 0/1 for 8.1] TLS crash fix Richard Henderson
  0 siblings, 2 replies; 3+ messages in thread
From: Daniel P. Berrangé @ 2023-08-01 17:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P. Berrangé, qemu-stable, Marc-André Lureau,
	Mauro Matteo Cascella, Michael Tokarev

The following changes since commit 802341823f1720511dd5cf53ae40285f7978c61b:

  Merge tag 'pull-tcg-20230731' of https://gitlab.com/rth7680/qemu into staging (2023-07-31 14:02:51 -0700)

are available in the Git repository at:

  https://gitlab.com/berrange/qemu tags/io-tls-hs-crash-pull-request

for you to fetch changes up to 10be627d2b5ec2d6b3dce045144aa739eef678b4:

  io: remove io watch if TLS channel is closed during handshake (2023-08-01 18:45:27 +0100)

----------------------------------------------------------------
Fix crash during early close of TLS channel

----------------------------------------------------------------

Daniel P. Berrangé (1):
  io: remove io watch if TLS channel is closed during handshake

 include/io/channel-tls.h |  1 +
 io/channel-tls.c         | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.41.0



^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PULL 1/1] io: remove io watch if TLS channel is closed during handshake
  2023-08-01 17:46 [PULL 0/1 for 8.1] TLS crash fix Daniel P. Berrangé
@ 2023-08-01 17:46 ` Daniel P. Berrangé
  2023-08-02 15:14 ` [PULL 0/1 for 8.1] TLS crash fix Richard Henderson
  1 sibling, 0 replies; 3+ messages in thread
From: Daniel P. Berrangé @ 2023-08-01 17:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P. Berrangé, qemu-stable, Marc-André Lureau,
	Mauro Matteo Cascella, Michael Tokarev, jiangyegen

The TLS handshake make take some time to complete, during which time an
I/O watch might be registered with the main loop. If the owner of the
I/O channel invokes qio_channel_close() while the handshake is waiting
to continue the I/O watch must be removed. Failing to remove it will
later trigger the completion callback which the owner is not expecting
to receive. In the case of the VNC server, this results in a SEGV as
vnc_disconnect_start() tries to shutdown a client connection that is
already gone / NULL.

CVE-2023-3354
Reported-by: jiangyegen <jiangyegen@huawei.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/io/channel-tls.h |  1 +
 io/channel-tls.c         | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h
index 5672479e9e..26c67f17e2 100644
--- a/include/io/channel-tls.h
+++ b/include/io/channel-tls.h
@@ -48,6 +48,7 @@ struct QIOChannelTLS {
     QIOChannel *master;
     QCryptoTLSSession *session;
     QIOChannelShutdown shutdown;
+    guint hs_ioc_tag;
 };
 
 /**
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 9805dd0a3f..847d5297c3 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -198,12 +198,13 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
         }
 
         trace_qio_channel_tls_handshake_pending(ioc, status);
-        qio_channel_add_watch_full(ioc->master,
-                                   condition,
-                                   qio_channel_tls_handshake_io,
-                                   data,
-                                   NULL,
-                                   context);
+        ioc->hs_ioc_tag =
+            qio_channel_add_watch_full(ioc->master,
+                                       condition,
+                                       qio_channel_tls_handshake_io,
+                                       data,
+                                       NULL,
+                                       context);
     }
 }
 
@@ -218,6 +219,7 @@ static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc,
     QIOChannelTLS *tioc = QIO_CHANNEL_TLS(
         qio_task_get_source(task));
 
+    tioc->hs_ioc_tag = 0;
     g_free(data);
     qio_channel_tls_handshake_task(tioc, task, context);
 
@@ -378,6 +380,10 @@ static int qio_channel_tls_close(QIOChannel *ioc,
 {
     QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
 
+    if (tioc->hs_ioc_tag) {
+        g_clear_handle_id(&tioc->hs_ioc_tag, g_source_remove);
+    }
+
     return qio_channel_close(tioc->master, errp);
 }
 
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PULL 0/1 for 8.1] TLS crash fix
  2023-08-01 17:46 [PULL 0/1 for 8.1] TLS crash fix Daniel P. Berrangé
  2023-08-01 17:46 ` [PULL 1/1] io: remove io watch if TLS channel is closed during handshake Daniel P. Berrangé
@ 2023-08-02 15:14 ` Richard Henderson
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2023-08-02 15:14 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel
  Cc: qemu-stable, Marc-André Lureau, Mauro Matteo Cascella,
	Michael Tokarev

On 8/1/23 10:46, Daniel P. Berrangé wrote:
> The following changes since commit 802341823f1720511dd5cf53ae40285f7978c61b:
> 
>    Merge tag 'pull-tcg-20230731' ofhttps://gitlab.com/rth7680/qemu  into staging (2023-07-31 14:02:51 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/berrange/qemu  tags/io-tls-hs-crash-pull-request
> 
> for you to fetch changes up to 10be627d2b5ec2d6b3dce045144aa739eef678b4:
> 
>    io: remove io watch if TLS channel is closed during handshake (2023-08-01 18:45:27 +0100)
> 
> ----------------------------------------------------------------
> Fix crash during early close of TLS channel

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-08-02 15:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-01 17:46 [PULL 0/1 for 8.1] TLS crash fix Daniel P. Berrangé
2023-08-01 17:46 ` [PULL 1/1] io: remove io watch if TLS channel is closed during handshake Daniel P. Berrangé
2023-08-02 15:14 ` [PULL 0/1 for 8.1] TLS crash fix Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).