[Spice-devel] [PATCH 05/13] server/reds: RFC: improve ssl_{read, write} return values

[Marc-André Lureau]

There is no known bug related to this patch, it might cause more harm
than good.

The implementation of ssl_writev() looks incorrect: it never returned
an error (return_code is always >= 0). But, it should return the
number of bytes written before it reached EAGAIN, otherwise, we risk
of sending corrupted data.

Finally, since the SSL usage is hidden from the reader/writter, and it
assumes it is a fd-kind/libc of API (checking ERRNO), we should make
sure errno is set to a correct value in that case (EAGAIN for instance).

Note: we should be careful with the polling of SSL fd, since
SSL_read() or SSL_write() both can return WANT_READ or WANT_WRITE.  We
should poll both IN and OUT, not sure how it's handled now.
---
 server/reds.c |   31 +++++++++++++++++++------------
 1 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 45e4964..2aefe31 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -319,6 +319,13 @@ static ssize_t stream_read_cb(RedsStream *s, void *buf, size_t size)
     return read(s->socket, buf, size);
 }
 
+static void ssl_error_eagain(int error)
+{
+    /* can this help? */
+    if (error == SSL_ERROR_WANT_READ || error == SSL_ERROR_WANT_WRITE)
+        errno = EAGAIN;
+}
+
 static ssize_t stream_ssl_write_cb(RedsStream *s, const void *buf, size_t size)
 {
     int return_code;
@@ -326,8 +333,10 @@ static ssize_t stream_ssl_write_cb(RedsStream *s, const void *buf, size_t size)
 
     return_code = SSL_write(s->ssl, buf, size);
 
-    if (return_code < 0)
+    if (return_code < 0) {
         ssl_error = SSL_get_error(s->ssl, return_code);
+        ssl_error_eagain(ssl_error); /* just to be sure */
+    }
 
     return return_code;
 }
@@ -339,8 +348,10 @@ static ssize_t stream_ssl_read_cb(RedsStream *s, void *buf, size_t size)
 
     return_code = SSL_read(s->ssl, buf, size);
 
-    if (return_code < 0)
+    if (return_code < 0) {
         ssl_error = SSL_get_error(s->ssl, return_code);
+        ssl_error_eagain(ssl_error); /* just to be sure */
+    }
 
     return return_code;
 }
@@ -349,24 +360,20 @@ static ssize_t stream_ssl_writev_cb(RedsStream *s, const struct iovec *vector, i
 {
     int i;
     int n;
-    ssize_t return_code = 0;
+    ssize_t ret = 0;
     int ssl_error;
 
     for (i = 0; i < count; ++i) {
         n = SSL_write(s->ssl, vector[i].iov_base, vector[i].iov_len);
         if (n <= 0) {
             ssl_error = SSL_get_error(s->ssl, n);
-            if (return_code <= 0) {
-                return n;
-            } else {
-                break;
-            }
-        } else {
-            return_code += n;
-        }
+            ssl_error_eagain(ssl_error); /* just to be sure we have EAGAIN */
+            return ret == 0 ? n : ret;
+        } else
+            ret += n;
     }
 
-    return return_code;
+    return ret;
 }
 
 static void reds_stream_remove_watch(RedsStream* s)
-- 
1.7.4