Xvisor Development › [PATCH] CORE: blockdev: Fix stack frame corruption

README.md

CORE: vmm_blockdev: Fix stack frame corruption

This patch fixes stack frame corruption caused by a race condition between issuer and worker on the request queue.

Patch

diff --git a/core/block/vmm_blockdev.c b/core/block/vmm_blockdev.c
index 0c8b488..d32d768 100644
--- a/core/block/vmm_blockdev.c
+++ b/core/block/vmm_blockdev.c
@@ -127,7 +127,6 @@ int vmm_blockdev_complete_request(struct vmm_request *r)
        vmm_spin_lock_irqsave(&rq->lock, flags);
        __blockdev_done_request(rq);
        vmm_spin_unlock_irqrestore(&rq->lock, flags);
-       r->bdev = NULL;
 
        return VMM_OK;
 }
@@ -149,7 +148,6 @@ int vmm_blockdev_fail_request(struct vmm_request *r)
        vmm_spin_lock_irqsave(&rq->lock, flags);
        __blockdev_done_request(rq);
        vmm_spin_unlock_irqrestore(&rq->lock, flags);
-       r->bdev = NULL;
 
        return VMM_OK;
 }
@@ -268,6 +266,7 @@ static void blockdev_rw_completed(struct vmm_request *req)
                return;
        }
 
+       req->bdev = NULL;
        rw->failed = FALSE;
        vmm_completion_complete(&rw->done);
 }
@@ -280,6 +279,7 @@ static void blockdev_rw_failed(struct vmm_request *req)
                return;
        }
 
+       req->bdev = NULL;
        rw->failed = TRUE;
        vmm_completion_complete(&rw->done);
 }

Sequence Diagram

Edit@www.planttext.com

bdev_complete_race.svg

Investigation tools

run-qemu.sh

#!/bin/sh                                                                                                                                                                 
~/github/qemu-3.1.0/aarch64-softmmu/qemu-system-aarch64 \
 -gdb tcp::9000 -S -semihosting \
 -nodefaults -nographic -monitor none -serial stdio -usb \
 -M raspi3 \
 -kernel build/vmm.bin \
 -drive if=sd,format=raw,file=mmc0.img \
 -dtb build/arch/arm/board/generic/dts/broadcom/bcm2837-rpi-3-b.dtb

dlist.gdb

define dlist
  set $offset = 0
  if $argc > 1
    set $offset = (int)$arg1
  end
  printf "---> entries of (struct dlist*)%p\n", &$arg0
  set $cur = $arg0.next
  set $i = 0
  while $cur != &$arg0
    if $argc > 2
      eval $arg2, (void*)$cur - $offset
    else
      printf "dlist@%d = (struct ???*)%p\n", $i++, (void*)$cur - $offset
    end
    set $cur = $cur.next
  end
  printf "<--- end of dlist\n"
end

document dlist
dlist <dlist> [<dlist offset in target structure>] [<eval format string>]
usage: dlist 'core/vmm_workqueue.c'::wqctrl 8
usage: dlist $wqctl->wq_list 8
usage: dlist $wqctl->wq_list 8 "p (struct vmm_work*)%p"

How to calculate the <dlist offset in target structure>:
  (gdb) p/a &$wqctl->wq_list
  $2 = 0x101f1008 <wqctrl+8>
  (gdb) p/a &((struct vmm_workqueue_ctrl *)0)->wq_list
  $1 = 0x8
  (gdb) info symbol &$wqctl->wq_list
  wqctrl + 8 in section .bss
These results implies 8.
end

# @note mterm vcpu is assigned to hcpu0, and priority is VMM_VCPU_DEF_PRIORITY==3.
define stat_mterm
  set $schedp0 = (struct vmm_scheduler_ctrl*)((void*)&percpu_sched + __percpu_offset[0])
  set $schedp0rq = (struct vmm_schedalgo_rq*)$schedp0->rq
  if $schedp0rq->list[3].next == &$schedp0rq->list[3]
    if $schedp0->current_vcpu->name[0] == 'm' && $schedp0->current_vcpu->name[1] == 't' && $schedp0->current_vcpu->name[2] == 'e' && $schedp0->current_vcpu->name[3] == 'r' && $schedp0->current_vcpu->name[4] == 'm'
      printf "mterm is current_vcpu [#####]: "
    else
      printf "mterm vcpu is unreachable from schedp [?????]: "
    end
  else
    printf "mterm exists in rq [@@@@@]: "
  end
  printf "$schedp0=%p $schedp0rq=%p $schedp0->current_vcpu=%p{state=%d}\n", $schedp0, $schedp0rq, $schedp0->current_vcpu, $schedp0->current_vcpu->state.counter
  if $schedp0rq->list[3].next != &$schedp0rq->list[3]
    dlist $schedp0rq->list[3] 0 "p (struct vmm_schedalgo_rq_entry*)%p"
  end
end

# usage: stat_brwq $rw_done
define stat_brwq
  printf "(vmm_completion*)%p: wq.vcpu_count=%d\n", &$arg0, $arg0.wq.vcpu_count
  if $arg0.wq.vcpu_count > 0
    set $vcpu_off = (off_t)&((struct vmm_vcpu*)0)->wq_head
    dlist $arg0.wq.vcpu_list $vcpu_off "p (struct vmm_vcpu*)%p"
  end
end

define stat_mmc0
  set $wqctl=&'core/vmm_workqueue.c'::wqctrl
  set $cur = $wqctl.wq_list.next
  while $cur != &$wqctl.wq_list
    set $wq = (struct vmm_workqueue*)((void*)$cur - (off_t)&(((struct vmm_workqueue*)0)->head))
    set $wq_vcpu = $wq->thread->tvcpu
    if $wq_vcpu->name[0] == 'm' && $wq_vcpu->name[1] == 'm' && $wq_vcpu->name[2] == 'c' && $wq_vcpu->name[3] == '0'
      printf "mmc0: wq=%p {thread->vcpu=%p, work_list=%p, work_avail=%p}\n", $wq, $wq_vcpu, &$wq->work_list, &$wq->work_avail

      printf "### mmc0: foreach (struct blockrq_work*)wq->work_list ###\n"
      set $off_bw = (off_t)&((struct blockrq_work*)0)->work.head
      dlist $wq->work_list $off_bw "p *(struct blockrq_work*)%p"

      printf "### mmc0: (struct vmm_request*)bwork->d.rw.r @wq->work_list ###\n"
      dlist $wq->work_list $off_bw "p *((struct blockrq_work*)%p)->d.rw.r"

      printf "### mmc0: foreach (struct vmm_completion)wq.work_avail ### \n"
      stat_brwq $wq->work_avail

      set $cur = &$wqctl.wq_list
    else
      set $cur = $cur.next
    end
  end
end

run-qemu.gdb

# usage: M-x gdb => ../gcc-linaro-5.3.1-2016.05-rc2-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-gdb -i=mi -x run-qemu.gdb

source ./dlist.gdb

add-symbol-file build/vmm.elf 0x10000000

# NOTE: It is necessary to execute qemu as remote target in advance.
# % qemu-2.12.0/aarch64-softmmu/qemu-system-aarch64 -gdb tcp::9000 -S -semihosting -nodefaults -serial mon:stdio -nographic -M raspi3 -kernel build/vmm.bin -dtb bcm2837-r
target remote localhost:9000

# copy dtb (QEMU modified) from 0x8000000 to 0x800000
set $x0 = 0x800000
set $x1 = 0x8000000
set $x2 = 0x1000
call memcpy

# b core/vmm_workqueue.c:263
# commands
#   set $mmc0_host = (struct mmc_host*)((void*)mmc_host_list->next - (off_t)&((struct mmc_host*)0)->link)
#   if $mmc0_host->brq->wq == wq
#     printf "$mmc0_host->brq->wq = (struct vmm_workqueue*)%p ====\n", wq
#     set $off = (off_t)&((struct vmm_work*)0)->head
#     dlist wq->work_list $off "p *(struct vmm_work*)%p"
#   end
#   cont
# end


#????????	list_add_tail(&bwork->head, &brq->wq_pending_list);
# dlist $mmc0_host->brq->wq_pending_list


b vmm_blockdev.c:308
commands
  set $rw_done = &rw.done
  printf ">>>> &rw.done=%p, rw.req=", &rw.done
  p rw.req
  stat_mmc0
#  watch -l *&rw.req.bdev
#  commands
#    frame
#    cont
#  end
  cont
end

## vmm_workqueue_schedule_work@vmm_workqueue.c
b vmm_workqueue.c:258
commands
  set $wq_vcpu = wq->thread->tvcpu
  if $wq_vcpu->name[0] == 'm' && $wq_vcpu->name[1] == 'm' && $wq_vcpu->name[2] == 'c' && $wq_vcpu->name[3] == '0'
    set $bwork = (struct blockrq_work *)((void*)work - (off_t)&((struct blockrq_work*)0)->work)
    p *$bwork
    p *$bwork->d.rw.r
    if $bwork->is_rw && $bwork->d.rw.r.bdev == 0
      printf "************** ERROR: bdev=0 ***************\n"
    else
      cont
    end
  else
    cont
  end
end

b vmm_blockdev.c:310 if $rw_done == &rw.done
commands
  printf "<<<< &rw.done=%p, rw.req=", &rw.done
  p rw.req
  stat_mmc0
  cont
end

#### blockrq_rw_done() -> vmm_blockdev_complete_request(struct vmm_request *r) で r.bdev が 0
b vmm_blockdev.c:120
### blockdev_rw_blocks() -> vmm_blockdev_submit_request()@vmm_blockdev.c:158 -> __blockdev_make_request()@vmm_blockdev.c:64 でbdevに値がセットされる
b vmm_blockdev.c:87
commands
  printf "((struct vmm_request*)%p)->bdev=%p\n", r, r->bdev
  if r->bdev != 0
    cont
  else
    printf "r->bdev == 0 !!!!!!!!!!!!!!!!"
  end
end

### __blockdev_make_request()@vmm_blockdev.c:83 でエラー時 bdevが 0にされる
b vmm_blockdev.c:83
# 上書きされていたりする？チェック
set $prev_r_bdev = -1
b vmm_blockdev.c:64
commands
  if $prev_r_bdev == r->bdev && bdev == 0
    printf "************** ERROR: BAD OVERWRITE ***************\n"
  else
    set $prev_r_bdev = r->bdev
    printf "__blockdev_make_request(): ((struct vmm_request*)%p)->bdev=%p\n", r, r->bdev
    cont
  end
end


# workqueue_main
b vmm_workqueue.c:317
commands
  set $bwork = (struct blockrq_work *)((void*)work - (off_t)&((struct blockrq_work*)0)->work)
  if wq->thread->tvcpu->name[0] == 'm' && wq->thread->tvcpu->name[1] == 'm' && wq->thread->tvcpu->name[2] == 'c'
    if $bwork->is_rw && $bwork->d.rw.r.bdev == 0
      printf "************** ERROR: bdev=0 ***************\n"
      printf "*(struct blockrq_work*)%p = ", $bwork
      p *$bwork
      printf "*(struct struct vmm_request*)%p = ", $bwork->d.rw.r
      p *$bwork->d.rw.r
    else
      cont
    end
  else
    cont
  end
end

#blockrq_work_func
b vmm_blockrq.c:201 if bwork->is_rw && bwork->d.rw.r.bdev == 0
commands
    printf "************** ERROR: bdev=0 ***************\n"
end
b vmm_blockrq.c:211 if bwork->d.rw.r.bdev == 0
commands
    printf "************** ERROR: bdev=0 ***************\n"
end
b vmm_blockrq.c:221 if bwork->d.rw.r.bdev == 0
commands
    printf "************** ERROR: bdev=0 ***************\n"
end

# blockrq_rw_done
b vmm_blockrq.c:188 if $rw_done == &((struct blockdev_rw*)r.priv)->done
commands
  printf "blockrq_rw_done(): ((struct vmm_request*)%p)->bdev=%p\n", r, r->bdev
  p *r
  printf "blockrq_rw_done(): rw.done=%p\n", &((struct blockdev_rw*)r->priv)->done
  if r->type == VMM_REQUEST_READ && r->bdev == 0
    printf "************** ERROR: r->bdev broken ***************\n"
  else
    cont
  end
end

# blockrq_dequeue_work(bwork) => list_del(bwork->head)
b vmm_blockdev_complete_request if $rw_done == &((struct blockdev_rw *)r->priv)->done
commands
  printf "vmm_blockdev_complete_request(): r->completed = %p = ", r->completed
  if r->completed == vmm_completion_complete
    printf "vmm_completion_complete\n"
  end
  if r->completed == blockdev_rw_completed
    printf "blockdev_rw_completed\n"
  end
  cont
end

b vmm_blockdev.c:130
commands
  printf "vmm_blockdev_complete_request(): r->bdev = %p => NULL", r->bdev
  cont
end

#  r->completed(vmm_request *r) -> blockdev_rw_completed(r) -> vmm_completion_complete(vmm_completion* cmpl=((blockdev_rw *)r->priv)->done) -> __vmm_waitqueue_wakefirst(&cmpl->wq) -> __vmm_waitqueue_wake(wq, vcpu)
b 'core/block/vmm_blockdev.c':blockdev_rw_completed if $rw_done == &((struct blockdev_rw *)req->priv)->done
commands
  printf "WAKEUP requester ===> *(struct completion*)%p = ", $rw_done
  p *$rw_done
  set $off = (off_t)&((struct vmm_vcpu*)0)->wq_head
  dlist $rw_done->wq.vcpu_list $off "p ((struct vmm_vcpu*)%p)->name"
  cont
end
b vmm_completion_complete if $rw_done == cmpl
commands
  cont
end

#  __blockdev_done_request((vmm_request_queue *)r->bdev->rq)
    b vmm_completion.c:89 if cmpl == $rw_done
    commands
      printf "!!!! &rw.done=0x%08x rw.done=", cmpl
      p/x *cmpl
      stat_mterm
      cont
    end

    b vmm_completion.c:91 if cmpl == $rw_done
    commands
      printf "__vmm_waitqueue_wakefirst(&cmpl->wq): rc=%d\n", rc
      stat_mterm
      cont
    end

b vmm_blockrq.c:83
commands
  printf "blockrq_queue_rw(): bowrk->is_rw = TRUE: wq_pending_list ... \n"
  set $off = (off_t)&((struct blockrq_work*)0)->head
  dlist brq->wq_pending_list $off "p *((struct blockrq_work*)%p)->d.rw.r"
  printf "blockrq_queue_rw(): (struct vmm_request*)bwork->d.rw.r @brq->wq->work_list ###\n"
  set $off_bw = (off_t)&((struct blockrq_work*)0)->work.head
  dlist brq->wq->work_list $off_bw "p *((struct blockrq_work*)%p)->d.rw.r"
  cont
end

b vmm_blockrq.c:115
commands
  printf "blockrq_queue_work(): bowrk->is_rw = FALSE: wq_pending_list ... \n"
  set $off = (off_t)&((struct blockrq_work*)0)->head
  dlist brq->wq_pending_list $off "p *((struct blockrq_work*)%p)->d.rw.r"
  printf "blockrq_queue_rw(): (struct vmm_request*)bwork->d.rw.r @brq->wq->work_list ###\n"
  set $off_bw = (off_t)&((struct blockrq_work*)0)->work.head
  dlist brq->wq->work_list $off_bw "p *((struct blockrq_work*)%p)->d.rw.r"
  cont
end

b vmm_scheduler.c:531
commands
  if vcpu->name[0] == 'm' && vcpu->name[1] == 'm' && vcpu->name[2] == 'c'
    printf "vmm_scheduler_switch(schedp, schedp->irq_regs): "
    p schedp->current_vcpu->name
    printf " -> "
    p vcpu->name
  else
    cont
  end
end


# ---- disable all break points until `XVisor# vfs guest_load` command input.
disable

# trap XVisor# vfs guest_load guest0 0x400a0000 /loader.bin
b cmd_vfs_load
commands
  enable
  cont
end

# set scheduler-locking on