From: Ian Campbell Date: Thu, 20 Nov 2014 15:48:47 +0000 (+0000) Subject: libxc: don't leak buffer containing the uncompressed PV kernel X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~4061 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=8f4023dd7d77de7b2c1af77e86637202a33f948a;p=xen.git libxc: don't leak buffer containing the uncompressed PV kernel The libxc xc_dom_* infrastructure uses a very simple malloc memory pool which is freed by xc_dom_release. However the various xc_try_*_decode routines (other than the gzip one) just use plain malloc/realloc and therefore the buffer ends up leaked. The memory pool currently supports mmap'd buffers as well as a directly allocated buffers, however the try decode routines make use of realloc and do not fit well into this model. Introduce a concept of an external memory block to the memory pool and provide an interface to register such memory. The mmap_ptr and mmap_len fields of the memblock tracking struct lose their mmap_ prefix since they are now also used for external memory blocks. We are only seeing this now because the gzip decoder doesn't leak and it's only relatively recently that kernels in the wild have switched to better compression. This is https://bugs.debian.org/767295 Reported by: Gedalya Signed-off-by: Ian Campbell Reviewed-by: Wei Liu --- diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h index 6ae6a9f97c..07d7224b76 100644 --- a/tools/libxc/include/xc_dom.h +++ b/tools/libxc/include/xc_dom.h @@ -33,8 +33,13 @@ struct xc_dom_seg { struct xc_dom_mem { struct xc_dom_mem *next; - void *mmap_ptr; - size_t mmap_len; + void *ptr; + enum { + XC_DOM_MEM_TYPE_MALLOC_INTERNAL, + XC_DOM_MEM_TYPE_MALLOC_EXTERNAL, + XC_DOM_MEM_TYPE_MMAP, + } type; + size_t len; unsigned char memory[0]; }; @@ -298,6 +303,7 @@ void xc_dom_log_memory_footprint(struct xc_dom_image *dom); /* --- simple memory pool ------------------------------------------ */ void *xc_dom_malloc(struct xc_dom_image *dom, size_t size); +int xc_dom_register_external(struct xc_dom_image *dom, void *ptr, size_t size); void *xc_dom_malloc_page_aligned(struct xc_dom_image *dom, size_t size); void *xc_dom_malloc_filemap(struct xc_dom_image *dom, const char *filename, size_t * size, diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c index 2225699b8a..964ebdc64a 100644 --- a/tools/libxc/xc_dom_bzimageloader.c +++ b/tools/libxc/xc_dom_bzimageloader.c @@ -161,6 +161,13 @@ static int xc_try_bzip2_decode( total = (((uint64_t)stream.total_out_hi32) << 32) | stream.total_out_lo32; + if ( xc_dom_register_external(dom, out_buf, total) ) + { + DOMPRINTF("BZIP2: Error registering stream output"); + free(out_buf); + goto bzip2_cleanup; + } + DOMPRINTF("%s: BZIP2 decompress OK, 0x%zx -> 0x%lx", __FUNCTION__, *size, (long unsigned int) total); @@ -305,6 +312,13 @@ static int _xc_try_lzma_decode( } } + if ( xc_dom_register_external(dom, out_buf, stream->total_out) ) + { + DOMPRINTF("%s: Error registering stream output", what); + free(out_buf); + goto lzma_cleanup; + } + DOMPRINTF("%s: %s decompress OK, 0x%zx -> 0x%zx", __FUNCTION__, what, *size, (size_t)stream->total_out); @@ -464,7 +478,13 @@ static int xc_try_lzo1x_decode( dst_len = lzo_read_32(cur); if ( !dst_len ) + { + msg = "Error registering stream output"; + if ( xc_dom_register_external(dom, out_buf, out_len) ) + break; + return 0; + } if ( dst_len > LZOP_MAX_BLOCK_SIZE ) { diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c index baa62a197e..ecbf981988 100644 --- a/tools/libxc/xc_dom_core.c +++ b/tools/libxc/xc_dom_core.c @@ -132,6 +132,7 @@ void *xc_dom_malloc(struct xc_dom_image *dom, size_t size) return NULL; } memset(block, 0, sizeof(*block) + size); + block->type = XC_DOM_MEM_TYPE_MALLOC_INTERNAL; block->next = dom->memblocks; dom->memblocks = block; dom->alloc_malloc += sizeof(*block) + size; @@ -151,23 +152,45 @@ void *xc_dom_malloc_page_aligned(struct xc_dom_image *dom, size_t size) return NULL; } memset(block, 0, sizeof(*block)); - block->mmap_len = size; - block->mmap_ptr = mmap(NULL, block->mmap_len, - PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, - -1, 0); - if ( block->mmap_ptr == MAP_FAILED ) + block->len = size; + block->ptr = mmap(NULL, block->len, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, + -1, 0); + if ( block->ptr == MAP_FAILED ) { DOMPRINTF("%s: mmap failed", __FUNCTION__); free(block); return NULL; } + block->type = XC_DOM_MEM_TYPE_MMAP; block->next = dom->memblocks; dom->memblocks = block; dom->alloc_malloc += sizeof(*block); - dom->alloc_mem_map += block->mmap_len; + dom->alloc_mem_map += block->len; if ( size > (100 * 1024) ) print_mem(dom, __FUNCTION__, size); - return block->mmap_ptr; + return block->ptr; +} + +int xc_dom_register_external(struct xc_dom_image *dom, void *ptr, size_t size) +{ + struct xc_dom_mem *block; + + block = malloc(sizeof(*block)); + if ( block == NULL ) + { + DOMPRINTF("%s: allocation failed", __FUNCTION__); + return -1; + } + memset(block, 0, sizeof(*block)); + block->ptr = ptr; + block->len = size; + block->type = XC_DOM_MEM_TYPE_MALLOC_EXTERNAL; + block->next = dom->memblocks; + dom->memblocks = block; + dom->alloc_malloc += sizeof(*block); + dom->alloc_mem_map += block->len; + return 0; } void *xc_dom_malloc_filemap(struct xc_dom_image *dom, @@ -212,24 +235,25 @@ void *xc_dom_malloc_filemap(struct xc_dom_image *dom, } memset(block, 0, sizeof(*block)); - block->mmap_len = *size; - block->mmap_ptr = mmap(NULL, block->mmap_len, PROT_READ, + block->len = *size; + block->ptr = mmap(NULL, block->len, PROT_READ, MAP_SHARED, fd, 0); - if ( block->mmap_ptr == MAP_FAILED ) { + if ( block->ptr == MAP_FAILED ) { xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, "failed to mmap file: %s", strerror(errno)); goto err; } + block->type = XC_DOM_MEM_TYPE_MMAP; block->next = dom->memblocks; dom->memblocks = block; dom->alloc_malloc += sizeof(*block); - dom->alloc_file_map += block->mmap_len; + dom->alloc_file_map += block->len; close(fd); if ( *size > (100 * 1024) ) print_mem(dom, __FUNCTION__, *size); - return block->mmap_ptr; + return block->ptr; err: if ( fd != -1 ) @@ -246,8 +270,17 @@ static void xc_dom_free_all(struct xc_dom_image *dom) while ( (block = dom->memblocks) != NULL ) { dom->memblocks = block->next; - if ( block->mmap_ptr ) - munmap(block->mmap_ptr, block->mmap_len); + switch ( block->type ) + { + case XC_DOM_MEM_TYPE_MALLOC_INTERNAL: + break; + case XC_DOM_MEM_TYPE_MALLOC_EXTERNAL: + free(block->ptr); + break; + case XC_DOM_MEM_TYPE_MMAP: + munmap(block->ptr, block->len); + break; + } free(block); } } diff --git a/tools/libxc/xc_dom_decompress_lz4.c b/tools/libxc/xc_dom_decompress_lz4.c index 490ec56206..b6a33f27a8 100644 --- a/tools/libxc/xc_dom_decompress_lz4.c +++ b/tools/libxc/xc_dom_decompress_lz4.c @@ -103,6 +103,11 @@ int xc_try_lz4_decode( if (size == 0) { + if ( xc_dom_register_external(dom, output, out_len) ) + { + msg = "Error registering stream output"; + goto exit_2; + } *blob = output; *psize = out_len; return 0;