From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59513) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMZPJ-0007oC-Ae for qemu-devel@nongnu.org; Wed, 06 Dec 2017 08:06:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMZPE-0001C6-BV for qemu-devel@nongnu.org; Wed, 06 Dec 2017 08:06:37 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:46952) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eMZPD-00018q-Rt for qemu-devel@nongnu.org; Wed, 06 Dec 2017 08:06:32 -0500 From: Ilya Maximets Date: Wed, 06 Dec 2017 16:06:18 +0300 Message-id: <1512565578-12078-1-git-send-email-i.maximets@samsung.com> References: Subject: [Qemu-devel] [PATCH] vhost: fix crash on virtio_error while device stop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S . Tsirkin" Cc: qemu-devel@nongnu.org, Marc-Andre Lureau , Heetae Ahn , Maxime Coquelin , Ilya Maximets In case virtio error occured after vhost_dev_close(), qemu will crash in nested cleanup while checking IOMMU flag because dev->vdev already set to zero and resources are already freed. Example: Program received signal SIGSEGV, Segmentation fault. vhost_virtqueue_stop at hw/virtio/vhost.c:1155 #0 vhost_virtqueue_stop at hw/virtio/vhost.c:1155 #1 vhost_dev_stop at hw/virtio/vhost.c:1594 #2 vhost_net_stop_one at hw/net/vhost_net.c:289 #3 vhost_net_stop at hw/net/vhost_net.c:368 Nested call to vhost_net_stop(). First time was at #14. #4 virtio_net_vhost_status at hw/net/virtio-net.c:180 #5 virtio_net_set_status (status=79) at hw/net/virtio-net.c:254 #6 virtio_set_status at hw/virtio/virtio.c:1146 #7 virtio_error at hw/virtio/virtio.c:2455 virtqueue_get_head() failed here. #8 virtqueue_get_head at hw/virtio/virtio.c:543 #9 virtqueue_drop_all at hw/virtio/virtio.c:984 #10 virtio_net_drop_tx_queue_data at hw/net/virtio-net.c:240 #11 virtio_bus_set_host_notifier at hw/virtio/virtio-bus.c:297 #12 vhost_dev_disable_notifiers at hw/virtio/vhost.c:1431 vhost_dev_stop() was executed here. dev->vdev == NULL now. #13 vhost_net_stop_one at hw/net/vhost_net.c:290 #14 vhost_net_stop at hw/net/vhost_net.c:368 #15 virtio_net_vhost_status at hw/net/virtio-net.c:180 #16 virtio_net_set_status (status=15) at hw/net/virtio-net.c:254 #17 qmp_set_link ("hostnet0", up=false) at net/net.c:1430 #18 chr_closed_bh at net/vhost-user.c:214 #19 aio_bh_call at util/async.c:90 #20 aio_bh_poll at util/async.c:118 #21 aio_dispatch at util/aio-posix.c:429 #22 aio_ctx_dispatch at util/async.c:261 #23 g_main_context_dispatch #24 glib_pollfds_poll at util/main-loop.c:213 #25 os_host_main_loop_wait at util/main-loop.c:261 #26 main_loop_wait at util/main-loop.c:515 #27 main_loop () at vl.c:1917 #28 main at vl.c:4795 Above backtrace captured from qemu crash on vhost disconnect while virtio driver in guest was in failed state. We can just add checking for 'vdev' in 'vhost_dev_has_iommu()' but it will assert further trying to free already freed ioeventfds. The real problem is that we're allowing nested calls to 'vhost_net_stop'. This patch is aimed to forbid nested calls to 'vhost_net_stop' to avoid any possible double frees and segmentation faults doue to using of already freed resources by setting 'vhost_started' flag to zero prior to 'vhost_net_stop' call. Signed-off-by: Ilya Maximets --- This issue was already addressed more than a year ago by the following patch: https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg06732.html but it was dropped without review due to not yet implemented re-connection of vhost-user. Re-connection implementation lately fixed most of the nested calls, but few of them still exists. For example, above backtrace captured after 'virtqueue_get_head()' failure on vhost-user disconnection. hw/net/virtio-net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 38674b0..4d95a18 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -177,8 +177,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) n->vhost_started = 0; } } else { - vhost_net_stop(vdev, n->nic->ncs, queues); n->vhost_started = 0; + vhost_net_stop(vdev, n->nic->ncs, queues); } } -- 2.7.4