[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxc: fix memory leak in migration v2

On 24/07/2015 23:40, Wei Liu wrote:
> Originally there was only one counter to keep track of pages. It was
> used erroneously to keep track of how many pages were mapped and how
> many pages needed to be send. In the end munmap(2) always has 0 as the
> length argument, which resulted in leaking the mapping.
> This problem is discovered on 32bit toolstack because 32bit application
> has notably smaller address space. In fact this bug affects 64bit
> toolstack too.
> Use a separate counter to keep track of how many pages to send to solve
> this issue.
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Yikes - good catch.  (This logic definitely worked at first, because I
had debugging which confirmed as much.  It must have gotten broken in
the 18 months of minor tweaks since it was originally written).

I believe the patch is correct.  However, can I recommend instead having
"nr_pages_mapped" which is set after the call to xc_map_foreign_bulk(). 
This will make a shorter and more-obviously correct patch.

Valgrind doesn't track mmap()/munmap() calls because of their typical
use for shared libraries.  However, it turns out that the
VALGRIND_{MALLOC,FREE}LIKE_BLOCK client requests can be used to mark
arbitrary blocks of memory as tracked by the standard memcheck

For 4.7 (which happens to coincide with the splitting up of libxc), I
recommend introducing xc_unmap() and requiring that all uses of
xc_map_$FOO() use it.  This way, the client requests can be present in
the library, and all mappings of foreign memory can be tracked by
valgrind in debug builds.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.