ia64/xen-unstable

changeset 16638:28921e83000b

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@sun.com>
Signed-off-by: Samuel Thibault <samuel.thibault@citrix.com>
author Keir Fraser <keir.fraser@citrix.com>
date Wed Dec 19 14:45:45 2007 +0000 (2007-12-19)
parents 9b37cabe0485
children ca461349620a
files tools/console/daemon/io.c tools/ioemu/vl.c
line diff
     1.1 --- a/tools/console/daemon/io.c	Wed Dec 19 14:45:04 2007 +0000
     1.2 +++ b/tools/console/daemon/io.c	Wed Dec 19 14:45:45 2007 +0000
     1.3 @@ -36,11 +36,15 @@
     1.4  #include <stdarg.h>
     1.5  #include <sys/mman.h>
     1.6  #include <sys/time.h>
     1.7 +#include <assert.h>
     1.8  #if defined(__NetBSD__) || defined(__OpenBSD__)
     1.9  #include <util.h>
    1.10  #elif defined(__linux__) || defined(__Linux__)
    1.11  #include <pty.h>
    1.12  #endif
    1.13 +#if defined(__sun__)
    1.14 +#include <stropts.h>
    1.15 +#endif
    1.16  
    1.17  #define MAX(a, b) (((a) > (b)) ? (a) : (b))
    1.18  #define MIN(a, b) (((a) < (b)) ? (a) : (b))
    1.19 @@ -75,7 +79,8 @@ struct buffer
    1.20  struct domain
    1.21  {
    1.22  	int domid;
    1.23 -	int tty_fd;
    1.24 +	int master_fd;
    1.25 +	int slave_fd;
    1.26  	int log_fd;
    1.27  	bool is_dead;
    1.28  	struct buffer buffer;
    1.29 @@ -227,78 +232,91 @@ static int create_domain_log(struct doma
    1.30  	return fd;
    1.31  }
    1.32  
    1.33 +static void domain_close_tty(struct domain *dom)
    1.34 +{
    1.35 +	if (dom->master_fd != -1) {
    1.36 +		close(dom->master_fd);
    1.37 +		dom->master_fd = -1;
    1.38 +	}
    1.39 +
    1.40 +	if (dom->slave_fd != -1) {
    1.41 +		close(dom->slave_fd);
    1.42 +		dom->slave_fd = -1;
    1.43 +	}
    1.44 +}
    1.45 +
    1.46  #ifdef __sun__
    1.47  /* Once Solaris has openpty(), this is going to be removed. */
    1.48 -int openpty(int *amaster, int *aslave, char *name,
    1.49 -	    struct termios *termp, struct winsize *winp)
    1.50 +static int openpty(int *amaster, int *aslave, char *name,
    1.51 +                   struct termios *termp, struct winsize *winp)
    1.52  {
    1.53 -	int mfd, sfd;
    1.54 +	const char *slave;
    1.55 +	int mfd = -1, sfd = -1;
    1.56  
    1.57  	*amaster = *aslave = -1;
    1.58 -	mfd = sfd = -1;
    1.59  
    1.60  	mfd = open("/dev/ptmx", O_RDWR | O_NOCTTY);
    1.61  	if (mfd < 0)
    1.62 -		goto err0;
    1.63 +		goto err;
    1.64  
    1.65  	if (grantpt(mfd) == -1 || unlockpt(mfd) == -1)
    1.66 -		goto err1;
    1.67 +		goto err;
    1.68  
    1.69 -	/* This does not match openpty specification,
    1.70 -	 * but as long as this does not hurt, this is acceptable.
    1.71 -	 */
    1.72 -	mfd = sfd;
    1.73 +	if ((slave = ptsname(mfd)) == NULL)
    1.74 +		goto err;
    1.75  
    1.76 -	if (termp != NULL && tcgetattr(sfd, termp) < 0)
    1.77 -		goto err1;	
    1.78 +	if ((sfd = open(slave, O_RDONLY | O_NOCTTY)) == -1)
    1.79 +		goto err;
    1.80 +
    1.81 +	if (ioctl(sfd, I_PUSH, "ptem") == -1)
    1.82 +		goto err;
    1.83  
    1.84  	if (amaster)
    1.85  		*amaster = mfd;
    1.86  	if (aslave)
    1.87  		*aslave = sfd;
    1.88 -	if (name)
    1.89 -		strlcpy(name, ptsname(mfd), sizeof(slave));
    1.90  	if (winp)
    1.91  		ioctl(sfd, TIOCSWINSZ, winp);
    1.92  
    1.93 +	assert(name == NULL);
    1.94 +	assert(termp == NULL);
    1.95 +
    1.96  	return 0;
    1.97  
    1.98 -err1:
    1.99 +err:
   1.100 +	if (sfd != -1)
   1.101 +		close(sfd);
   1.102  	close(mfd);
   1.103 -err0:
   1.104  	return -1;
   1.105  }
   1.106  #endif
   1.107  
   1.108 -
   1.109  static int domain_create_tty(struct domain *dom)
   1.110  {
   1.111 -	char slave[80];
   1.112 -	struct termios term;
   1.113 +	const char *slave;
   1.114  	char *path;
   1.115 -	int master, slavefd;
   1.116  	int err;
   1.117  	bool success;
   1.118  	char *data;
   1.119  	unsigned int len;
   1.120  
   1.121 -	if (openpty(&master, &slavefd, slave, &term, NULL) < 0) {
   1.122 -		master = -1;
   1.123 +	assert(dom->slave_fd == -1);
   1.124 +	assert(dom->master_fd == -1);
   1.125 +
   1.126 +	if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) {
   1.127  		err = errno;
   1.128  		dolog(LOG_ERR, "Failed to create tty for domain-%d (errno = %i, %s)",
   1.129  		      dom->domid, err, strerror(err));
   1.130 -		return master;
   1.131 +		return 0;
   1.132  	}
   1.133  
   1.134 -	cfmakeraw(&term);
   1.135 -	if (tcsetattr(master, TCSAFLUSH, &term) < 0) {
   1.136 +	if ((slave = ptsname(dom->master_fd)) == NULL) {
   1.137  		err = errno;
   1.138 -		dolog(LOG_ERR, "Failed to set tty attribute  for domain-%d (errno = %i, %s)",
   1.139 +		dolog(LOG_ERR, "Failed to get slave name for domain-%d (errno = %i, %s)",
   1.140  		      dom->domid, err, strerror(err));
   1.141  		goto out;
   1.142  	}
   1.143  
   1.144 -
   1.145  	if (dom->use_consolepath) {
   1.146  		success = asprintf(&path, "%s/limit", dom->conspath) !=
   1.147  			-1;
   1.148 @@ -340,15 +358,15 @@ static int domain_create_tty(struct doma
   1.149  			goto out;
   1.150  	}
   1.151  
   1.152 -	if (fcntl(master, F_SETFL, O_NONBLOCK) == -1)
   1.153 +	if (fcntl(dom->master_fd, F_SETFL, O_NONBLOCK) == -1)
   1.154  		goto out;
   1.155  
   1.156 -	return master;
   1.157 - out:
   1.158 -	close(master);
   1.159 -	return -1;
   1.160 +	return 1;
   1.161 +out:
   1.162 +	domain_close_tty(dom);
   1.163 +	return 0;
   1.164  }
   1.165 -
   1.166 + 
   1.167  /* Takes tuples of names, scanf-style args, and void **, NULL terminated. */
   1.168  int xs_gather(struct xs_handle *xs, const char *dir, ...)
   1.169  {
   1.170 @@ -454,10 +472,8 @@ static int domain_create_ring(struct dom
   1.171  	dom->local_port = rc;
   1.172  	dom->remote_port = remote_port;
   1.173  
   1.174 -	if (dom->tty_fd == -1) {
   1.175 -		dom->tty_fd = domain_create_tty(dom);
   1.176 -
   1.177 -		if (dom->tty_fd == -1) {
   1.178 +	if (dom->master_fd == -1) {
   1.179 +		if (!domain_create_tty(dom)) {
   1.180  			err = errno;
   1.181  			xc_evtchn_close(dom->xce_handle);
   1.182  			dom->xce_handle = -1;
   1.183 @@ -535,7 +551,8 @@ static struct domain *create_domain(int 
   1.184  	dom->conspath = s;
   1.185  	strcat(dom->conspath, "/console");
   1.186  
   1.187 -	dom->tty_fd = -1;
   1.188 +	dom->master_fd = -1;
   1.189 +	dom->slave_fd = -1;
   1.190  	dom->log_fd = -1;
   1.191  
   1.192  	dom->is_dead = false;
   1.193 @@ -597,14 +614,7 @@ static void remove_domain(struct domain 
   1.194  
   1.195  static void cleanup_domain(struct domain *d)
   1.196  {
   1.197 -	if (d->tty_fd != -1) {
   1.198 -		close(d->tty_fd);
   1.199 -		d->tty_fd = -1;
   1.200 -	}
   1.201 -	if (d->log_fd != -1) {
   1.202 -		close(d->log_fd);
   1.203 -		d->log_fd = -1;
   1.204 -	}
   1.205 +	domain_close_tty(d);
   1.206  
   1.207  	free(d->buffer.data);
   1.208  	d->buffer.data = NULL;
   1.209 @@ -683,13 +693,17 @@ static void handle_tty_read(struct domai
   1.210  	if (len > sizeof(msg))
   1.211  		len = sizeof(msg);
   1.212  
   1.213 -	len = read(dom->tty_fd, msg, len);
   1.214 -	if (len < 1) {
   1.215 -		close(dom->tty_fd);
   1.216 -		dom->tty_fd = -1;
   1.217 +	len = read(dom->master_fd, msg, len);
   1.218 +	/*
   1.219 +	 * Note: on Solaris, len == 0 means the slave closed, and this
   1.220 +	 * is no problem, but Linux can't handle this usefully, so we
   1.221 +	 * keep the slave open for the duration.
   1.222 +	 */
   1.223 +	if (len < 0) {
   1.224 +		domain_close_tty(dom);
   1.225  
   1.226  		if (domain_is_valid(dom->domid)) {
   1.227 -			dom->tty_fd = domain_create_tty(dom);
   1.228 +			domain_create_tty(dom);
   1.229  		} else {
   1.230  			shutdown_domain(dom);
   1.231  		}
   1.232 @@ -703,8 +717,7 @@ static void handle_tty_read(struct domai
   1.233  		intf->in_prod = prod;
   1.234  		xc_evtchn_notify(dom->xce_handle, dom->local_port);
   1.235  	} else {
   1.236 -		close(dom->tty_fd);
   1.237 -		dom->tty_fd = -1;
   1.238 +		domain_close_tty(dom);
   1.239  		shutdown_domain(dom);
   1.240  	}
   1.241  }
   1.242 @@ -716,17 +729,16 @@ static void handle_tty_write(struct doma
   1.243  	if (dom->is_dead)
   1.244  		return;
   1.245  
   1.246 -	len = write(dom->tty_fd, dom->buffer.data + dom->buffer.consumed,
   1.247 +	len = write(dom->master_fd, dom->buffer.data + dom->buffer.consumed,
   1.248  		    dom->buffer.size - dom->buffer.consumed);
   1.249   	if (len < 1) {
   1.250  		dolog(LOG_DEBUG, "Write failed on domain %d: %zd, %d\n",
   1.251  		      dom->domid, len, errno);
   1.252  
   1.253 -		close(dom->tty_fd);
   1.254 -		dom->tty_fd = -1;
   1.255 +		domain_close_tty(dom);
   1.256  
   1.257  		if (domain_is_valid(dom->domid)) {
   1.258 -			dom->tty_fd = domain_create_tty(dom);
   1.259 +			domain_create_tty(dom);
   1.260  		} else {
   1.261  			shutdown_domain(dom);
   1.262  		}
   1.263 @@ -895,13 +907,13 @@ void handle_io(void)
   1.264  				max_fd = MAX(evtchn_fd, max_fd);
   1.265  			}
   1.266  
   1.267 -			if (d->tty_fd != -1) {
   1.268 +			if (d->master_fd != -1) {
   1.269  				if (!d->is_dead && ring_free_bytes(d))
   1.270 -					FD_SET(d->tty_fd, &readfds);
   1.271 +					FD_SET(d->master_fd, &readfds);
   1.272  
   1.273  				if (!buffer_empty(&d->buffer))
   1.274 -					FD_SET(d->tty_fd, &writefds);
   1.275 -				max_fd = MAX(d->tty_fd, max_fd);
   1.276 +					FD_SET(d->master_fd, &writefds);
   1.277 +				max_fd = MAX(d->master_fd, max_fd);
   1.278  			}
   1.279  		}
   1.280  
   1.281 @@ -951,10 +963,10 @@ void handle_io(void)
   1.282  					handle_ring_read(d);
   1.283  			}
   1.284  
   1.285 -			if (d->tty_fd != -1 && FD_ISSET(d->tty_fd, &readfds))
   1.286 +			if (d->master_fd != -1 && FD_ISSET(d->master_fd, &readfds))
   1.287  				handle_tty_read(d);
   1.288  
   1.289 -			if (d->tty_fd != -1 && FD_ISSET(d->tty_fd, &writefds))
   1.290 +			if (d->master_fd != -1 && FD_ISSET(d->master_fd, &writefds))
   1.291  				handle_tty_write(d);
   1.292  
   1.293  			if (d->is_dead)
     2.1 --- a/tools/ioemu/vl.c	Wed Dec 19 14:45:04 2007 +0000
     2.2 +++ b/tools/ioemu/vl.c	Wed Dec 19 14:45:45 2007 +0000
     2.3 @@ -66,6 +66,9 @@
     2.4  #include <linux/ppdev.h>
     2.5  #endif
     2.6  #endif
     2.7 +#if defined(__sun__)
     2.8 +#include <stropts.h>
     2.9 +#endif
    2.10  #endif
    2.11  
    2.12  #if defined(CONFIG_SLIRP)
    2.13 @@ -1801,7 +1804,65 @@ static int store_dev_info(char *devName,
    2.14      return 0;
    2.15  }
    2.16  
    2.17 -#if defined(__linux__) || defined(__NetBSD__) || defined(__OpenBSD__)
    2.18 +#ifdef __sun__
    2.19 +/* Once Solaris has openpty(), this is going to be removed. */
    2.20 +int openpty(int *amaster, int *aslave, char *name,
    2.21 +            struct termios *termp, struct winsize *winp)
    2.22 +{
    2.23 +	const char *slave;
    2.24 +	int mfd = -1, sfd = -1;
    2.25 +
    2.26 +	*amaster = *aslave = -1;
    2.27 +
    2.28 +	mfd = open("/dev/ptmx", O_RDWR | O_NOCTTY);
    2.29 +	if (mfd < 0)
    2.30 +		goto err;
    2.31 +
    2.32 +	if (grantpt(mfd) == -1 || unlockpt(mfd) == -1)
    2.33 +		goto err;
    2.34 +
    2.35 +	if ((slave = ptsname(mfd)) == NULL)
    2.36 +		goto err;
    2.37 +
    2.38 +	if ((sfd = open(slave, O_RDONLY | O_NOCTTY)) == -1)
    2.39 +		goto err;
    2.40 +
    2.41 +	if (ioctl(sfd, I_PUSH, "ptem") == -1 ||
    2.42 +	    (termp != NULL && tcgetattr(sfd, termp) < 0))
    2.43 +		goto err;
    2.44 +
    2.45 +	if (amaster)
    2.46 +		*amaster = mfd;
    2.47 +	if (aslave)
    2.48 +		*aslave = sfd;
    2.49 +	if (winp)
    2.50 +		ioctl(sfd, TIOCSWINSZ, winp);
    2.51 +
    2.52 +	return 0;
    2.53 +
    2.54 +err:
    2.55 +	if (sfd != -1)
    2.56 +		close(sfd);
    2.57 +	close(mfd);
    2.58 +	return -1;
    2.59 +}
    2.60 +
    2.61 +void cfmakeraw (struct termios *termios_p)
    2.62 +{
    2.63 +	termios_p->c_iflag &=
    2.64 +		~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
    2.65 +	termios_p->c_oflag &= ~OPOST;
    2.66 +	termios_p->c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
    2.67 +	termios_p->c_cflag &= ~(CSIZE|PARENB);
    2.68 +	termios_p->c_cflag |= CS8;
    2.69 +
    2.70 +	termios_p->c_cc[VMIN] = 0;
    2.71 +	termios_p->c_cc[VTIME] = 0;
    2.72 +}
    2.73 +
    2.74 +#endif
    2.75 +
    2.76 +#if defined(__linux__) || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__sun__)
    2.77  static CharDriverState *qemu_chr_open_pty(void)
    2.78  {
    2.79      struct termios tty;
    2.80 @@ -1816,6 +1877,8 @@ static CharDriverState *qemu_chr_open_pt
    2.81      cfmakeraw(&tty);
    2.82      tcsetattr(slave_fd, TCSAFLUSH, &tty);
    2.83      
    2.84 +    close(slave_fd);
    2.85 +
    2.86      fprintf(stderr, "char device redirected to %s\n", ptsname(master_fd));
    2.87  
    2.88      return qemu_chr_open_fd(master_fd, master_fd);
    2.89 @@ -2038,7 +2101,7 @@ static CharDriverState *qemu_chr_open_pt
    2.90  {
    2.91      return NULL;
    2.92  }
    2.93 -#endif /* __linux__ || __NetBSD__ || __OpenBSD__ */
    2.94 +#endif /* __linux__ || __NetBSD__ || __OpenBSD__ || __sun__ */
    2.95  
    2.96  #endif /* !defined(_WIN32) */
    2.97