From nobody Sun Oct 26 01:38:39 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1516127187362199.06385228235547; Tue, 16 Jan 2018 10:26:27 -0800 (PST) Received: from localhost ([::1]:53303 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ebVw5-0004Qp-75 for importer@patchew.org; Tue, 16 Jan 2018 13:26:13 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51207) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ebVca-00053h-Bx for qemu-devel@nongnu.org; Tue, 16 Jan 2018 13:06:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ebVcS-0003lD-Nl for qemu-devel@nongnu.org; Tue, 16 Jan 2018 13:06:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36418) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ebVcS-0003kU-F0 for qemu-devel@nongnu.org; Tue, 16 Jan 2018 13:05:56 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 848D78FABC for ; Tue, 16 Jan 2018 18:05:55 +0000 (UTC) Received: from dgilbert-t530.redhat.com (ovpn-117-47.ams2.redhat.com [10.36.117.47]) by smtp.corp.redhat.com (Postfix) with ESMTP id 202A560C9C; Tue, 16 Jan 2018 18:04:53 +0000 (UTC) From: "Dr. David Alan Gilbert (git)" To: qemu-devel@nongnu.org, mst@redhat.com, imammedo@redhat.com Date: Tue, 16 Jan 2018 18:04:02 +0000 Message-Id: <20180116180408.11279-2-dgilbert@redhat.com> In-Reply-To: <20180116180408.11279-1-dgilbert@redhat.com> References: <20180116180408.11279-1-dgilbert@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 16 Jan 2018 18:05:55 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v6 1/7] vhost: Build temporary section list and deref after commit X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: maxime.coquelin@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: "Dr. David Alan Gilbert" Igor spotted that there's a race, where a region that's unref'd in a _del callback might be free'd before the set_mem_table call in the _commit callback, and thus the vhost might end up using free memory. Fix this by building a complete temporary sections list, ref'ing every section (during add and nop) and then unref'ing the whole list right at the end of commit. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Igor Mammedov --- hw/virtio/vhost.c | 73 ++++++++++++++++++++++++++++++-------------= ---- include/hw/virtio/vhost.h | 2 ++ 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index e4290ce93d..610bfe6945 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -621,6 +621,8 @@ static void vhost_begin(MemoryListener *listener) memory_listener); dev->mem_changed_end_addr =3D 0; dev->mem_changed_start_addr =3D -1; + dev->tmp_sections =3D NULL; + dev->n_tmp_sections =3D 0; } =20 static void vhost_commit(MemoryListener *listener) @@ -629,17 +631,25 @@ static void vhost_commit(MemoryListener *listener) memory_listener); hwaddr start_addr =3D 0; ram_addr_t size =3D 0; + MemoryRegionSection *old_sections; + int n_old_sections; + uint64_t log_size; int r; =20 + old_sections =3D dev->mem_sections; + n_old_sections =3D dev->n_mem_sections; + dev->mem_sections =3D dev->tmp_sections; + dev->n_mem_sections =3D dev->n_tmp_sections; + if (!dev->memory_changed) { - return; + goto out; } if (!dev->started) { - return; + goto out; } if (dev->mem_changed_start_addr > dev->mem_changed_end_addr) { - return; + goto out; } =20 if (dev->started) { @@ -656,7 +666,7 @@ static void vhost_commit(MemoryListener *listener) VHOST_OPS_DEBUG("vhost_set_mem_table failed"); } dev->memory_changed =3D false; - return; + goto out; } log_size =3D vhost_get_log_size(dev); /* We allocate an extra 4K bytes to log, @@ -675,6 +685,27 @@ static void vhost_commit(MemoryListener *listener) vhost_dev_log_resize(dev, log_size); } dev->memory_changed =3D false; + +out: + /* Deref the old list of sections, this must happen _after_ the + * vhost_set_mem_table to ensure the client isn't still using the + * section we're about to unref. + */ + while (n_old_sections--) { + memory_region_unref(old_sections[n_old_sections].mr); + } + g_free(old_sections); + return; +} + +static void vhost_add_section(struct vhost_dev *dev, + MemoryRegionSection *section) +{ + ++dev->n_tmp_sections; + dev->tmp_sections =3D g_renew(MemoryRegionSection, dev->tmp_sections, + dev->n_tmp_sections); + dev->tmp_sections[dev->n_tmp_sections - 1] =3D *section; + memory_region_ref(section->mr); } =20 static void vhost_region_add(MemoryListener *listener, @@ -687,36 +718,31 @@ static void vhost_region_add(MemoryListener *listener, return; } =20 - ++dev->n_mem_sections; - dev->mem_sections =3D g_renew(MemoryRegionSection, dev->mem_sections, - dev->n_mem_sections); - dev->mem_sections[dev->n_mem_sections - 1] =3D *section; - memory_region_ref(section->mr); + vhost_add_section(dev, section); vhost_set_memory(listener, section, true); } =20 -static void vhost_region_del(MemoryListener *listener, +static void vhost_region_nop(MemoryListener *listener, MemoryRegionSection *section) { struct vhost_dev *dev =3D container_of(listener, struct vhost_dev, memory_listener); - int i; =20 if (!vhost_section(section)) { return; } =20 - vhost_set_memory(listener, section, false); - memory_region_unref(section->mr); - for (i =3D 0; i < dev->n_mem_sections; ++i) { - if (dev->mem_sections[i].offset_within_address_space - =3D=3D section->offset_within_address_space) { - --dev->n_mem_sections; - memmove(&dev->mem_sections[i], &dev->mem_sections[i+1], - (dev->n_mem_sections - i) * sizeof(*dev->mem_sections)= ); - break; - } + vhost_add_section(dev, section); +} + +static void vhost_region_del(MemoryListener *listener, + MemoryRegionSection *section) +{ + if (!vhost_section(section)) { + return; } + + vhost_set_memory(listener, section, false); } =20 static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotl= b) @@ -783,11 +809,6 @@ static void vhost_iommu_region_del(MemoryListener *lis= tener, } } =20 -static void vhost_region_nop(MemoryListener *listener, - MemoryRegionSection *section) -{ -} - static int vhost_virtqueue_set_addr(struct vhost_dev *dev, struct vhost_virtqueue *vq, unsigned idx, bool enable_log) diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index 467dc7794b..21f3022499 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -54,6 +54,8 @@ struct vhost_dev { struct vhost_memory *mem; int n_mem_sections; MemoryRegionSection *mem_sections; + int n_tmp_sections; + MemoryRegionSection *tmp_sections; struct vhost_virtqueue *vqs; int nvqs; /* the first virtqueue which would be used by this vhost dev */ --=20 2.14.3