qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: "Fabiano Rosas" <farosas@suse.de>,
	peterx@redhat.com, "David Hildenbrand" <david@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Juraj Marcin" <jmarcin@redhat.com>
Subject: [PULL 27/45] io/crypto: Move tls premature termination handling into QIO layer
Date: Fri,  3 Oct 2025 11:39:30 -0400	[thread overview]
Message-ID: <20251003153948.1304776-28-peterx@redhat.com> (raw)
In-Reply-To: <20251003153948.1304776-1-peterx@redhat.com>

QCryptoTLSSession allows TLS premature termination in two cases, one of the
case is when the channel shutdown() is invoked on READ side.

It's possible the shutdown() happened after the read thread blocked at
gnutls_record_recv().  In this case, we should allow the premature
termination to happen.

The problem is by the time qcrypto_tls_session_read() was invoked,
tioc->shutdown may not have been set, so this may instead be treated as an
error if there is concurrent shutdown() calls.

To allow the flag to reflect the latest status of tioc->shutdown, move the
check upper into the QIOChannel level, so as to read the flag only after
QEMU gets an GNUTLS_E_PREMATURE_TERMINATION.

When at it, introduce qio_channel_tls_allow_premature_termination() helper
to make the condition checks easier to read.  When doing so, change the
qatomic_load_acquire() to qatomic_read(): here we don't need any ordering
of memory accesses, but reading a flag.  qatomic_read() would suffice
because it guarantees fetching from memory.  Nothing else we should need to
order on memory access.

This patch will fix a qemu qtest warning when running the preempt tls test,
reporting premature termination:

QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test --full -r /x86_64/migration/postcopy/preempt/tls/psk
...
qemu-kvm: Cannot read from TLS channel: The TLS connection was non-properly terminated.
...

In this specific case, the error was set by postcopy_preempt_thread, which
normally will be concurrently shutdown()ed by the main thread.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20250918203937.200833-2-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/crypto/tlssession.h | 10 +++-------
 crypto/tlssession.c         |  7 ++-----
 io/channel-tls.c            | 21 +++++++++++++++++++--
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index 2f62ce2d67..2e9fe11cf6 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -110,6 +110,7 @@
 typedef struct QCryptoTLSSession QCryptoTLSSession;
 
 #define QCRYPTO_TLS_SESSION_ERR_BLOCK -2
+#define QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION -3
 
 /**
  * qcrypto_tls_session_new:
@@ -259,7 +260,6 @@ ssize_t qcrypto_tls_session_write(QCryptoTLSSession *sess,
  * @sess: the TLS session object
  * @buf: to fill with plain text received
  * @len: the length of @buf
- * @gracefulTermination: treat premature termination as graceful EOF
  * @errp: pointer to hold returned error object
  *
  * Receive up to @len bytes of data from the remote peer
@@ -267,22 +267,18 @@ ssize_t qcrypto_tls_session_write(QCryptoTLSSession *sess,
  * qcrypto_tls_session_set_callbacks(), decrypt it and
  * store it in @buf.
  *
- * If @gracefulTermination is true, then a premature termination
- * of the TLS session will be treated as indicating EOF, as
- * opposed to an error.
- *
  * It is an error to call this before
  * qcrypto_tls_session_handshake() returns
  * QCRYPTO_TLS_HANDSHAKE_COMPLETE
  *
  * Returns: the number of bytes received,
  * or QCRYPTO_TLS_SESSION_ERR_BLOCK if the receive would block,
- * or -1 on error.
+ * or QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION if a premature termination
+ * is detected, or -1 on error.
  */
 ssize_t qcrypto_tls_session_read(QCryptoTLSSession *sess,
                                  char *buf,
                                  size_t len,
-                                 bool gracefulTermination,
                                  Error **errp);
 
 /**
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index 86d407a142..ac38c2121d 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -552,7 +552,6 @@ ssize_t
 qcrypto_tls_session_read(QCryptoTLSSession *session,
                          char *buf,
                          size_t len,
-                         bool gracefulTermination,
                          Error **errp)
 {
     ssize_t ret;
@@ -570,9 +569,8 @@ qcrypto_tls_session_read(QCryptoTLSSession *session,
     if (ret < 0) {
         if (ret == GNUTLS_E_AGAIN) {
             return QCRYPTO_TLS_SESSION_ERR_BLOCK;
-        } else if ((ret == GNUTLS_E_PREMATURE_TERMINATION) &&
-                   gracefulTermination){
-            return 0;
+        } else if (ret == GNUTLS_E_PREMATURE_TERMINATION) {
+            return QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION;
         } else {
             if (session->rerr) {
                 error_propagate(errp, session->rerr);
@@ -789,7 +787,6 @@ ssize_t
 qcrypto_tls_session_read(QCryptoTLSSession *sess,
                          char *buf,
                          size_t len,
-                         bool gracefulTermination,
                          Error **errp)
 {
     error_setg(errp, "TLS requires GNUTLS support");
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 7135896f79..1fbed4be0c 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -346,6 +346,19 @@ static void qio_channel_tls_finalize(Object *obj)
     qcrypto_tls_session_free(ioc->session);
 }
 
+static bool
+qio_channel_tls_allow_premature_termination(QIOChannelTLS *tioc, int flags)
+{
+    if (flags & QIO_CHANNEL_READ_FLAG_RELAXED_EOF) {
+        return true;
+    }
+
+    if (qatomic_read(&tioc->shutdown) & QIO_CHANNEL_SHUTDOWN_READ) {
+        return true;
+    }
+
+    return false;
+}
 
 static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
                                      const struct iovec *iov,
@@ -364,8 +377,6 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
             tioc->session,
             iov[i].iov_base,
             iov[i].iov_len,
-            flags & QIO_CHANNEL_READ_FLAG_RELAXED_EOF ||
-            qatomic_load_acquire(&tioc->shutdown) & QIO_CHANNEL_SHUTDOWN_READ,
             errp);
         if (ret == QCRYPTO_TLS_SESSION_ERR_BLOCK) {
             if (got) {
@@ -373,6 +384,12 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
             } else {
                 return QIO_CHANNEL_ERR_BLOCK;
             }
+        } else if (ret == QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION) {
+            if (qio_channel_tls_allow_premature_termination(tioc, flags)) {
+                ret = 0;
+            } else {
+                return -1;
+            }
         } else if (ret < 0) {
             return -1;
         }
-- 
2.50.1



  parent reply	other threads:[~2025-10-03 15:49 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-03 15:39 [PULL 00/45] Staging patches Peter Xu
2025-10-03 15:39 ` [PULL 01/45] migration: push Error **errp into vmstate_subsection_load() Peter Xu
2025-10-03 15:39 ` [PULL 02/45] migration: push Error **errp into vmstate_load_state() Peter Xu
2025-10-03 15:39 ` [PULL 03/45] migration: push Error **errp into qemu_loadvm_state_header() Peter Xu
2025-10-03 15:39 ` [PULL 04/45] migration: push Error **errp into vmstate_load() Peter Xu
2025-10-03 15:39 ` [PULL 05/45] migration: push Error **errp into loadvm_process_command() Peter Xu
2025-10-03 15:39 ` [PULL 06/45] migration: push Error **errp into loadvm_handle_cmd_packaged() Peter Xu
2025-10-03 15:39 ` [PULL 07/45] migration: push Error **errp into qemu_loadvm_state() Peter Xu
2025-10-03 15:39 ` [PULL 08/45] migration: push Error **errp into qemu_load_device_state() Peter Xu
2025-10-03 15:39 ` [PULL 09/45] migration: push Error **errp into qemu_loadvm_state_main() Peter Xu
2025-10-03 15:39 ` [PULL 10/45] migration: push Error **errp into qemu_loadvm_section_start_full() Peter Xu
2025-10-03 15:39 ` [PULL 11/45] migration: push Error **errp into qemu_loadvm_section_part_end() Peter Xu
2025-10-03 15:39 ` [PULL 12/45] migration: Update qemu_file_get_return_path() docs and remove dead checks Peter Xu
2025-10-03 15:39 ` [PULL 13/45] migration: make loadvm_postcopy_handle_resume() void Peter Xu
2025-10-03 15:39 ` [PULL 14/45] migration: push Error **errp into ram_postcopy_incoming_init() Peter Xu
2025-10-03 15:39 ` [PULL 15/45] migration: push Error **errp into loadvm_postcopy_handle_advise() Peter Xu
2025-10-03 15:39 ` [PULL 16/45] migration: push Error **errp into loadvm_postcopy_handle_listen() Peter Xu
2025-10-03 15:39 ` [PULL 17/45] migration: push Error **errp into loadvm_postcopy_handle_run() Peter Xu
2025-10-03 15:39 ` [PULL 18/45] migration: push Error **errp into loadvm_postcopy_ram_handle_discard() Peter Xu
2025-10-03 15:39 ` [PULL 19/45] migration: push Error **errp into loadvm_handle_recv_bitmap() Peter Xu
2025-10-03 15:39 ` [PULL 20/45] migration: Return -1 on memory allocation failure in ram.c Peter Xu
2025-10-03 15:39 ` [PULL 21/45] migration: push Error **errp into loadvm_process_enable_colo() Peter Xu
2025-10-03 15:39 ` [PULL 22/45] migration: push Error **errp into loadvm_postcopy_handle_switchover_start() Peter Xu
2025-10-03 15:39 ` [PULL 23/45] migration: Capture error in postcopy_ram_listen_thread() Peter Xu
2025-10-03 15:39 ` [PULL 24/45] migration: Remove error variant of vmstate_save_state() function Peter Xu
2025-10-03 15:39 ` [PULL 25/45] migration: Add error-parameterized function variants in VMSD struct Peter Xu
2025-10-03 15:39 ` [PULL 26/45] backends/tpm: Propagate vTPM error on migration failure Peter Xu
2025-10-03 15:39 ` Peter Xu [this message]
2025-10-10  8:00   ` iotest 233 is failing (was: [PULL 27/45] io/crypto: Move tls premature termination handling into QIO layer) Thomas Huth
2025-10-10  8:35     ` iotest 233 is failing Thomas Huth
2025-10-03 15:39 ` [PULL 28/45] migration: Make migration_has_failed() work even for CANCELLING Peter Xu
2025-10-03 15:39 ` [PULL 29/45] migration: HMP: Adjust the order of output fields Peter Xu
2025-10-03 15:39 ` [PULL 30/45] migration/multifd/tls: Cleanup BYE message processing on sender side Peter Xu
2025-10-03 15:39 ` [PULL 31/45] migration: Fix state transition in postcopy_start() error handling Peter Xu
2025-10-03 15:39 ` [PULL 32/45] migration: ensure APIC is loaded prior to VFIO PCI devices Peter Xu
2025-10-03 15:39 ` [PULL 33/45] include/system/memory.h: Clarify address_space_destroy() behaviour Peter Xu
2025-10-03 15:39 ` [PULL 34/45] memory: New AS helper to serialize destroy+free Peter Xu
2025-10-03 15:39 ` [PULL 35/45] physmem: Destroy all CPU AddressSpaces on unrealize Peter Xu
2025-10-03 15:39 ` [PULL 36/45] migration: simplify error reporting after channel read Peter Xu
2025-10-03 15:39 ` [PULL 37/45] migration: multi-mode notifier Peter Xu
2025-10-03 15:39 ` [PULL 38/45] migration: add cpr_walk_fd Peter Xu
2025-10-03 15:39 ` [PULL 39/45] oslib: qemu_clear_cloexec Peter Xu
2025-10-03 15:39 ` [PULL 40/45] migration: cpr-exec-command parameter Peter Xu
2025-10-03 15:39 ` [PULL 41/45] migration: cpr-exec save and load Peter Xu
2025-10-03 15:39 ` [PULL 42/45] migration: cpr-exec mode Peter Xu
2025-10-03 15:39 ` [PULL 43/45] migration: cpr-exec docs Peter Xu
2025-10-03 15:39 ` [PULL 44/45] vfio: cpr-exec mode Peter Xu
2025-10-03 15:39 ` [PULL 45/45] migration-test: test cpr-exec Peter Xu
2025-10-04 17:53 ` [PULL 00/45] Staging patches Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251003153948.1304776-28-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=berrange@redhat.com \
    --cc=david@redhat.com \
    --cc=farosas@suse.de \
    --cc=jmarcin@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).