All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: Juan Quintela <quintela@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Amit Shah <amit.shah@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [Qemu-devel] [PATCH FYI 21/46] char: don't assume telnet initialization will not block
Date: Thu,  3 Sep 2015 16:39:03 +0100	[thread overview]
Message-ID: <1441294768-8712-22-git-send-email-berrange@redhat.com> (raw)
In-Reply-To: <1441294768-8712-1-git-send-email-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 f2724b9..fe75f09 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2769,19 +2769,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, NULL);
 }
 
 static int tcp_chr_new_client(CharDriverState *chr, QIOChannelSocket *sioc)
@@ -2802,7 +2851,12 @@ static int tcp_chr_new_client(CharDriverState *chr, QIOChannelSocket *sioc)
         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;
 }
@@ -2827,7 +2881,6 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
                                void *opaque)
 {
     CharDriverState *chr = opaque;
-    TCPCharDriver *s = chr->opaque;
     QIOChannelSocket *sioc;
 
     sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(channel),
@@ -2836,10 +2889,6 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
         return TRUE;
     }
 
-    if (s->do_telnetopt) {
-        tcp_chr_telnet_init(QIO_CHANNEL(sioc));
-    }
-
     tcp_chr_new_client(chr, sioc);
 
     object_unref(OBJECT(sioc));
-- 
2.4.3

  parent reply	other threads:[~2015-09-03 15:58 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-03 15:38 [Qemu-devel] [PATCH FYI 00/46] Generic TLS support across VNC/chardev/migration Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 01/46] sockets: add helpers for creating SocketAddress from a socket Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 02/46] sockets: move qapi_copy_SocketAddress into qemu-sockets.c Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 03/46] sockets: allow port to be NULL when listening on IP address Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 04/46] osdep: add qemu_fork() wrapper for safely handling signals Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 05/46] coroutine: move into libqemuutil.a library Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 06/46] io: add abstract QIOChannel classes Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 07/46] io: add helper module for creating watches on FDs Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 08/46] io: pull Buffer code out of VNC module Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 09/46] io: add QIOTask class for async operations Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 10/46] io: add QIOChannelSocket class Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 11/46] io: add QIOChannelFile class Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 12/46] io: add QIOChannelTLS class Daniel P. Berrange
2015-09-07 15:31   ` Dr. David Alan Gilbert
2015-09-07 15:41     ` Daniel P. Berrange
2015-09-07 15:51       ` Dr. David Alan Gilbert
2015-09-07 16:04         ` Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 13/46] io: add QIOChannelWebsock class Daniel P. Berrange
2015-09-07 15:44   ` Dr. David Alan Gilbert
2015-09-07 15:50     ` Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 14/46] io: add QIOChannelCommand class Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 15/46] ui: convert VNC startup code to use SocketAddress Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 16/46] ui: convert VNC server to use QIOChannelSocket Daniel P. Berrange
2015-09-03 15:38 ` [Qemu-devel] [PATCH FYI 17/46] ui: convert VNC server to use QIOChannelTLS Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 18/46] ui: convert VNC server to use QIOChannelWebsock Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 19/46] char: remove fixed length filename allocation Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 20/46] char: convert from GIOChannel to QIOChannel Daniel P. Berrange
2015-09-03 15:39 ` Daniel P. Berrange [this message]
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 22/46] char: introduce support for TLS encrypted TCP chardev backend Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 23/46] nbd: convert to use the QAPI SocketAddress object Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 24/46] qemu-nbd: " Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 25/46] sockets: remove use of QemuOpts from header file Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 26/46] sockets: remove use of QemuOpts from socket_listen Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 27/46] sockets: remove use of QemuOpts from socket_connect Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 28/46] sockets: remove use of QemuOpts from socket_dgram Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 29/46] migration: remove use of qemu_bufopen from vmstate tests Daniel P. Berrange
2015-09-07 16:08   ` Dr. David Alan Gilbert
2015-09-07 16:17     ` Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 30/46] migration: remove memory buffer based QEMUFile backend Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 31/46] migration: move definition of struct QEMUFile back into qemu-file.c Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 32/46] migration: split migration hooks out of QEMUFileOps Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 33/46] migration: ensure qemu_fflush() always writes full data amount Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 34/46] migration: introduce qemu_fset_blocking function on QEMUFile Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 35/46] migration: force QEMUFile to blocking mode for outgoing migration Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 36/46] migration: introduce a new QEMUFile impl based on QIOChannel Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 37/46] migration: convert unix socket protocol to use QIOChannel Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 38/46] migration: convert tcp " Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 39/46] migration: convert fd " Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 40/46] migration: convert exec " Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 41/46] migration: convert RDMA to use QIOChannel interface Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 42/46] migration: convert savevm to use QIOChannel for writing to files Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 43/46] migration: delete QEMUFile sockets implementation Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 44/46] migration: delete QEMUFile stdio implementation Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 45/46] migration: support TLS encryption with TCP migration backend Daniel P. Berrange
2015-09-07 16:23   ` Dr. David Alan Gilbert
2015-09-07 16:29     ` Daniel P. Berrange
2015-09-03 15:39 ` [Qemu-devel] [PATCH FYI 46/46] migration: remove support for non-iovec based write handlers Daniel P. Berrange

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=1441294768-8712-22-git-send-email-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.