Commit c6742b14 authored by Paolo Bonzini's avatar Paolo Bonzini
Browse files

memory: fix refcount leak in memory_region_present 



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: default avatarPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 24b41d66

memory.c

+28 −16
Original line number Diff line number Diff line
@@ -1887,22 +1887,15 @@ 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,

/* 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 };
@@ -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;

    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);

out:

    }

    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;