Skip to content
/ src Public

Commit 08fd0ce

Browse files
pdvyaspdvyas
committed
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@
1 parent e171566 commit 08fd0ce

File tree

5 files changed

+203
-16
lines changed

5 files changed

+203
-16
lines changed

‎usr.sbin/vmd/i8253.c‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: i8253.c,v 1.31 2019/11/30 00:51:29 mlarkin Exp $ */
1+
/* $OpenBSD: i8253.c,v 1.32 2020/06/28 16:52:45 pd Exp $ */
22
/*
33
* Copyright (c) 2016 Mike Larkin <mlarkin@openbsd.org>
44
*
@@ -30,6 +30,7 @@
3030

3131
#include "i8253.h"
3232
#include "proc.h"
33+
#include "vmd.h"
3334
#include "vmm.h"
3435
#include "atomicio.h"
3536

@@ -43,6 +44,35 @@ extern char *__progname;
4344
*/
4445
struct i8253_channel i8253_channel[3];
4546

47+
static struct vm_dev_pipe dev_pipe;
48+
49+
/*
50+
* i8253_pipe_dispatch
51+
*
52+
* Reads a message off the pipe, expecting one that corresponds to a
53+
* reset request for a specific channel.
54+
*/
55+
static void
56+
i8253_pipe_dispatch(int fd, short event, void *arg)
57+
{
58+
enum pipe_msg_type msg;
59+
60+
msg = vm_pipe_recv(&dev_pipe);
61+
switch (msg) {
62+
case I8253_RESET_CHAN_0:
63+
i8253_reset(0);
64+
break;
65+
case I8253_RESET_CHAN_1:
66+
i8253_reset(1);
67+
break;
68+
case I8253_RESET_CHAN_2:
69+
i8253_reset(2);
70+
break;
71+
default:
72+
fatalx("%s: unexpected pipe message %d", __func__, msg);
73+
}
74+
}
75+
4676
/*
4777
* i8253_init
4878
*
@@ -77,6 +107,9 @@ i8253_init(uint32_t vm_id)
77107
evtimer_set(&i8253_channel[0].timer, i8253_fire, &i8253_channel[0]);
78108
evtimer_set(&i8253_channel[1].timer, i8253_fire, &i8253_channel[1]);
79109
evtimer_set(&i8253_channel[2].timer, i8253_fire, &i8253_channel[2]);
110+
111+
vm_pipe_init(&dev_pipe, i8253_pipe_dispatch);
112+
event_add(&dev_pipe.read_ev, NULL);
80113
}
81114

82115
/*
@@ -271,7 +304,7 @@ vcpu_exit_i8253(struct vm_run_params *vrp)
271304
sel, i8253_channel[sel].mode,
272305
i8253_channel[sel].start);
273306

274-
i8253_reset(sel);
307+
vm_pipe_send(&dev_pipe, sel);
275308
}
276309
} else {
277310
if (i8253_channel[sel].rbs) {

‎usr.sbin/vmd/mc146818.c‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: mc146818.c,v 1.21 2019/11/30 00:51:29 mlarkin Exp $ */
1+
/* $OpenBSD: mc146818.c,v 1.22 2020/06/28 16:52:45 pd Exp $ */
22
/*
33
* Copyright (c) 2016 Mike Larkin <mlarkin@openbsd.org>
44
*
@@ -63,6 +63,29 @@ struct mc146818 {
6363

6464
struct mc146818 rtc;
6565

66+
static struct vm_dev_pipe dev_pipe;
67+
68+
static void rtc_reschedule_per(void);
69+
70+
/*
71+
* mc146818_pipe_dispatch
72+
*
73+
* Reads a message off the pipe, expecting a request to reschedule periodic
74+
* interrupt rate.
75+
*/
76+
static void
77+
mc146818_pipe_dispatch(int fd, short event, void *arg)
78+
{
79+
enum pipe_msg_type msg;
80+
81+
msg = vm_pipe_recv(&dev_pipe);
82+
if (msg == MC146818_RESCHEDULE_PER) {
83+
rtc_reschedule_per();
84+
} else {
85+
fatalx("%s: unexpected pipe message %d", __func__, msg);
86+
}
87+
}
88+
6689
/*
6790
* rtc_updateregs
6891
*
@@ -175,6 +198,9 @@ mc146818_init(uint32_t vm_id, uint64_t memlo, uint64_t memhi)
175198
evtimer_add(&rtc.sec, &rtc.sec_tv);
176199

177200
evtimer_set(&rtc.per, rtc_fireper, (void *)(intptr_t)rtc.vm_id);
201+
202+
vm_pipe_init(&dev_pipe, mc146818_pipe_dispatch);
203+
event_add(&dev_pipe.read_ev, NULL);
178204
}
179205

180206
/*
@@ -217,7 +243,7 @@ rtc_update_rega(uint32_t data)
217243

218244
rtc.regs[MC_REGA] = data;
219245
if ((rtc.regs[MC_REGA] ^ data) & 0x0f)
220-
rtc_reschedule_per();
246+
vm_pipe_send(&dev_pipe, MC146818_RESCHEDULE_PER);
221247
}
222248

223249
/*
@@ -240,7 +266,7 @@ rtc_update_regb(uint32_t data)
240266
rtc.regs[MC_REGB] = data;
241267

242268
if (data & MC_REGB_PIE)
243-
rtc_reschedule_per();
269+
vm_pipe_send(&dev_pipe, MC146818_RESCHEDULE_PER);
244270
}
245271

246272
/*

‎usr.sbin/vmd/ns8250.c‎

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ns8250.c,v 1.28 2020/06/21 20:36:07 pd Exp $ */
1+
/* $OpenBSD: ns8250.c,v 1.29 2020/06/28 16:52:45 pd Exp $ */
22
/*
33
* Copyright (c) 2016 Mike Larkin <mlarkin@openbsd.org>
44
*
@@ -37,8 +37,40 @@
3737
extern char *__progname;
3838
struct ns8250_dev com1_dev;
3939

40+
/* Flags to distinguish calling threads to com_rcv */
41+
#define NS8250_DEV_THREAD 0
42+
#define NS8250_CPU_THREAD 1
43+
44+
static struct vm_dev_pipe dev_pipe;
45+
4046
static void com_rcv_event(int, short, void *);
41-
static void com_rcv(struct ns8250_dev *, uint32_t, uint32_t);
47+
static void com_rcv(struct ns8250_dev *, uint32_t, uint32_t, int);
48+
49+
/*
50+
* ns8250_pipe_dispatch
51+
*
52+
* Reads a message off the pipe, expecting a reguest to reset events after a
53+
* zero-byte read from the com device.
54+
*/
55+
static void
56+
ns8250_pipe_dispatch(int fd, short event, void *arg)
57+
{
58+
enum pipe_msg_type msg;
59+
60+
msg = vm_pipe_recv(&dev_pipe);
61+
switch(msg) {
62+
case NS8250_ZERO_READ:
63+
log_debug("%s: resetting events after zero byte read", __func__);
64+
event_del(&com1_dev.event);
65+
event_add(&com1_dev.wake, NULL);
66+
break;
67+
case NS8250_RATELIMIT:
68+
evtimer_add(&com1_dev.rate, &com1_dev.rate_tv);
69+
break;
70+
default:
71+
fatalx("%s: unexpected pipe message %d", __func__, msg);
72+
}
73+
}
4274

4375
/*
4476
* ratelimit
@@ -75,6 +107,7 @@ ns8250_init(int fd, uint32_t vmid)
75107
errno = ret;
76108
fatal("could not initialize com1 mutex");
77109
}
110+
78111
com1_dev.fd = fd;
79112
com1_dev.irq = 4;
80113
com1_dev.portid = NS8250_COM1;
@@ -115,6 +148,9 @@ ns8250_init(int fd, uint32_t vmid)
115148
timerclear(&com1_dev.rate_tv);
116149
com1_dev.rate_tv.tv_usec = 10000;
117150
evtimer_set(&com1_dev.rate, ratelimit, NULL);
151+
152+
vm_pipe_init(&dev_pipe, ns8250_pipe_dispatch);
153+
event_add(&dev_pipe.read_ev, NULL);
118154
}
119155

120156
static void
@@ -137,7 +173,7 @@ com_rcv_event(int fd, short kind, void *arg)
137173
if (com1_dev.regs.lsr & LSR_RXRDY)
138174
com1_dev.rcv_pending = 1;
139175
else
140-
com_rcv(&com1_dev, (uintptr_t)arg, 0);
176+
com_rcv(&com1_dev, (uintptr_t)arg, 0, NS8250_DEV_THREAD);
141177
}
142178

143179
/* If pending interrupt, inject */
@@ -181,7 +217,7 @@ com_rcv_handle_break(struct ns8250_dev *com, uint8_t cmd)
181217
* Must be called with the mutex of the com device acquired
182218
*/
183219
static void
184-
com_rcv(struct ns8250_dev *com, uint32_t vm_id, uint32_t vcpu_id)
220+
com_rcv(struct ns8250_dev *com, uint32_t vm_id, uint32_t vcpu_id, int thread)
185221
{
186222
char buf[2];
187223
ssize_t sz;
@@ -201,8 +237,13 @@ com_rcv(struct ns8250_dev *com, uint32_t vm_id, uint32_t vcpu_id)
201237
if (errno != EAGAIN)
202238
log_warn("unexpected read error on com device");
203239
} else if (sz == 0) {
204-
event_del(&com->event);
205-
event_add(&com->wake, NULL);
240+
if (thread == NS8250_DEV_THREAD) {
241+
event_del(&com->event);
242+
event_add(&com->wake, NULL);
243+
} else {
244+
/* Called by vcpu thread, use pipe for event changes */
245+
vm_pipe_send(&dev_pipe, NS8250_ZERO_READ);
246+
}
206247
return;
207248
} else if (sz != 1 && sz != 2)
208249
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)
258299
/* Limit output rate if needed */
259300
if (com1_dev.pause_ct > 0 &&
260301
com1_dev.byte_out % com1_dev.pause_ct == 0) {
261-
evtimer_add(&com1_dev.rate,
262-
&com1_dev.rate_tv);
302+
vm_pipe_send(&dev_pipe, NS8250_RATELIMIT);
263303
} else {
264304
/* Set TXRDY and clear "no pending interrupt" */
265305
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)
300340
com1_dev.regs.iir = 0x1;
301341

302342
if (com1_dev.rcv_pending)
303-
com_rcv(&com1_dev, vm_id, vcpu_id);
343+
com_rcv(&com1_dev, vm_id, vcpu_id, NS8250_CPU_THREAD);
304344
}
305345

306346
/* If pending interrupt, make sure it gets injected */

‎usr.sbin/vmd/vm.c‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: vm.c,v 1.57 2020/04/30 03:50:53 pd Exp $ */
1+
/* $OpenBSD: vm.c,v 1.58 2020/06/28 16:52:45 pd Exp $ */
22

33
/*
44
* 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)
22432243

22442244
return (0);
22452245
}
2246+
2247+
/*
2248+
* vm_pipe_init
2249+
*
2250+
* Initialize a vm_dev_pipe, setting up its file descriptors and its
2251+
* event structure with the given callback.
2252+
*
2253+
* Parameters:
2254+
* p: pointer to vm_dev_pipe struct to initizlize
2255+
* cb: callback to use for READ events on the read end of the pipe
2256+
*/
2257+
void
2258+
vm_pipe_init(struct vm_dev_pipe *p, void (*cb)(int, short, void *))
2259+
{
2260+
int ret;
2261+
int fds[2];
2262+
2263+
memset(p, 0, sizeof(struct vm_dev_pipe));
2264+
2265+
ret = pipe(fds);
2266+
if (ret)
2267+
fatal("failed to create vm_dev_pipe pipe");
2268+
2269+
p->read = fds[0];
2270+
p->write = fds[1];
2271+
2272+
event_set(&p->read_ev, p->read, EV_READ | EV_PERSIST, cb, NULL);
2273+
}
2274+
2275+
/*
2276+
* vm_pipe_send
2277+
*
2278+
* Send a message to an emulated device vie the provided vm_dev_pipe.
2279+
*
2280+
* Parameters:
2281+
* p: pointer to initialized vm_dev_pipe
2282+
* msg: message to send in the channel
2283+
*/
2284+
void
2285+
vm_pipe_send(struct vm_dev_pipe *p, enum pipe_msg_type msg)
2286+
{
2287+
size_t n;
2288+
n = write(p->write, &msg, sizeof(msg));
2289+
if (n != sizeof(msg))
2290+
fatal("failed to write to device pipe");
2291+
}
2292+
2293+
/*
2294+
* vm_pipe_recv
2295+
*
2296+
* Receive a message for an emulated device via the provided vm_dev_pipe.
2297+
* Returns the message value, otherwise will exit on failure.
2298+
*
2299+
* Parameters:
2300+
* p: pointer to initialized vm_dev_pipe
2301+
*
2302+
* Return values:
2303+
* a value of enum pipe_msg_type or fatal exit on read(2) error
2304+
*/
2305+
enum pipe_msg_type
2306+
vm_pipe_recv(struct vm_dev_pipe *p)
2307+
{
2308+
size_t n;
2309+
enum pipe_msg_type msg;
2310+
n = read(p->read, &msg, sizeof(msg));
2311+
if (n != sizeof(msg))
2312+
fatal("failed to read from device pipe");
2313+
2314+
return msg;
2315+
}

‎usr.sbin/vmd/vmd.h‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: vmd.h,v 1.98 2019/12/12 03:53:38 pd Exp $ */
1+
/* $OpenBSD: vmd.h,v 1.99 2020/06/28 16:52:45 pd Exp $ */
22

33
/*
44
* Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -354,6 +354,21 @@ struct vmd {
354354
int vmd_ptmfd;
355355
};
356356

357+
struct vm_dev_pipe {
358+
int read;
359+
int write;
360+
struct event read_ev;
361+
};
362+
363+
enum pipe_msg_type {
364+
I8253_RESET_CHAN_0 = 0,
365+
I8253_RESET_CHAN_1 = 1,
366+
I8253_RESET_CHAN_2 = 2,
367+
NS8250_ZERO_READ,
368+
NS8250_RATELIMIT,
369+
MC146818_RESCHEDULE_PER
370+
};
371+
357372
static inline struct sockaddr_in *
358373
ss2sin(struct sockaddr_storage *ss)
359374
{
@@ -442,6 +457,9 @@ int vmm_pipe(struct vmd_vm *, int, void (*)(int, short, void *));
442457
/* vm.c */
443458
int start_vm(struct vmd_vm *, int);
444459
__dead void vm_shutdown(unsigned int);
460+
void vm_pipe_init(struct vm_dev_pipe *, void (*)(int, short, void *));
461+
void vm_pipe_send(struct vm_dev_pipe *, enum pipe_msg_type);
462+
enum pipe_msg_type vm_pipe_recv(struct vm_dev_pipe *);
445463

446464
/* control.c */
447465
int config_init(struct vmd *);

0 commit comments

Comments
 (0)