The way that premature termination was handled in TLS connections was
changed to handle an ordering problem during graceful shutdown in the
migration code.

Unfortunately one of the codepaths returned -1 to indicate an error
condition, but failed to set the 'errp' parameter.

This broke error handling in the qio_channel_tls_handshake function,
as the QTask callback would no longer see that an error was raised.
As a result, the client will go on to try to use the already closed
TLS connection, resulting in misleading errors.

This was evidenced in the I/O test 233 which showed changes such as

-qemu-nbd: Certificate does not match the hostname localhost
+qemu-nbd: Failed to read initial magic: Unable to read from socket: Connection 
reset by peer

Fixes: 7e0c22d585581b8083ffdeb332ea497218665daf
Acked-by: Peter Xu <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
---
 crypto/tlssession.c |  8 +++++---
 io/channel-tls.c    | 13 +++++++------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index ac38c2121d..8c0bf457ad 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -569,8 +569,6 @@ 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) {
-            return QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION;
         } else {
             if (session->rerr) {
                 error_propagate(errp, session->rerr);
@@ -580,7 +578,11 @@ qcrypto_tls_session_read(QCryptoTLSSession *session,
                            "Cannot read from TLS channel: %s",
                            gnutls_strerror(ret));
             }
-            return -1;
+            if (ret == GNUTLS_E_PREMATURE_TERMINATION) {
+                return QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION;
+            } else {
+                return -1;
+            }
         }
     }
 
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 1fbed4be0c..70fad38d18 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -368,6 +368,7 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
                                      int flags,
                                      Error **errp)
 {
+    ERRP_GUARD();
     QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
     size_t i;
     ssize_t got = 0;
@@ -384,13 +385,13 @@ 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) {
+            if (ret == QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION &&
+                qio_channel_tls_allow_premature_termination(tioc, flags)) {
+                error_free(*errp);
+                *errp = NULL;
+                return got;
+            }
             return -1;
         }
         got += ret;
-- 
2.50.1


Reply via email to