|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] Fix xenconsole's "Could not read tty from store"
On Tue, Dec 18, 2007 at 01:41:44AM +0000, John Levon wrote:
> On Mon, Dec 17, 2007 at 04:54:01PM +0000, Samuel Thibault wrote:
>
> > > If it helps I can send out an updated patch merging these fixes. Will
> > > you test it if so Samuel?
>
> See below. Thanks a lot.
Keir, Samuel has tested the below patch and verifies it works on Linux.
Please apply.
thanks
john
# HG changeset patch
# User john.levon@xxxxxxx
# Date 1197910843 28800
# Node ID 6490ff3cf622271793c7c7f48cb33508084e4d9e
# Parent 9011d85ef535ce57ccfb459f42e3483db4dad3da
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
- openpty() sets, not gets, terminal settings
Signed-off-by: John Levon <john.levon@xxxxxxx>
Signed-off-by: Samuel Thibault <samuel.thibault@xxxxxxxxxx>
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
@@ -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))
@@ -230,50 +234,66 @@ 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)
+ 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;
}
+
+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 +302,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, NULL, NULL) < 0) {
master = -1;
err = errno;
dolog(LOG_ERR, "Failed to create tty for domain-%d (errno = %i,
%s)",
@@ -290,14 +310,30 @@ 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;
}
+ if (tcgetattr(slavefd, &term) < 0) {
+ err = errno;
+ dolog(LOG_ERR, "Failed to get tty attribute 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 +720,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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |