On Wed, Oct 03, 2018 at 12:19:09PM -0700, Mike Larkin wrote:
> On Wed, Oct 03, 2018 at 12:13:05PM -0700, Mike Larkin wrote:
> > On Wed, Oct 03, 2018 at 12:06:47PM -0700, Pratik Vyas wrote:
> > > * Greg Steuck <g...@nest.cx> [2018-10-03 11:40:22 -0700]:
> > >
> > > > $ egdb /syzkaller/src/usr.sbin/vmd/obj/vmd /var/crash/vmd/38082.core
> > > > Core was generated by `vmd'.
> > > > Program terminated with signal SIGFPE, Arithmetic exception.
> > > > #0 0x00000c07a64174a0 in vcpu_process_com_data (vei=<optimized out>,
> > > > vm_id=<optimized out>, vcpu_id=<optimized out>)
> > > > at /syzkaller/src/usr.sbin/vmd/ns8250.c:240
> > > > 240 if (com1_dev.byte_out %
> > > > com1_dev.pause_ct
> > > > == 0) {
> > > > [Current thread is 1 (process 259192)]
> > > > (gdb) p com1_dev.pause_ct
> > > > $1 = 0
> > > > $1 = {mutex = 0xc0a4b1242c0, regs = {lcr = 5 '\005', fcr = 6 '\006',
> > > > iir =
> > > > 3 '\003', ier = 15 '\017', divlo = 64 '@', divhi = 56 '8',
> > > > msr = 0 '\000', lsr = 0 '\000', mcr = 11 '\v', scr = 0 '\000', data
> > > > = 0
> > > > '\000'}, event = {ev_next = {tqe_next = 0xc0a4b120808,
> > > > tqe_prev = 0xc09e1428848}, ev_active_next = {tqe_next = 0x0,
> > > > tqe_prev
> > > > = 0x0}, ev_signal_next = {tqe_next = 0x0, tqe_prev = 0x0},
> > > > min_heap_idx = 4294967295, ev_base = 0xc0a52c3bc00, ev_fd = 9,
> > > > ev_events = 18, ev_ncalls = 0, ev_pncalls = 0x0, ev_timeout = {tv_sec =
> > > > 0,
> > > > tv_usec = 0}, ev_pri = 0, ev_callback = 0xc07a6417340
> > > > <com_rcv_event>, ev_arg = 0x6, ev_res = 0, ev_flags = 4226}, rate =
> > > > {ev_next = {
> > > > tqe_next = 0x0, tqe_prev = 0x0}, ev_active_next = {tqe_next =
> > > > 0xc07a66b65c8 <rtc+256>, tqe_prev = 0xc0a4b121f40}, ev_signal_next = {
> > > > tqe_next = 0x0, tqe_prev = 0x0}, min_heap_idx = 4294967295,
> > > > ev_base =
> > > > 0xc0a52c3bc00, ev_fd = -1, ev_events = 0, ev_ncalls = 0,
> > > > ev_pncalls = 0xc0a4b8f3f68, ev_timeout = {tv_sec = 2745, tv_usec =
> > > > 969355}, ev_pri = 0, ev_callback = 0xc07a64173c0 <ratelimit>, ev_arg =
> > > > 0x0,
> > > > ev_res = 1, ev_flags = 128}, rate_tv = {tv_sec = 0, tv_usec = 10000},
> > > > fd = 9, irq = 4, rcv_pending = 0, vmid = 6, byte_out = 56924,
> > > > baudrate = 8, pause_ct = 0}
> > > >
> > >
> > > Nice :)
> > >
> > > Easy to repro, boot cd and stty com0 4800 in boot>
> > > and continue
> > >
> > > crude diff attached.
> > >
> > >
> > > --
> > > Pratik
> > >
> > > Index: usr.sbin/vmd/ns8250.c
> > > ===================================================================
> > > RCS file: /home/pdvyas/cvs/src/usr.sbin/vmd/ns8250.c,v
> > > retrieving revision 1.17
> > > diff -u -p -a -u -r1.17 ns8250.c
> > > --- usr.sbin/vmd/ns8250.c 12 Jul 2018 10:15:44 -0000 1.17
> > > +++ usr.sbin/vmd/ns8250.c 3 Oct 2018 19:03:08 -0000
> > > @@ -312,13 +312,13 @@ vcpu_process_com_lcr(struct vm_exit *vei
> > > if (vei->vei.vei_dir == VEI_DIR_OUT) {
> > > if (com1_dev.regs.lcr & LCR_DLAB) {
> > > if (!(data & LCR_DLAB)) {
> > > - if (com1_dev.regs.divlo == 0 &&
> > > - com1_dev.regs.divhi == 0) {
> > > + divisor = com1_dev.regs.divlo |
> > > + com1_dev.regs.divhi << 8;
> > > + /* can't set baud < 9600 */
> > > + if (divisor == 0 || (divisor > (115200/9600))) {
> > > log_warnx("%s: ignoring invalid "
> > > "baudrate", __func__);
> > > } else {
> > > - divisor = com1_dev.regs.divlo |
> > > - com1_dev.regs.divhi << 8;
> > > com1_dev.baudrate = 115200 / divisor;
> > > com1_dev.pause_ct =
> > > (com1_dev.baudrate / 8) / 1000 * 10;
> >
> > I have a better diff, stay tuned.
> >
> > -ml
>
> How about this? pd, thoughts?
>
> This code is just the rate limiter code.
>
> Today the code says "have I reached the number of characters output based on
> my baud rate that indicates I need to pause a bit?". And pausing after 0
> characters has been output makes no sense, so only engage the limiter if
> we are pausing between each "1 character or more".
>
> -ml
>
I just noticed that this will effectively remove rate limiting for low
baud rates (like pd's example of 4800 baud). Since this is only used for
the console, I don't think this is a big deal. The rate is only simulated
anyway.
-ml
> Index: ns8250.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/ns8250.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 ns8250.c
> --- ns8250.c 12 Jul 2018 10:15:44 -0000 1.17
> +++ ns8250.c 3 Oct 2018 19:09:37 -0000
> @@ -237,8 +237,9 @@ vcpu_process_com_data(struct vm_exit *ve
>
> if (com1_dev.regs.ier & IER_ETXRDY) {
> /* Limit output rate if needed */
> - if (com1_dev.byte_out % com1_dev.pause_ct == 0) {
> - evtimer_add(&com1_dev.rate, &com1_dev.rate_tv);
> + if (com1_dev.pause_ct > 0) {
> + if (com1_dev.byte_out % com1_dev.pause_ct == 0)
> + evtimer_add(&com1_dev.rate,
> &com1_dev.rate_tv);
> } else {
> /* Set TXRDY and clear "no pending interrupt" */
> com1_dev.regs.iir |= IIR_TXRDY;
>
