From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40552) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFnVC-0001gE-92 for qemu-devel@nongnu.org; Thu, 16 Jul 2015 14:03:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFnV9-0002ws-QI for qemu-devel@nongnu.org; Thu, 16 Jul 2015 14:03:22 -0400 Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:33129) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFnV9-0002wZ-KE for qemu-devel@nongnu.org; Thu, 16 Jul 2015 14:03:19 -0400 Received: by widic2 with SMTP id ic2so21698909wid.0 for ; Thu, 16 Jul 2015 11:03:19 -0700 (PDT) Received: from donizetti.redhat.com (host231-210-dynamic.21-79-r.retail.telecomitalia.it. [79.21.210.231]) by smtp.gmail.com with ESMTPSA id l2sm4422399wib.11.2015.07.16.11.03.17 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Jul 2015 11:03:18 -0700 (PDT) Sender: Paolo Bonzini From: Paolo Bonzini Date: Thu, 16 Jul 2015 20:02:56 +0200 Message-Id: <1437069778-8954-7-git-send-email-pbonzini@redhat.com> In-Reply-To: <1437069778-8954-1-git-send-email-pbonzini@redhat.com> References: <1437069778-8954-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PULL 6/8] memory: fix refcount leak in memory_region_present List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org memory_region_present() leaks a reference to a MemoryRegion in the case "mr == container". While fixing it, avoid reference counting altogether for memory_region_present(), by using RCU only. The return value could in principle be already invalid immediately after memory_region_present returns, but presumably the caller knows that and it's using memory_region_present to probe for devices that are unpluggable, or something like that. The RCU critical section is needed anyway, because it protects as->current_map. Reported-by: Peter Maydell Signed-off-by: Paolo Bonzini --- memory.c | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/memory.c b/memory.c index 5a0cc66..0acebb1 100644 --- a/memory.c +++ b/memory.c @@ -1887,23 +1887,16 @@ static FlatRange *flatview_lookup(FlatView *view, AddrRange addr) sizeof(FlatRange), cmp_flatrange_addr); } -bool memory_region_present(MemoryRegion *container, hwaddr addr) -{ - MemoryRegion *mr = memory_region_find(container, addr, 1).mr; - if (!mr || (mr == container)) { - return false; - } - memory_region_unref(mr); - return true; -} - bool memory_region_is_mapped(MemoryRegion *mr) { return mr->container ? true : false; } -MemoryRegionSection memory_region_find(MemoryRegion *mr, - hwaddr addr, uint64_t size) +/* Same as memory_region_find, but it does not add a reference to the + * returned region. It must be called from an RCU critical section. + */ +static MemoryRegionSection memory_region_find_rcu(MemoryRegion *mr, + hwaddr addr, uint64_t size) { MemoryRegionSection ret = { .mr = NULL }; MemoryRegion *root; @@ -1924,11 +1917,10 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, } range = addrrange_make(int128_make64(addr), int128_make64(size)); - rcu_read_lock(); view = atomic_rcu_read(&as->current_map); fr = flatview_lookup(view, range); if (!fr) { - goto out; + return ret; } while (fr > view->ranges && addrrange_intersects(fr[-1].addr, range)) { @@ -1944,12 +1936,32 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr, ret.size = range.size; ret.offset_within_address_space = int128_get64(range.start); ret.readonly = fr->readonly; - memory_region_ref(ret.mr); -out: + return ret; +} + +MemoryRegionSection memory_region_find(MemoryRegion *mr, + hwaddr addr, uint64_t size) +{ + MemoryRegionSection ret; + rcu_read_lock(); + ret = memory_region_find_rcu(mr, addr, size); + if (ret.mr) { + memory_region_ref(ret.mr); + } rcu_read_unlock(); return ret; } +bool memory_region_present(MemoryRegion *container, hwaddr addr) +{ + MemoryRegion *mr; + + rcu_read_lock(); + mr = memory_region_find_rcu(container, addr, 1).mr; + rcu_read_unlock(); + return mr && mr != container; +} + void address_space_sync_dirty_bitmap(AddressSpace *as) { FlatView *view; -- 2.4.3