From owner-svn-src-head@freebsd.org Mon Apr 22 16:27:50 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6D730159E89E; Mon, 22 Apr 2019 16:27:50 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id B49F26BF50; Mon, 22 Apr 2019 16:27:49 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (localhost [127.0.0.1]) by gndrsh.dnsmgr.net (8.13.3/8.13.3) with ESMTP id x3MGRlSg032857; Mon, 22 Apr 2019 09:27:47 -0700 (PDT) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: (from freebsd@localhost) by gndrsh.dnsmgr.net (8.13.3/8.13.3/Submit) id x3MGRlUi032856; Mon, 22 Apr 2019 09:27:47 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201904221627.x3MGRlUi032856@gndrsh.dnsmgr.net> Subject: Re: svn commit: r346550 - head/usr.sbin/bhyve In-Reply-To: <201904221357.x3MDvreI024911@repo.freebsd.org> To: Mark Johnston Date: Mon, 22 Apr 2019 09:27:47 -0700 (PDT) CC: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: B49F26BF50 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.94 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.94)[-0.940,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Apr 2019 16:27:50 -0000 > Author: markj > Date: Mon Apr 22 13:57:52 2019 > New Revision: 346550 > URL: https://svnweb.freebsd.org/changeset/base/346550 > > Log: > Use separate descriptors in bhyve's stdio uart backend. > > bhyve was previously using stdin for both reading and writing to the > console, which made it difficult to redirect console output. Use > stdin for reading and stdout for writing. This makes it easier to use > bhyve as a backend for syzkaller. > > As a side effect, the change fixes a minor bug which would cause bhyve > to fail with ENOTCAPABLE if configured to use nmdm for com1 and stdio > for com2. > > bhyveload already uses separate descriptors, as does the bvmcons driver. > > Reviewed by: jhb > MFC after: 1 month We are approaching the 11.3 release slush, unless there is some high risk associated with this change I would like to see it in the early 11.3 alpha/beta tests rather than have it come in late in the test cycle. Thanks, Rod > Sponsored by: The FreeBSD Foundation > Differential Revision: https://reviews.freebsd.org/D19788 > > Modified: > head/usr.sbin/bhyve/uart_emul.c > > Modified: head/usr.sbin/bhyve/uart_emul.c > ============================================================================== > --- head/usr.sbin/bhyve/uart_emul.c Mon Apr 22 13:55:06 2019 (r346549) > +++ head/usr.sbin/bhyve/uart_emul.c Mon Apr 22 13:57:52 2019 (r346550) > @@ -100,8 +100,8 @@ struct fifo { > > struct ttyfd { > bool opened; > - int fd; /* tty device file descriptor */ > - struct termios tio_orig, tio_new; /* I/O Terminals */ > + int rfd; /* fd for reading */ > + int wfd; /* fd for writing, may be == rfd */ > }; > > struct uart_softc { > @@ -141,16 +141,15 @@ ttyclose(void) > static void > ttyopen(struct ttyfd *tf) > { > + struct termios orig, new; > > - tcgetattr(tf->fd, &tf->tio_orig); > - > - tf->tio_new = tf->tio_orig; > - cfmakeraw(&tf->tio_new); > - tf->tio_new.c_cflag |= CLOCAL; > - tcsetattr(tf->fd, TCSANOW, &tf->tio_new); > - > - if (tf->fd == STDIN_FILENO) { > - tio_stdio_orig = tf->tio_orig; > + tcgetattr(tf->rfd, &orig); > + new = orig; > + cfmakeraw(&new); > + new.c_cflag |= CLOCAL; > + tcsetattr(tf->rfd, TCSANOW, &new); > + if (uart_stdio) { > + tio_stdio_orig = orig; > atexit(ttyclose); > } > } > @@ -160,7 +159,7 @@ ttyread(struct ttyfd *tf) > { > unsigned char rb; > > - if (read(tf->fd, &rb, 1) == 1) > + if (read(tf->rfd, &rb, 1) == 1) > return (rb); > else > return (-1); > @@ -170,7 +169,7 @@ static void > ttywrite(struct ttyfd *tf, unsigned char wb) > { > > - (void)write(tf->fd, &wb, 1); > + (void)write(tf->wfd, &wb, 1); > } > > static void > @@ -190,7 +189,7 @@ rxfifo_reset(struct uart_softc *sc, int size) > * Flush any unread input from the tty buffer. > */ > while (1) { > - nread = read(sc->tty.fd, flushbuf, sizeof(flushbuf)); > + nread = read(sc->tty.rfd, flushbuf, sizeof(flushbuf)); > if (nread != sizeof(flushbuf)) > break; > } > @@ -277,7 +276,7 @@ uart_opentty(struct uart_softc *sc) > { > > ttyopen(&sc->tty); > - sc->mev = mevent_add(sc->tty.fd, EVF_READ, uart_drain, sc); > + sc->mev = mevent_add(sc->tty.rfd, EVF_READ, uart_drain, sc); > assert(sc->mev != NULL); > } > > @@ -374,7 +373,7 @@ uart_drain(int fd, enum ev_type ev, void *arg) > > sc = arg; > > - assert(fd == sc->tty.fd); > + assert(fd == sc->tty.rfd); > assert(ev == EVF_READ); > > /* > @@ -637,68 +636,79 @@ uart_init(uart_intr_func_t intr_assert, uart_intr_func > } > > static int > -uart_tty_backend(struct uart_softc *sc, const char *opts) > +uart_stdio_backend(struct uart_softc *sc) > { > - int fd; > - int retval; > +#ifndef WITHOUT_CAPSICUM > + cap_rights_t rights; > + cap_ioctl_t cmds[] = { TIOCGETA, TIOCSETA, TIOCGWINSZ }; > +#endif > > - retval = -1; > + if (uart_stdio) > + return (-1); > > - fd = open(opts, O_RDWR | O_NONBLOCK); > - if (fd > 0 && isatty(fd)) { > - sc->tty.fd = fd; > - sc->tty.opened = true; > - retval = 0; > - } > + sc->tty.rfd = STDIN_FILENO; > + sc->tty.wfd = STDOUT_FILENO; > + sc->tty.opened = true; > > - return (retval); > + if (fcntl(sc->tty.rfd, F_SETFL, O_NONBLOCK) != 0) > + return (-1); > + if (fcntl(sc->tty.wfd, F_SETFL, O_NONBLOCK) != 0) > + return (-1); > + > +#ifndef WITHOUT_CAPSICUM > + cap_rights_init(&rights, CAP_EVENT, CAP_IOCTL, CAP_READ); > + if (caph_rights_limit(sc->tty.rfd, &rights) == -1) > + errx(EX_OSERR, "Unable to apply rights for sandbox"); > + if (caph_ioctls_limit(sc->tty.rfd, cmds, nitems(cmds)) == -1) > + errx(EX_OSERR, "Unable to apply rights for sandbox"); > +#endif > + > + uart_stdio = true; > + > + return (0); > } > > -int > -uart_set_backend(struct uart_softc *sc, const char *opts) > +static int > +uart_tty_backend(struct uart_softc *sc, const char *opts) > { > - int retval; > #ifndef WITHOUT_CAPSICUM > cap_rights_t rights; > cap_ioctl_t cmds[] = { TIOCGETA, TIOCSETA, TIOCGWINSZ }; > #endif > + int fd; > > - retval = -1; > + fd = open(opts, O_RDWR | O_NONBLOCK); > + if (fd < 0 || !isatty(fd)) > + return (-1); > > + sc->tty.rfd = sc->tty.wfd = fd; > + sc->tty.opened = true; > + > +#ifndef WITHOUT_CAPSICUM > + cap_rights_init(&rights, CAP_EVENT, CAP_IOCTL, CAP_READ, CAP_WRITE); > + if (caph_rights_limit(fd, &rights) == -1) > + errx(EX_OSERR, "Unable to apply rights for sandbox"); > + if (caph_ioctls_limit(fd, cmds, nitems(cmds)) == -1) > + errx(EX_OSERR, "Unable to apply rights for sandbox"); > +#endif > + > + return (0); > +} > + > +int > +uart_set_backend(struct uart_softc *sc, const char *opts) > +{ > + int retval; > + > if (opts == NULL) > return (0); > > - if (strcmp("stdio", opts) == 0) { > - if (!uart_stdio) { > - sc->tty.fd = STDIN_FILENO; > - sc->tty.opened = true; > - uart_stdio = true; > - retval = 0; > - } > - } else if (uart_tty_backend(sc, opts) == 0) { > - retval = 0; > - } > - > - /* Make the backend file descriptor non-blocking */ > + if (strcmp("stdio", opts) == 0) > + retval = uart_stdio_backend(sc); > + else > + retval = uart_tty_backend(sc, opts); > if (retval == 0) > - retval = fcntl(sc->tty.fd, F_SETFL, O_NONBLOCK); > - > - if (retval == 0) { > -#ifndef WITHOUT_CAPSICUM > - cap_rights_init(&rights, CAP_EVENT, CAP_IOCTL, CAP_READ, > - CAP_WRITE); > - if (caph_rights_limit(sc->tty.fd, &rights) == -1) > - errx(EX_OSERR, "Unable to apply rights for sandbox"); > - if (caph_ioctls_limit(sc->tty.fd, cmds, nitems(cmds)) == -1) > - errx(EX_OSERR, "Unable to apply rights for sandbox"); > - if (!uart_stdio) { > - if (caph_limit_stdin() == -1) > - errx(EX_OSERR, > - "Unable to apply rights for sandbox"); > - } > -#endif > uart_opentty(sc); > - } > > return (retval); > } > > -- Rod Grimes rgrimes@freebsd.org