[Qemu-devel] [PATCH v1 RFC 33/34] char: don't assume telnet initialization will not block

["Daniel P. Berrange" <berrange@redhat.com>]

The current code for doing telnet initialization is writing to
a socket without checking the return status. While it is highly
unlikely to be a problem when writing to a bare socket, as the
buffers are large enough to prevent blocking, this cannot be
assumed safe with TLS sockets. So write the telnet initialization
code into a memory buffer and then use an I/O watch to fully
send the data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-char.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 67 insertions(+), 18 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index fada1a7..85fbbaf 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2805,19 +2805,68 @@ static void tcp_chr_update_read_handler(CharDriverState *chr)
     }
 }
 
-#define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
-static void tcp_chr_telnet_init(QIOChannel *ioc)
+typedef struct {
+    CharDriverState *chr;
+    char buf[12];
+    size_t buflen;
+} TCPCharDriverTelnetInit;
+
+static gboolean tcp_chr_telnet_init_io(QIOChannel *ioc,
+                                       GIOCondition cond G_GNUC_UNUSED,
+                                       gpointer user_data)
 {
-    char buf[3];
-    /* Send the telnet negotion to put telnet in binary, no echo, single char mode */
-    IACSET(buf, 0xff, 0xfb, 0x01);  /* IAC WILL ECHO */
-    qio_channel_write(ioc, buf, 3, NULL);
-    IACSET(buf, 0xff, 0xfb, 0x03);  /* IAC WILL Suppress go ahead */
-    qio_channel_write(ioc, buf, 3, NULL);
-    IACSET(buf, 0xff, 0xfb, 0x00);  /* IAC WILL Binary */
-    qio_channel_write(ioc, buf, 3, NULL);
-    IACSET(buf, 0xff, 0xfd, 0x00);  /* IAC DO Binary */
-    qio_channel_write(ioc, buf, 3, NULL);
+    TCPCharDriverTelnetInit *init = user_data;
+    ssize_t ret;
+
+    ret = qio_channel_write(ioc, init->buf, init->buflen, NULL);
+    if (ret < 0) {
+        if (ret == QIO_CHANNEL_ERR_BLOCK) {
+            ret = 0;
+        } else {
+            tcp_chr_disconnect(init->chr);
+            return FALSE;
+        }
+    }
+    init->buflen -= ret;
+
+    if (init->buflen == 0) {
+        tcp_chr_connect(init->chr);
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
+static void tcp_chr_telnet_init(CharDriverState *chr)
+{
+    TCPCharDriver *s = chr->opaque;
+    TCPCharDriverTelnetInit *init =
+        g_new0(TCPCharDriverTelnetInit, 1);
+    size_t n = 0;
+
+    init->chr = chr;
+    init->buflen = 12;
+
+#define IACSET(x, a, b, c)                      \
+    do {                                        \
+        x[n++] = a;                             \
+        x[n++] = b;                             \
+        x[n++] = c;                             \
+    } while (0)
+
+    /* Prep the telnet negotion to put telnet in binary,
+     * no echo, single char mode */
+    IACSET(init->buf, 0xff, 0xfb, 0x01);  /* IAC WILL ECHO */
+    IACSET(init->buf, 0xff, 0xfb, 0x03);  /* IAC WILL Suppress go ahead */
+    IACSET(init->buf, 0xff, 0xfb, 0x00);  /* IAC WILL Binary */
+    IACSET(init->buf, 0xff, 0xfd, 0x00);  /* IAC DO Binary */
+
+#undef IACSET
+
+    qio_channel_add_watch(
+        s->ioc, G_IO_OUT,
+        tcp_chr_telnet_init_io,
+        init);
 }
 
 static int tcp_chr_new_client(CharDriverState *chr, QIOChannel *ioc)
@@ -2838,7 +2887,12 @@ static int tcp_chr_new_client(CharDriverState *chr, QIOChannel *ioc)
         g_source_remove(s->listen_tag);
         s->listen_tag = 0;
     }
-    tcp_chr_connect(chr);
+
+    if (s->do_telnetopt) {
+        tcp_chr_telnet_init(chr);
+    } else {
+        tcp_chr_connect(chr);
+    }
 
     return 0;
 }
@@ -2863,7 +2917,6 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
                                void *opaque)
 {
     CharDriverState *chr = opaque;
-    TCPCharDriver *s = chr->opaque;
     QIOChannel *ioc;
 
     ioc = QIO_CHANNEL(
@@ -2872,10 +2925,6 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
         return TRUE;
     }
 
-    if (s->do_telnetopt) {
-        tcp_chr_telnet_init(ioc);
-    }
-
     tcp_chr_new_client(chr, ioc);
 
     object_unref(OBJECT(ioc));
-- 
2.1.0