From patchwork Mon Nov 11 10:37:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Privoznik X-Patchwork-Id: 11236751 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8B0611390 for ; Mon, 11 Nov 2019 10:39:48 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5F95620856 for ; Mon, 11 Nov 2019 10:39:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="OyGmLsom" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5F95620856 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Received: from localhost ([::1]:50688 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iU76p-0001sE-Ey for patchwork-qemu-devel@patchwork.kernel.org; Mon, 11 Nov 2019 05:39:47 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:50659) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iU76E-0000vr-8D for qemu-devel@nongnu.org; Mon, 11 Nov 2019 05:39:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iU76B-0003jY-W7 for qemu-devel@nongnu.org; Mon, 11 Nov 2019 05:39:09 -0500 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:56585 helo=us-smtp-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iU76B-0003i5-L7 for qemu-devel@nongnu.org; Mon, 11 Nov 2019 05:39:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1573468747; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=plhy1uZkekv8VMNLA+nPZe58rkKQkQ/37jG9PB7wxcI=; b=OyGmLsomMelP1oEzY4G71AvxbMSe1ADCZWJENpbfEobLvwHWjoyuISe+jPHYyxkqkjXr5Q dktHrqOgnT3/cObtC3ILQn4qUhpAunynoGCzHiKZgXv7TGtSsAK1DJGneRiQ4UKYR+06II rCfuAcIdj3now4wvXTqY5zDhIGEylXo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-386-Z8WlT_zjM0uQGtESJw46PA-1; Mon, 11 Nov 2019 05:38:00 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 96786107ACC4 for ; Mon, 11 Nov 2019 10:37:59 +0000 (UTC) Received: from moe.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA84C5C883; Mon, 11 Nov 2019 10:37:58 +0000 (UTC) From: Michal Privoznik To: qemu-devel@nongnu.org Subject: [PATCH 1/2] hw/vfio/pci: Fix double free of migration_blocker Date: Mon, 11 Nov 2019 11:37:41 +0100 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: Z8WlT_zjM0uQGtESJw46PA-1 X-Mimecast-Spam-Score: 0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 205.139.110.120 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alex.williamson@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" When user tries to hotplug a VFIO device, but the operation fails somewhere in the middle (in my testing it failed because of RLIMIT_MEMLOCK forbidding more memory allocation), then a double free occurs. In vfio_realize() the vdev->migration_blocker is allocated, then something goes wrong which causes control to jump onto 'error' label where the error is freed. But the pointer is left pointing to invalid memory. Later, when vfio_instance_finalize() is called, the memory is freed again. In my testing the second hunk was sufficient to fix the bug, but I figured the first hunk doesn't hurt either. ==169952== Invalid read of size 8 ==169952== at 0xA47DCD: error_free (error.c:266) ==169952== by 0x4E0A18: vfio_instance_finalize (pci.c:3040) ==169952== by 0x8DF74C: object_deinit (object.c:606) ==169952== by 0x8DF7BE: object_finalize (object.c:620) ==169952== by 0x8E0757: object_unref (object.c:1074) ==169952== by 0x45079C: memory_region_unref (memory.c:1779) ==169952== by 0x45376B: do_address_space_destroy (memory.c:2793) ==169952== by 0xA5C600: call_rcu_thread (rcu.c:283) ==169952== by 0xA427CB: qemu_thread_start (qemu-thread-posix.c:519) ==169952== by 0x80A8457: start_thread (in /lib64/libpthread-2.29.so) ==169952== by 0x81C96EE: clone (in /lib64/libc-2.29.so) ==169952== Address 0x143137e0 is 0 bytes inside a block of size 48 free'd ==169952== at 0x4A342BB: free (vg_replace_malloc.c:530) ==169952== by 0xA47E05: error_free (error.c:270) ==169952== by 0x4E0945: vfio_realize (pci.c:3025) ==169952== by 0x76A4FF: pci_qdev_realize (pci.c:2099) ==169952== by 0x689B9A: device_set_realized (qdev.c:876) ==169952== by 0x8E2C80: property_set_bool (object.c:2080) ==169952== by 0x8E0EF6: object_property_set (object.c:1272) ==169952== by 0x8E3FC8: object_property_set_qobject (qom-qobject.c:26) ==169952== by 0x8E11DB: object_property_set_bool (object.c:1338) ==169952== by 0x5E7BDD: qdev_device_add (qdev-monitor.c:673) ==169952== by 0x5E81E5: qmp_device_add (qdev-monitor.c:798) ==169952== by 0x9E18A8: do_qmp_dispatch (qmp-dispatch.c:132) ==169952== Block was alloc'd at ==169952== at 0x4A35476: calloc (vg_replace_malloc.c:752) ==169952== by 0x51B1158: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.6) ==169952== by 0xA47357: error_setv (error.c:61) ==169952== by 0xA475D9: error_setg_internal (error.c:97) ==169952== by 0x4DF8C2: vfio_realize (pci.c:2737) ==169952== by 0x76A4FF: pci_qdev_realize (pci.c:2099) ==169952== by 0x689B9A: device_set_realized (qdev.c:876) ==169952== by 0x8E2C80: property_set_bool (object.c:2080) ==169952== by 0x8E0EF6: object_property_set (object.c:1272) ==169952== by 0x8E3FC8: object_property_set_qobject (qom-qobject.c:26) ==169952== by 0x8E11DB: object_property_set_bool (object.c:1338) ==169952== by 0x5E7BDD: qdev_device_add (qdev-monitor.c:673) Signed-off-by: Michal Privoznik Reviewed-by: Cornelia Huck --- hw/vfio/pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index e6569a7968..9c165995df 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -2740,6 +2740,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) if (err) { error_propagate(errp, err); error_free(vdev->migration_blocker); + vdev->migration_blocker = NULL; return; } } @@ -3023,6 +3024,7 @@ error: if (vdev->migration_blocker) { migrate_del_blocker(vdev->migration_blocker); error_free(vdev->migration_blocker); + vdev->migration_blocker = NULL; } }