WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

[Xen-changelog] [xen-unstable] Fix master/slave handling in xenconsoled

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] [xen-unstable] Fix master/slave handling in xenconsoled and qemu
From: Xen patchbot-unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>
Date: Wed, 19 Dec 2007 12:40:12 -0800
Delivery-date: Wed, 19 Dec 2007 12:41:15 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1198075545 0
# Node ID 28921e83000b9846662759a5ee07caf34f4853a6
# Parent  9b37cabe048542ccf8449e20a4b256159c20e169
Fix master/slave handling in xenconsoled and qemu

Fix a number of problems with the pty handling:

- make openpty() implementation work on Solaris
- set raw on the slave fd, not the master, as the master doesn't
  have a line discipline pushed on Solaris
- make sure we don't leak the slave fd returned from openpty()
- don't use the 'name' argument of openpty() as it's a security risk
- note behaviour of a zero read of the master on Solaris
- remove pointless tcget/setattr

Signed-off-by: John Levon <john.levon@xxxxxxx>
Signed-off-by: Samuel Thibault <samuel.thibault@xxxxxxxxxx>
---
 tools/console/daemon/io.c |  156 ++++++++++++++++++++++++----------------------
 tools/ioemu/vl.c          |   67 +++++++++++++++++++
 2 files changed, 149 insertions(+), 74 deletions(-)

diff -r 9b37cabe0485 -r 28921e83000b tools/console/daemon/io.c
--- a/tools/console/daemon/io.c Wed Dec 19 14:45:04 2007 +0000
+++ b/tools/console/daemon/io.c Wed Dec 19 14:45:45 2007 +0000
@@ -36,10 +36,14 @@
 #include <stdarg.h>
 #include <sys/mman.h>
 #include <sys/time.h>
+#include <assert.h>
 #if defined(__NetBSD__) || defined(__OpenBSD__)
 #include <util.h>
 #elif defined(__linux__) || defined(__Linux__)
 #include <pty.h>
+#endif
+#if defined(__sun__)
+#include <stropts.h>
 #endif
 
 #define MAX(a, b) (((a) > (b)) ? (a) : (b))
@@ -75,7 +79,8 @@ struct domain
 struct domain
 {
        int domid;
-       int tty_fd;
+       int master_fd;
+       int slave_fd;
        int log_fd;
        bool is_dead;
        struct buffer buffer;
@@ -227,77 +232,90 @@ static int create_domain_log(struct doma
        return fd;
 }
 
+static void domain_close_tty(struct domain *dom)
+{
+       if (dom->master_fd != -1) {
+               close(dom->master_fd);
+               dom->master_fd = -1;
+       }
+
+       if (dom->slave_fd != -1) {
+               close(dom->slave_fd);
+               dom->slave_fd = -1;
+       }
+}
+
 #ifdef __sun__
 /* Once Solaris has openpty(), this is going to be removed. */
-int openpty(int *amaster, int *aslave, char *name,
-           struct termios *termp, struct winsize *winp)
-{
-       int mfd, sfd;
+static int openpty(int *amaster, int *aslave, char *name,
+                   struct termios *termp, struct winsize *winp)
+{
+       const char *slave;
+       int mfd = -1, sfd = -1;
 
        *amaster = *aslave = -1;
-       mfd = sfd = -1;
 
        mfd = open("/dev/ptmx", O_RDWR | O_NOCTTY);
        if (mfd < 0)
-               goto err0;
+               goto err;
 
        if (grantpt(mfd) == -1 || unlockpt(mfd) == -1)
-               goto err1;
-
-       /* This does not match openpty specification,
-        * but as long as this does not hurt, this is acceptable.
-        */
-       mfd = sfd;
-
-       if (termp != NULL && tcgetattr(sfd, termp) < 0)
-               goto err1;      
+               goto err;
+
+       if ((slave = ptsname(mfd)) == NULL)
+               goto err;
+
+       if ((sfd = open(slave, O_RDONLY | O_NOCTTY)) == -1)
+               goto err;
+
+       if (ioctl(sfd, I_PUSH, "ptem") == -1)
+               goto err;
 
        if (amaster)
                *amaster = mfd;
        if (aslave)
                *aslave = sfd;
-       if (name)
-               strlcpy(name, ptsname(mfd), sizeof(slave));
        if (winp)
                ioctl(sfd, TIOCSWINSZ, winp);
 
+       assert(name == NULL);
+       assert(termp == NULL);
+
        return 0;
 
-err1:
+err:
+       if (sfd != -1)
+               close(sfd);
        close(mfd);
-err0:
        return -1;
 }
 #endif
 
-
 static int domain_create_tty(struct domain *dom)
 {
-       char slave[80];
-       struct termios term;
+       const char *slave;
        char *path;
-       int master, slavefd;
        int err;
        bool success;
        char *data;
        unsigned int len;
 
-       if (openpty(&master, &slavefd, slave, &term, NULL) < 0) {
-               master = -1;
+       assert(dom->slave_fd == -1);
+       assert(dom->master_fd == -1);
+
+       if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) {
                err = errno;
                dolog(LOG_ERR, "Failed to create tty for domain-%d (errno = %i, 
%s)",
                      dom->domid, err, strerror(err));
-               return master;
-       }
-
-       cfmakeraw(&term);
-       if (tcsetattr(master, TCSAFLUSH, &term) < 0) {
+               return 0;
+       }
+
+       if ((slave = ptsname(dom->master_fd)) == NULL) {
                err = errno;
-               dolog(LOG_ERR, "Failed to set tty attribute  for domain-%d 
(errno = %i, %s)",
+               dolog(LOG_ERR, "Failed to get slave name for domain-%d (errno = 
%i, %s)",
                      dom->domid, err, strerror(err));
                goto out;
        }
-
 
        if (dom->use_consolepath) {
                success = asprintf(&path, "%s/limit", dom->conspath) !=
@@ -340,15 +358,15 @@ static int domain_create_tty(struct doma
                        goto out;
        }
 
-       if (fcntl(master, F_SETFL, O_NONBLOCK) == -1)
-               goto out;
-
-       return master;
- out:
-       close(master);
-       return -1;
-}
-
+       if (fcntl(dom->master_fd, F_SETFL, O_NONBLOCK) == -1)
+               goto out;
+
+       return 1;
+out:
+       domain_close_tty(dom);
+       return 0;
+}
+ 
 /* Takes tuples of names, scanf-style args, and void **, NULL terminated. */
 int xs_gather(struct xs_handle *xs, const char *dir, ...)
 {
@@ -454,10 +472,8 @@ static int domain_create_ring(struct dom
        dom->local_port = rc;
        dom->remote_port = remote_port;
 
-       if (dom->tty_fd == -1) {
-               dom->tty_fd = domain_create_tty(dom);
-
-               if (dom->tty_fd == -1) {
+       if (dom->master_fd == -1) {
+               if (!domain_create_tty(dom)) {
                        err = errno;
                        xc_evtchn_close(dom->xce_handle);
                        dom->xce_handle = -1;
@@ -535,7 +551,8 @@ static struct domain *create_domain(int 
        dom->conspath = s;
        strcat(dom->conspath, "/console");
 
-       dom->tty_fd = -1;
+       dom->master_fd = -1;
+       dom->slave_fd = -1;
        dom->log_fd = -1;
 
        dom->is_dead = false;
@@ -597,14 +614,7 @@ static void remove_domain(struct domain 
 
 static void cleanup_domain(struct domain *d)
 {
-       if (d->tty_fd != -1) {
-               close(d->tty_fd);
-               d->tty_fd = -1;
-       }
-       if (d->log_fd != -1) {
-               close(d->log_fd);
-               d->log_fd = -1;
-       }
+       domain_close_tty(d);
 
        free(d->buffer.data);
        d->buffer.data = NULL;
@@ -683,13 +693,17 @@ static void handle_tty_read(struct domai
        if (len > sizeof(msg))
                len = sizeof(msg);
 
-       len = read(dom->tty_fd, msg, len);
-       if (len < 1) {
-               close(dom->tty_fd);
-               dom->tty_fd = -1;
+       len = read(dom->master_fd, msg, len);
+       /*
+        * Note: on Solaris, len == 0 means the slave closed, and this
+        * is no problem, but Linux can't handle this usefully, so we
+        * keep the slave open for the duration.
+        */
+       if (len < 0) {
+               domain_close_tty(dom);
 
                if (domain_is_valid(dom->domid)) {
-                       dom->tty_fd = domain_create_tty(dom);
+                       domain_create_tty(dom);
                } else {
                        shutdown_domain(dom);
                }
@@ -703,8 +717,7 @@ static void handle_tty_read(struct domai
                intf->in_prod = prod;
                xc_evtchn_notify(dom->xce_handle, dom->local_port);
        } else {
-               close(dom->tty_fd);
-               dom->tty_fd = -1;
+               domain_close_tty(dom);
                shutdown_domain(dom);
        }
 }
@@ -716,17 +729,16 @@ static void handle_tty_write(struct doma
        if (dom->is_dead)
                return;
 
-       len = write(dom->tty_fd, dom->buffer.data + dom->buffer.consumed,
+       len = write(dom->master_fd, dom->buffer.data + dom->buffer.consumed,
                    dom->buffer.size - dom->buffer.consumed);
        if (len < 1) {
                dolog(LOG_DEBUG, "Write failed on domain %d: %zd, %d\n",
                      dom->domid, len, errno);
 
-               close(dom->tty_fd);
-               dom->tty_fd = -1;
+               domain_close_tty(dom);
 
                if (domain_is_valid(dom->domid)) {
-                       dom->tty_fd = domain_create_tty(dom);
+                       domain_create_tty(dom);
                } else {
                        shutdown_domain(dom);
                }
@@ -895,13 +907,13 @@ void handle_io(void)
                                max_fd = MAX(evtchn_fd, max_fd);
                        }
 
-                       if (d->tty_fd != -1) {
+                       if (d->master_fd != -1) {
                                if (!d->is_dead && ring_free_bytes(d))
-                                       FD_SET(d->tty_fd, &readfds);
+                                       FD_SET(d->master_fd, &readfds);
 
                                if (!buffer_empty(&d->buffer))
-                                       FD_SET(d->tty_fd, &writefds);
-                               max_fd = MAX(d->tty_fd, max_fd);
+                                       FD_SET(d->master_fd, &writefds);
+                               max_fd = MAX(d->master_fd, max_fd);
                        }
                }
 
@@ -951,10 +963,10 @@ void handle_io(void)
                                        handle_ring_read(d);
                        }
 
-                       if (d->tty_fd != -1 && FD_ISSET(d->tty_fd, &readfds))
+                       if (d->master_fd != -1 && FD_ISSET(d->master_fd, 
&readfds))
                                handle_tty_read(d);
 
-                       if (d->tty_fd != -1 && FD_ISSET(d->tty_fd, &writefds))
+                       if (d->master_fd != -1 && FD_ISSET(d->master_fd, 
&writefds))
                                handle_tty_write(d);
 
                        if (d->is_dead)
diff -r 9b37cabe0485 -r 28921e83000b tools/ioemu/vl.c
--- a/tools/ioemu/vl.c  Wed Dec 19 14:45:04 2007 +0000
+++ b/tools/ioemu/vl.c  Wed Dec 19 14:45:45 2007 +0000
@@ -65,6 +65,9 @@
 #include <linux/rtc.h>
 #include <linux/ppdev.h>
 #endif
+#endif
+#if defined(__sun__)
+#include <stropts.h>
 #endif
 #endif
 
@@ -1801,7 +1804,65 @@ static int store_dev_info(char *devName,
     return 0;
 }
 
-#if defined(__linux__) || defined(__NetBSD__) || defined(__OpenBSD__)
+#ifdef __sun__
+/* Once Solaris has openpty(), this is going to be removed. */
+int openpty(int *amaster, int *aslave, char *name,
+            struct termios *termp, struct winsize *winp)
+{
+       const char *slave;
+       int mfd = -1, sfd = -1;
+
+       *amaster = *aslave = -1;
+
+       mfd = open("/dev/ptmx", O_RDWR | O_NOCTTY);
+       if (mfd < 0)
+               goto err;
+
+       if (grantpt(mfd) == -1 || unlockpt(mfd) == -1)
+               goto err;
+
+       if ((slave = ptsname(mfd)) == NULL)
+               goto err;
+
+       if ((sfd = open(slave, O_RDONLY | O_NOCTTY)) == -1)
+               goto err;
+
+       if (ioctl(sfd, I_PUSH, "ptem") == -1 ||
+           (termp != NULL && tcgetattr(sfd, termp) < 0))
+               goto err;
+
+       if (amaster)
+               *amaster = mfd;
+       if (aslave)
+               *aslave = sfd;
+       if (winp)
+               ioctl(sfd, TIOCSWINSZ, winp);
+
+       return 0;
+
+err:
+       if (sfd != -1)
+               close(sfd);
+       close(mfd);
+       return -1;
+}
+
+void cfmakeraw (struct termios *termios_p)
+{
+       termios_p->c_iflag &=
+               ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
+       termios_p->c_oflag &= ~OPOST;
+       termios_p->c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
+       termios_p->c_cflag &= ~(CSIZE|PARENB);
+       termios_p->c_cflag |= CS8;
+
+       termios_p->c_cc[VMIN] = 0;
+       termios_p->c_cc[VTIME] = 0;
+}
+
+#endif
+
+#if defined(__linux__) || defined(__NetBSD__) || defined(__OpenBSD__) || 
defined(__sun__)
 static CharDriverState *qemu_chr_open_pty(void)
 {
     struct termios tty;
@@ -1816,6 +1877,8 @@ static CharDriverState *qemu_chr_open_pt
     cfmakeraw(&tty);
     tcsetattr(slave_fd, TCSAFLUSH, &tty);
     
+    close(slave_fd);
+
     fprintf(stderr, "char device redirected to %s\n", ptsname(master_fd));
 
     return qemu_chr_open_fd(master_fd, master_fd);
@@ -2038,7 +2101,7 @@ static CharDriverState *qemu_chr_open_pt
 {
     return NULL;
 }
-#endif /* __linux__ || __NetBSD__ || __OpenBSD__ */
+#endif /* __linux__ || __NetBSD__ || __OpenBSD__ || __sun__ */
 
 #endif /* !defined(_WIN32) */
 

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] [xen-unstable] Fix master/slave handling in xenconsoled and qemu, Xen patchbot-unstable <=