vmd(8): Eliminate libevent state corruption

libevent functions for com, pic and rtc are now only called on event_thread.
vcpu exit handlers send messages on a dev pipe and callbacks on these events do
the event management (event_add, evtimer_add, etc).  Previously, libevent state
was mutated by two threads, event_thread, that runs all the callbacks and the
vcpu thread when running exit handlers.  This could have lead to libevent state
corruption.

Patch from Dave Voutila <dave@sisu.io>

ok claudio@
tested by abieber@ and brynet@

usr.sbin/vmd/i8253.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: i8253.c,v 1.31 2019/11/30 00:51:29 mlarkin Exp $ */
+/* $OpenBSD: i8253.c,v 1.32 2020/06/28 16:52:45 pd Exp $ */
 /*
  * Copyright (c) 2016 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -30,6 +30,7 @@
 
 #include "i8253.h"
 #include "proc.h"
+#include "vmd.h"
 #include "vmm.h"
 #include "atomicio.h"
 
@@ -43,6 +44,35 @@ extern char *__progname;
  */
 struct i8253_channel i8253_channel[3];
 
+static struct vm_dev_pipe dev_pipe;
+
+/*
+ * i8253_pipe_dispatch
+ *
+ * Reads a message off the pipe, expecting one that corresponds to a
+ * reset request for a specific channel.
+ */
+static void
+i8253_pipe_dispatch(int fd, short event, void *arg)
+{
+	enum pipe_msg_type msg;
+
+	msg = vm_pipe_recv(&dev_pipe);
+	switch (msg) {
+	case I8253_RESET_CHAN_0:
+		i8253_reset(0);
+		break;
+	case I8253_RESET_CHAN_1:
+		i8253_reset(1);
+		break;
+	case I8253_RESET_CHAN_2:
+		i8253_reset(2);
+		break;
+	default:
+		fatalx("%s: unexpected pipe message %d", __func__, msg);
+	}
+}
+
 /*
  * i8253_init
  *
@@ -77,6 +107,9 @@ i8253_init(uint32_t vm_id)
 	evtimer_set(&i8253_channel[0].timer, i8253_fire, &i8253_channel[0]);
 	evtimer_set(&i8253_channel[1].timer, i8253_fire, &i8253_channel[1]);
 	evtimer_set(&i8253_channel[2].timer, i8253_fire, &i8253_channel[2]);
+
+	vm_pipe_init(&dev_pipe, i8253_pipe_dispatch);
+	event_add(&dev_pipe.read_ev, NULL);
 }
 
 /*
@@ -271,7 +304,7 @@ vcpu_exit_i8253(struct vm_run_params *vrp)
 				    sel, i8253_channel[sel].mode,
 				    i8253_channel[sel].start);
 
-				i8253_reset(sel);
+				vm_pipe_send(&dev_pipe, sel);
 			}
 		} else {
 			if (i8253_channel[sel].rbs) {

usr.sbin/vmd/mc146818.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: mc146818.c,v 1.21 2019/11/30 00:51:29 mlarkin Exp $ */
+/* $OpenBSD: mc146818.c,v 1.22 2020/06/28 16:52:45 pd Exp $ */
 /*
  * Copyright (c) 2016 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -63,6 +63,29 @@ struct mc146818 {
 
 struct mc146818 rtc;
 
+static struct vm_dev_pipe dev_pipe;
+
+static void rtc_reschedule_per(void);
+
+/*
+ * mc146818_pipe_dispatch
+ *
+ * Reads a message off the pipe, expecting a request to reschedule periodic
+ * interrupt rate.
+ */
+static void
+mc146818_pipe_dispatch(int fd, short event, void *arg)
+{
+	enum pipe_msg_type msg;
+
+	msg = vm_pipe_recv(&dev_pipe);
+	if (msg == MC146818_RESCHEDULE_PER) {
+		rtc_reschedule_per();
+	} else {
+		fatalx("%s: unexpected pipe message %d", __func__, msg);
+	}
+}
+
 /*
  * rtc_updateregs
  *
@@ -175,6 +198,9 @@ mc146818_init(uint32_t vm_id, uint64_t memlo, uint64_t memhi)
 	evtimer_add(&rtc.sec, &rtc.sec_tv);
 
 	evtimer_set(&rtc.per, rtc_fireper, (void *)(intptr_t)rtc.vm_id);
+
+	vm_pipe_init(&dev_pipe, mc146818_pipe_dispatch);
+	event_add(&dev_pipe.read_ev, NULL);
 }
 
 /*
@@ -217,7 +243,7 @@ rtc_update_rega(uint32_t data)
 
 	rtc.regs[MC_REGA] = data;
 	if ((rtc.regs[MC_REGA] ^ data) & 0x0f)
-		rtc_reschedule_per();
+		vm_pipe_send(&dev_pipe, MC146818_RESCHEDULE_PER);
 }
 
 /*
@@ -240,7 +266,7 @@ rtc_update_regb(uint32_t data)
 	rtc.regs[MC_REGB] = data;
 
 	if (data & MC_REGB_PIE)
-		rtc_reschedule_per();
+		vm_pipe_send(&dev_pipe, MC146818_RESCHEDULE_PER);
 }
 
 /*

usr.sbin/vmd/ns8250.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ns8250.c,v 1.28 2020/06/21 20:36:07 pd Exp $ */
+/* $OpenBSD: ns8250.c,v 1.29 2020/06/28 16:52:45 pd Exp $ */
 /*
  * Copyright (c) 2016 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -37,8 +37,40 @@
 extern char *__progname;
 struct ns8250_dev com1_dev;
 
+/* Flags to distinguish calling threads to com_rcv */
+#define NS8250_DEV_THREAD	0
+#define NS8250_CPU_THREAD 	1
+
+static struct vm_dev_pipe dev_pipe;
+
 static void com_rcv_event(int, short, void *);
-static void com_rcv(struct ns8250_dev *, uint32_t, uint32_t);
+static void com_rcv(struct ns8250_dev *, uint32_t, uint32_t, int);
+
+/*
+ * ns8250_pipe_dispatch
+ *
+ * Reads a message off the pipe, expecting a reguest to reset events after a
+ * zero-byte read from the com device.
+ */
+static void
+ns8250_pipe_dispatch(int fd, short event, void *arg)
+{
+	enum pipe_msg_type msg;
+
+	msg = vm_pipe_recv(&dev_pipe);
+	switch(msg) {
+	case NS8250_ZERO_READ:
+		log_debug("%s: resetting events after zero byte read", __func__);
+		event_del(&com1_dev.event);
+		event_add(&com1_dev.wake, NULL);
+		break;
+	case NS8250_RATELIMIT:
+		evtimer_add(&com1_dev.rate, &com1_dev.rate_tv);
+		break;
+	default:
+		fatalx("%s: unexpected pipe message %d", __func__, msg);
+	}
+}
 
 /*
  * ratelimit
@@ -75,6 +107,7 @@ ns8250_init(int fd, uint32_t vmid)
 		errno = ret;
 		fatal("could not initialize com1 mutex");
 	}
+
 	com1_dev.fd = fd;
 	com1_dev.irq = 4;
 	com1_dev.portid = NS8250_COM1;
@@ -115,6 +148,9 @@ ns8250_init(int fd, uint32_t vmid)
 	timerclear(&com1_dev.rate_tv);
 	com1_dev.rate_tv.tv_usec = 10000;
 	evtimer_set(&com1_dev.rate, ratelimit, NULL);
+
+	vm_pipe_init(&dev_pipe, ns8250_pipe_dispatch);
+	event_add(&dev_pipe.read_ev, NULL);
 }
 
 static void
@@ -137,7 +173,7 @@ com_rcv_event(int fd, short kind, void *arg)
 		if (com1_dev.regs.lsr & LSR_RXRDY)
 			com1_dev.rcv_pending = 1;
 		else
-			com_rcv(&com1_dev, (uintptr_t)arg, 0);
+			com_rcv(&com1_dev, (uintptr_t)arg, 0, NS8250_DEV_THREAD);
 	}
 
 	/* If pending interrupt, inject */
@@ -181,7 +217,7 @@ com_rcv_handle_break(struct ns8250_dev *com, uint8_t cmd)
  * Must be called with the mutex of the com device acquired
  */
 static void
-com_rcv(struct ns8250_dev *com, uint32_t vm_id, uint32_t vcpu_id)
+com_rcv(struct ns8250_dev *com, uint32_t vm_id, uint32_t vcpu_id, int thread)
 {
 	char buf[2];
 	ssize_t sz;
@@ -201,8 +237,13 @@ com_rcv(struct ns8250_dev *com, uint32_t vm_id, uint32_t vcpu_id)
 		if (errno != EAGAIN)
 			log_warn("unexpected read error on com device");
 	} else if (sz == 0) {
-		event_del(&com->event);
-		event_add(&com->wake, NULL);
+		if (thread == NS8250_DEV_THREAD) {
+			event_del(&com->event);
+			event_add(&com->wake, NULL);
+		} else {
+			/* Called by vcpu thread, use pipe for event changes */
+			vm_pipe_send(&dev_pipe, NS8250_ZERO_READ);
+		}
 		return;
 	} else if (sz != 1 && sz != 2)
 		log_warnx("unexpected read return value %zd on com device", sz);
@@ -258,8 +299,7 @@ vcpu_process_com_data(struct vm_exit *vei, uint32_t vm_id, uint32_t vcpu_id)
 			/* Limit output rate if needed */
 			if (com1_dev.pause_ct > 0 &&
 			    com1_dev.byte_out % com1_dev.pause_ct == 0) {
-					evtimer_add(&com1_dev.rate,
-					    &com1_dev.rate_tv);
+				vm_pipe_send(&dev_pipe, NS8250_RATELIMIT);
 			} else {
 				/* Set TXRDY and clear "no pending interrupt" */
 				com1_dev.regs.iir |= IIR_TXRDY;
@@ -300,7 +340,7 @@ vcpu_process_com_data(struct vm_exit *vei, uint32_t vm_id, uint32_t vcpu_id)
 			com1_dev.regs.iir = 0x1;
 
 		if (com1_dev.rcv_pending)
-			com_rcv(&com1_dev, vm_id, vcpu_id);
+			com_rcv(&com1_dev, vm_id, vcpu_id, NS8250_CPU_THREAD);
 	}
 
 	/* If pending interrupt, make sure it gets injected */

usr.sbin/vmd/vm.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: vm.c,v 1.57 2020/04/30 03:50:53 pd Exp $	*/
+/*	$OpenBSD: vm.c,v 1.58 2020/06/28 16:52:45 pd Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -2243,3 +2243,73 @@ translate_gva(struct vm_exit* exit, uint64_t va, uint64_t* pa, int mode)
 
 	return (0);
 }
+
+/*
+ * vm_pipe_init
+ *
+ * Initialize a vm_dev_pipe, setting up its file descriptors and its
+ * event structure with the given callback.
+ *
+ * Parameters:
+ *  p: pointer to vm_dev_pipe struct to initizlize
+ *  cb: callback to use for READ events on the read end of the pipe
+ */
+void
+vm_pipe_init(struct vm_dev_pipe *p, void (*cb)(int, short, void *))
+{
+	int ret;
+	int fds[2];
+
+	memset(p, 0, sizeof(struct vm_dev_pipe));
+
+	ret = pipe(fds);
+	if (ret)
+		fatal("failed to create vm_dev_pipe pipe");
+
+	p->read = fds[0];
+	p->write = fds[1];
+
+	event_set(&p->read_ev, p->read, EV_READ | EV_PERSIST, cb, NULL);
+}
+
+/*
+ * vm_pipe_send
+ *
+ * Send a message to an emulated device vie the provided vm_dev_pipe.
+ *
+ * Parameters:
+ *  p: pointer to initialized vm_dev_pipe
+ *  msg: message to send in the channel
+ */
+void
+vm_pipe_send(struct vm_dev_pipe *p, enum pipe_msg_type msg)
+{
+	size_t n;
+	n = write(p->write, &msg, sizeof(msg));
+	if (n != sizeof(msg))
+		fatal("failed to write to device pipe");
+}
+
+/*
+ * vm_pipe_recv
+ *
+ * Receive a message for an emulated device via the provided vm_dev_pipe.
+ * Returns the message value, otherwise will exit on failure.
+ *
+ * Parameters:
+ *  p: pointer to initialized vm_dev_pipe
+ *
+ * Return values:
+ *  a value of enum pipe_msg_type or fatal exit on read(2) error
+ */
+enum pipe_msg_type
+vm_pipe_recv(struct vm_dev_pipe *p)
+{
+	size_t n;
+	enum pipe_msg_type msg;
+	n = read(p->read, &msg, sizeof(msg));
+	if (n != sizeof(msg))
+		fatal("failed to read from device pipe");
+
+	return msg;
+}

usr.sbin/vmd/vmd.h

@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.h,v 1.98 2019/12/12 03:53:38 pd Exp $	*/
+/*	$OpenBSD: vmd.h,v 1.99 2020/06/28 16:52:45 pd Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -354,6 +354,21 @@ struct vmd {
 	int			 vmd_ptmfd;
 };
 
+struct vm_dev_pipe {
+	int			 read;
+	int			 write;
+	struct event		 read_ev;
+};
+
+enum pipe_msg_type {
+	I8253_RESET_CHAN_0 = 0,
+	I8253_RESET_CHAN_1 = 1,
+	I8253_RESET_CHAN_2 = 2,
+	NS8250_ZERO_READ,
+	NS8250_RATELIMIT,
+	MC146818_RESCHEDULE_PER
+};
+
 static inline struct sockaddr_in *
 ss2sin(struct sockaddr_storage *ss)
 {
@@ -442,6 +457,9 @@ int	 vmm_pipe(struct vmd_vm *, int, void (*)(int, short, void *));
 /* vm.c */
 int	 start_vm(struct vmd_vm *, int);
 __dead void vm_shutdown(unsigned int);
+void	 vm_pipe_init(struct vm_dev_pipe *, void (*)(int, short, void *));
+void	 vm_pipe_send(struct vm_dev_pipe *, enum pipe_msg_type);
+enum pipe_msg_type vm_pipe_recv(struct vm_dev_pipe *);
 
 /* control.c */
 int	 config_init(struct vmd *);