I haven''t tested this patch on Linux or BSD so I''d appreciate some testing (Dan? Christoph?). This makes xenconsoled in unstable work again on Solaris thanks john # HG changeset patch # User john.levon@sun.com # Date 1197495828 28800 # Node ID 3430f8e764191214e0761100e50caf69f167345f # Parent 3a972b4de3bc3ef90c641a6a036274edb7ca92dc 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 - a read of zero from the master means that a slave closed, and is not an error Signed-off-by: John Levon <john.levon@sun.com> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c --- a/tools/console/daemon/io.c +++ b/tools/console/daemon/io.c @@ -40,6 +40,9 @@ #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)) @@ -230,50 +233,64 @@ static int create_domain_log(struct doma #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) + struct termios *termp, struct winsize *winp) { - int mfd, sfd; + 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; + goto err; - /* This does not match openpty specification, - * but as long as this does not hurt, this is acceptable. - */ - mfd = sfd; + if ((slave = ptsname(mfd)) == NULL) + goto err; - if (termp != NULL && tcgetattr(sfd, termp) < 0) - goto err1; + 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 (name) - strlcpy(name, ptsname(mfd), sizeof(slave)); if (winp) ioctl(sfd, TIOCSWINSZ, winp); return 0; -err1: +err: + if (sfd != -1) + close(sfd); close(mfd); -err0: 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 - static int domain_create_tty(struct domain *dom) { - char slave[80]; + const char *slave; struct termios term; char *path; int master, slavefd; @@ -282,7 +299,7 @@ static int domain_create_tty(struct doma char *data; unsigned int len; - if (openpty(&master, &slavefd, slave, &term, NULL) < 0) { + if (openpty(&master, &slavefd, NULL, &term, NULL) < 0) { master = -1; err = errno; dolog(LOG_ERR, "Failed to create tty for domain-%d (errno = %i, %s)", @@ -290,14 +307,23 @@ static int domain_create_tty(struct doma return master; } - cfmakeraw(&term); - if (tcsetattr(master, TCSAFLUSH, &term) < 0) { + if ((slave = ptsname(master)) == 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; } + cfmakeraw(&term); + + if (tcsetattr(slavefd, TCSAFLUSH, &term) < 0) { + err = errno; + dolog(LOG_ERR, "Failed to set tty attribute for domain-%d (errno = %i, %s)", + dom->domid, err, strerror(err)); + goto out; + } + + close(slavefd); if (dom->use_consolepath) { success = asprintf(&path, "%s/limit", dom->conspath) !@@ -684,7 +710,9 @@ static void handle_tty_read(struct domai len = sizeof(msg); len = read(dom->tty_fd, msg, len); - if (len < 1) { + if (len == 0) { + /* slave did a close: that''s fine. */ + } else if (len < 0) { close(dom->tty_fd); dom->tty_fd = -1; diff --git a/tools/ioemu/vl.c b/tools/ioemu/vl.c --- a/tools/ioemu/vl.c +++ b/tools/ioemu/vl.c @@ -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-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel