svn commit: r327948 - head/sys/dev/virtio/console
Bryan Venteicher
bryanv at FreeBSD.org
Sat Jan 13 21:39:47 UTC 2018
Author: bryanv
Date: Sat Jan 13 21:39:46 2018
New Revision: 327948
URL: https://svnweb.freebsd.org/changeset/base/327948
Log:
Fix possible panic when creating VirtIO console dev aliases
Since we have no control over the name, the MAKEDEV_CHECKNAME flag must be
used to return an error on an invalid (to devfs) name instead of panicing.
r305900 that originally added this feature also introduced a few other bugs:
- Proper locking not performed
- Theoretically broke the expectation that the control event buffer would
not span more than one pages, but did not update the CTASSERT that was
in place to prevent this. However, since the struct virtio_console_control
and the bulk buffer together were quite small, this could not have happened.
Also workaround an QEMU VirtIO spec violation in that it includes the NUL
terminator in the buffer length when the spec says it is not included.
PR: 223531
MFC after: 1 week
Modified:
head/sys/dev/virtio/console/virtio_console.c
Modified: head/sys/dev/virtio/console/virtio_console.c
==============================================================================
--- head/sys/dev/virtio/console/virtio_console.c Sat Jan 13 21:37:14 2018 (r327947)
+++ head/sys/dev/virtio/console/virtio_console.c Sat Jan 13 21:39:46 2018 (r327948)
@@ -30,6 +30,7 @@
__FBSDID("$FreeBSD$");
#include <sys/param.h>
+#include <sys/ctype.h>
#include <sys/systm.h>
#include <sys/kernel.h>
#include <sys/malloc.h>
@@ -58,14 +59,19 @@ __FBSDID("$FreeBSD$");
#define VTCON_MAX_PORTS 32
#define VTCON_TTY_PREFIX "V"
+#define VTCON_TTY_ALIAS_PREFIX "vtcon"
#define VTCON_BULK_BUFSZ 128
+#define VTCON_CTRL_BUFSZ 128
/*
- * The buffer cannot cross more than one page boundary due to the
+ * The buffers cannot cross more than one page boundary due to the
* size of the sglist segment array used.
*/
CTASSERT(VTCON_BULK_BUFSZ <= PAGE_SIZE);
+CTASSERT(VTCON_CTRL_BUFSZ <= PAGE_SIZE);
+CTASSERT(sizeof(struct virtio_console_config) <= VTCON_CTRL_BUFSZ);
+
struct vtcon_softc;
struct vtcon_softc_port;
@@ -80,6 +86,7 @@ struct vtcon_port {
int vtcport_flags;
#define VTCON_PORT_FLAG_GONE 0x01
#define VTCON_PORT_FLAG_CONSOLE 0x02
+#define VTCON_PORT_FLAG_ALIAS 0x04
#if defined(KDB)
int vtcport_alt_break_state;
@@ -193,6 +200,8 @@ static void vtcon_port_requeue_buf(struct vtcon_port
static int vtcon_port_populate(struct vtcon_port *);
static void vtcon_port_destroy(struct vtcon_port *);
static int vtcon_port_create(struct vtcon_softc *, int);
+static void vtcon_port_dev_alias(struct vtcon_port *, const char *,
+ size_t);
static void vtcon_port_drain_bufs(struct virtqueue *);
static void vtcon_port_drain(struct vtcon_port *);
static void vtcon_port_teardown(struct vtcon_port *);
@@ -599,8 +608,7 @@ vtcon_ctrl_event_enqueue(struct vtcon_softc *sc,
vq = sc->vtcon_ctrl_rxvq;
sglist_init(&sg, 2, segs);
- error = sglist_append(&sg, control,
- sizeof(struct virtio_console_control) + VTCON_BULK_BUFSZ);
+ error = sglist_append(&sg, control, VTCON_CTRL_BUFSZ);
KASSERT(error == 0, ("%s: error %d adding control to sglist",
__func__, error));
@@ -613,10 +621,7 @@ vtcon_ctrl_event_create(struct vtcon_softc *sc)
struct virtio_console_control *control;
int error;
- control = malloc(
- sizeof(struct virtio_console_control) + VTCON_BULK_BUFSZ,
- M_DEVBUF, M_ZERO | M_NOWAIT);
-
+ control = malloc(VTCON_CTRL_BUFSZ, M_DEVBUF, M_ZERO | M_NOWAIT);
if (control == NULL)
return (ENOMEM);
@@ -633,8 +638,7 @@ vtcon_ctrl_event_requeue(struct vtcon_softc *sc,
{
int error;
- bzero(control, sizeof(struct virtio_console_control) +
- VTCON_BULK_BUFSZ);
+ bzero(control, VTCON_CTRL_BUFSZ);
error = vtcon_ctrl_event_enqueue(sc, control);
KASSERT(error == 0,
@@ -811,19 +815,36 @@ vtcon_ctrl_port_name_event(struct vtcon_softc *sc, int
dev = sc->vtcon_dev;
scport = &sc->vtcon_ports[id];
+ /*
+ * The VirtIO specification says the NUL terminator is not included in
+ * the length, but QEMU includes it. Adjust the length if needed.
+ */
+ if (name == NULL || len == 0)
+ return;
+ if (name[len - 1] == '\0') {
+ len--;
+ if (len == 0)
+ return;
+ }
+
+ VTCON_LOCK(sc);
port = scport->vcsp_port;
if (port == NULL) {
+ VTCON_UNLOCK(sc);
device_printf(dev, "%s: name port %d, but does not exist\n",
__func__, id);
return;
}
- tty_makealias(port->vtcport_tty, "vtcon/%*s", (int)len, name);
+ VTCON_PORT_LOCK(port);
+ VTCON_UNLOCK(sc);
+ vtcon_port_dev_alias(port, name, len);
+ VTCON_PORT_UNLOCK(port);
}
static void
vtcon_ctrl_process_event(struct vtcon_softc *sc,
- struct virtio_console_control *control, void *payload, size_t plen)
+ struct virtio_console_control *control, void *data, size_t data_len)
{
device_t dev;
int id;
@@ -857,9 +878,7 @@ vtcon_ctrl_process_event(struct vtcon_softc *sc,
break;
case VIRTIO_CONSOLE_PORT_NAME:
- if (payload != NULL && plen > 0)
- vtcon_ctrl_port_name_event(sc, id,
- (const char *)payload, plen);
+ vtcon_ctrl_port_name_event(sc, id, (const char *)data, data_len);
break;
}
}
@@ -870,10 +889,10 @@ vtcon_ctrl_task_cb(void *xsc, int pending)
struct vtcon_softc *sc;
struct virtqueue *vq;
struct virtio_console_control *control;
+ void *data;
+ size_t data_len;
int detached;
uint32_t len;
- size_t plen;
- void *payload;
sc = xsc;
vq = sc->vtcon_ctrl_rxvq;
@@ -882,19 +901,19 @@ vtcon_ctrl_task_cb(void *xsc, int pending)
while ((detached = (sc->vtcon_flags & VTCON_FLAG_DETACHED)) == 0) {
control = virtqueue_dequeue(vq, &len);
- payload = NULL;
- plen = 0;
-
if (control == NULL)
break;
- if (len > sizeof(*control)) {
- payload = (void *)(control + 1);
- plen = len - sizeof(*control);
+ if (len > sizeof(struct virtio_console_control)) {
+ data = (void *) &control[1];
+ data_len = len - sizeof(struct virtio_console_control);
+ } else {
+ data = NULL;
+ data_len = 0;
}
VTCON_UNLOCK(sc);
- vtcon_ctrl_process_event(sc, control, payload, plen);
+ vtcon_ctrl_process_event(sc, control, data, data_len);
VTCON_LOCK(sc);
vtcon_ctrl_event_requeue(sc, control);
}
@@ -1130,6 +1149,40 @@ vtcon_port_create(struct vtcon_softc *sc, int id)
device_get_unit(dev), id);
return (0);
+}
+
+static void
+vtcon_port_dev_alias(struct vtcon_port *port, const char *name, size_t len)
+{
+ struct vtcon_softc *sc;
+ struct cdev *pdev;
+ struct tty *tp;
+ int i, error;
+
+ sc = port->vtcport_sc;
+ tp = port->vtcport_tty;
+
+ if (port->vtcport_flags & VTCON_PORT_FLAG_ALIAS)
+ return;
+
+ /* Port name is UTF-8, but we can only handle ASCII. */
+ for (i = 0; i < len; i++) {
+ if (!isascii(name[i]))
+ return;
+ }
+
+ /*
+ * Port name may not conform to the devfs requirements so we cannot use
+ * tty_makealias() because the MAKEDEV_CHECKNAME flag must be specified.
+ */
+ error = make_dev_alias_p(MAKEDEV_NOWAIT | MAKEDEV_CHECKNAME, &pdev,
+ tp->t_dev, "%s/%*s", VTCON_TTY_ALIAS_PREFIX, (int)len, name);
+ if (error) {
+ device_printf(sc->vtcon_dev,
+ "%s: cannot make dev alias (%s/%*s) error %d\n", __func__,
+ VTCON_TTY_ALIAS_PREFIX, (int)len, name, error);
+ } else
+ port->vtcport_flags |= VTCON_PORT_FLAG_ALIAS;
}
static void
More information about the svn-src-head
mailing list