[Bug 1743637] Re: QEMU vhost-user shutdown suffers from use after free (missing clean shutdown)

[Rafael David Tinoco]

I have received some crash files from the lp test package I provided in
the PPA. Waiting for them in order to check if it is the same issue or a
different one. Will proceed with SRU after that has been clarified.

** Changed in: qemu (Ubuntu Xenial)
       Status: Fix Committed => In Progress

-- 
You received this bug notification because you are a member of Ubuntu
Sponsors Team, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/1743637

Title:
  QEMU vhost-user shutdown suffers from use after free (missing clean
  shutdown)

Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Xenial:
  In Progress

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_GET_VERING_BASE)
  - 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:0x556a934b0a90 "disconnected:unix:/run/openvswitch/vhostuser-vcic"
  Default:0x556a934b0a90 "disconnected:unix:/run/openvswitch/vhostuser-vcic"
  Decimal:93916226062992
  Hex:0x556a934b0a90
  Binary:10101010110101010010011010010110000101010010000
  Octal:02526522322605220

  When it realizes the connection is gone it creates an event:

  qemu_chr_be_event(chr, CHR_EVENT_CLOSED);

  Which will call:

  net_vhost_user_event

  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 c1bf3531aecf4a0ba25bb150dd5fe21edf406c88
  Author: Marc-André Lureau <marcandre.lureau at redhat.com> 2016-02-23 18:10:49
  Committer: Michael S. Tsirkin <mst at redhat.com> 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.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1743637/+subscriptions