QEMU vhost-user shutdown suffers from use after free (missing clean shutdown)
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
qemu (Ubuntu) |
Fix Released
|
Medium
|
Unassigned | ||
Xenial |
Fix Released
|
Medium
|
Rafael David Tinoco |
Bug Description
[Impact]
* vhost-user resources aren't cleaned on QEMU shutdown
* this can lead to memory leak (specially bad if hugepages)
* QEMU bad assertion blocks the cleanup logic
[Test Case]
* to use QEMU with vhost-user and stress test the shutdown
* eventually the faulty logic (race on the variable "name") will happen
[Regression Potential]
* based on upstream code
* based on core dump analysis
* could make qemu vhost-user virtio nic shutdown even worse
[Other Info]
* Check initial case description:
# BUG Description after dump analysis
- The logic net_cleanup calls the vhost_net_stop.
- This last one iterates over all vhost networks to stop one by one.
- Idea behind is to cleanly do the virtqueue stop, releasing resources.
- In order to stop the virtqueue, vhost has to get the vring base address
(by sending a msg of VHOST_USER_
- the char device would read from the socket the base address.
- if it reads nothing, the qemu tcp channel driver would disconnect the socket.
- when the socket is disconnected, vhost_user stops all the queues to that vhost_user socket.
From the dump:
By disconnecting charnet2 device we reach the error. Since the char device has already been disconnected, the vhost_user_stop tries to stop all queues but it accidentally treats all of them the same (and charnet4 is a TAP device, not a VHOST USER).
#### Logic Error:
Here is the charnet2 data at the time of the error:
Name : filename (from CharDriverState)
Details:
Default:
Decimal:
Hex:0x556a934b0a90
Binary:
Octal:025265223
When it realizes the connection is gone it creates an event:
qemu_chr_
Which will call:
net_vhost_
This last function finds all NetClientState using a pointer called "name".
The event was originated the device charnet2 and the event callback is running using charnet4, which explains why the bad decision (assert) was made (trying to assert if a TAP device is a VHOST_USER one).
#### Possible Fix
There is already a commit upstream that might address this:
commit c1bf3531aecf4a0
Author: Marc-André Lureau <email address hidden> 2016-02-23 18:10:49
Committer: Michael S. Tsirkin <email address hidden> 2016-03-11 14:59:12
Branches: master, origin/HEAD, origin/master, origin/stable-2.10, origin/stable-2.6, origin/stable-2.7, origin/stable-2.8, origin/stable-2.9
vhost-user: fix use after free
"name" is freed after visiting options, instead use the first NetClientState
name. Adds a few assert() for clarifying and checking some impossible states.
CVE References
tags: | added: sts |
tags: | removed: verification-needed verification-needed-xenial |
tags: | removed: sts-sponsor |
All the other threads were shutting down after VM_EXIT
#0 __lll_lock_wait () at ../nptl/ sysdeps/ unix/sysv/ linux/x86_ 64/lowlevellock .S:135 64-linux- gnu/libpthread. so.0 mutex_cond_ lock (mutex= 0x556a92b60800 <qemu_global_ mutex>) pthread_ mutex_lock. c:79 cond_wait@ @GLIBC_ 2.3.2 () sysdeps/ unix/sysv/ linux/x86_ 64/pthread_ cond_wait. S:259 mutex@entry= 0x556a92b60800 <qemu_global_ mutex>) qemu-KH8VkI/ qemu-2. 5+dfsg/ util/qemu- thread- posix.c: 132 wait_io_ event (cpu=<optimized out>) qemu-KH8VkI/ qemu-2. 5+dfsg/ cpus.c: 1016
#1 0x00007fe48e71b3f8 in _L_cond_lock_886 () from /lib/x86_
#2 0x00007fe48e71b164 in __pthread_
at ../nptl/
#3 0x00007fe48e715494 in pthread_
at ../nptl/
#4 0x0000556a925bd549 in qemu_cond_wait (cond=<optimized out>,
mutex=
at /build/
#5 0x0000556a922898db in qemu_kvm_
at /build/
The failing thread has a buggy assert:
(gdb) bt sysdeps/ unix/sysv/ linux/raise. c:56 0x7fe48e4c4018 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion@ entry=0x556a926 552a8 "ncs[i]->info->type == NET_CLIENT_ OPTIONS_ KIND_VHOST_ USER", file=file@ entry=0x556a926 551c8 "/build/ qemu-KH8VkI/ qemu-2. 5+dfsg/ net/vhost- user.c" , line=line@entry=50, function@ entry=0x556a926 55440 <__PRETTY_ FUNCTION_ _.31032> "vhost_user_stop") at assert.c:92 assertion@ entry=0x556a926 552a8 "ncs[i]->info->type == NET_CLIENT_ OPTIONS_ KIND_VHOST_ USER", file=file@ entry=0x556a926 551c8 "/build/ qemu-KH8VkI/ qemu-2. 5+dfsg/ net/vhost- user.c" , line=line@entry=50, function@ entry=0x556a926 55440 <__PRETTY_ FUNCTION_ _.31032> "vhost_user_stop") qemu-KH8VkI/ qemu-2. 5+dfsg/ net/vhost- user.c: 50 user_event (opaque= 0x556a934b15c0, event=5) qemu-KH8VkI/ qemu-2. 5+dfsg/ net/vhost- user.c: 192 0900) qemu-KH8VkI/ qemu-2. 5+dfsg/ qemu-char. c:2873 0900, buf=<optimized out>, qemu-KH8VkI/ qemu-2. 5+dfsg/ qemu-char. c:2920 fe_read_ all (s=s@entry= 0x556a934b0900, buf@entry= 0x7ffe4d074bf0 "\v", len=len@entry=12) qemu-KH8VkI/ qemu-2. 5+dfsg/ qemu-char. c:239 entry=0x7ffe4d0 74bf0, dev=0x556a934dd6d0) qemu-KH8VkI/ qemu-2. 5+dfsg/ hw/virtio/ vhost-user. c:122 get_vring_ base (dev=0x556a934d d6d0, ring=0x7ffe4d07 4d30) qemu-KH8VkI/ qemu-2. 5+dfsg/ hw/virtio/ vhost-user. c:366 _stop (dev=dev@ entry=0x556a934 dd6d0, vdev@entry= 0x556a93bca458, vq=0x556a934dd808, idx=0) qemu-KH8VkI/ qemu-2. 5+dfsg/ hw/virtio/ vhost.c: 895 entry=0x556a934 dd6d0, vdev@entry= 0x556a93bca4. ..
#0 0x00007fe48e376c37 in __GI_raise (sig=sig@entry=6) at ../nptl/
#1 0x00007fe48e37a028 in __GI_abort () at abort.c:89
#2 0x00007fe48e36fbf6 in __assert_fail_base (
fmt=
assertion=
function=
#3 0x00007fe48e36fca2 in __GI___assert_fail (
assertion=
function=
at assert.c:101
#4 0x0000556a924f07e1 in vhost_user_stop (queues=<optimized out>, ncs=<optimized out>)
at /build/
#5 0x0000556a924f089a in net_vhost_
at /build/
#6 0x0000556a9237fb4f in tcp_chr_disconnect (chr=0x556a934b
at /build/
#7 0x0000556a9237fc09 in tcp_chr_sync_read (chr=0x556a934b
len=<optimized out>) at /build/
#8 0x0000556a923814cd in qemu_chr_
buf=
at /build/
#9 0x0000556a922ecbf8 in vhost_user_read (msg=msg@
at /build/
#10 0x0000556a922ed00b in vhost_user_
at /build/
#11 0x0000556a922e8ed0 in vhost_virtqueue
vdev=
at /build/
#12 0x0000556a922eb824 in vhost_dev_stop (hdev=hdev@
vdev=