From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46611) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEyrs-0004PE-Ft for qemu-devel@nongnu.org; Tue, 14 Jul 2015 07:59:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZEyrl-0007nN-Pt for qemu-devel@nongnu.org; Tue, 14 Jul 2015 07:59:24 -0400 Received: from mail-wg0-x236.google.com ([2a00:1450:400c:c00::236]:34396) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEyrl-0007nF-Ip for qemu-devel@nongnu.org; Tue, 14 Jul 2015 07:59:17 -0400 Received: by wgkl9 with SMTP id l9so6893169wgk.1 for ; Tue, 14 Jul 2015 04:59:16 -0700 (PDT) Sender: Paolo Bonzini From: Paolo Bonzini Date: Tue, 14 Jul 2015 13:59:12 +0200 Message-Id: <1436875152-32758-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH] 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 Cc: peter.maydell@linaro.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. 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