From patchwork Thu Nov 20 15:48:47 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Campbell X-Patchwork-Id: 41244 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ee0-f71.google.com (mail-ee0-f71.google.com [74.125.83.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 0F74F23C27 for ; Thu, 20 Nov 2014 15:50:42 +0000 (UTC) Received: by mail-ee0-f71.google.com with SMTP id c13sf2252201eek.2 for ; Thu, 20 Nov 2014 07:50:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:date:message-id :mime-version:cc:subject:precedence:list-id:list-unsubscribe :list-post:list-help:list-subscribe:content-type :content-transfer-encoding:sender:errors-to:x-original-sender :x-original-authentication-results:mailing-list:list-archive; bh=1ZXzDXkS0X/GgHZQfVI7Z4AEtcYGwP++iJ7o3QkWf0s=; b=hr6N4M5rlbkLRom2MV4DcnuEItZu+jBrtQMo340bei9ddlND/z7g5ANknwBlpgvQW+ o/xOncQXKO2RueXkwNskzW+4aazcxh8WU81b9Y1o1GCnUJH2AO1Q0F86NAZn4XlXtELx 2j69C3pluD//6oF8Wj3LtN89a0E3qhhP8s1UxM5zxc3HTM7eG/hZmJbXQSgx4FZhycdP H3IqBdNrY+9rJQJ6LWRXDhL/wjd2ggXMgVggxj76Q7BQf20bmeqlbnGhuwAwOAo7z18V 0h8kOgIqm1vQpJoombE8qam8sQ7IeAKDZqJoJ9b13ogezm9lCcmIvUlhRYYo9DT2u3/y BKZw== X-Gm-Message-State: ALoCoQncqNVlWDOhz+IwAbvPI6iD998SrL4I5OLWUIKXd2FM/sz8rOunvjc2w34Rk0GXaLb+ICqY X-Received: by 10.112.147.131 with SMTP id tk3mr12414946lbb.2.1416498640882; Thu, 20 Nov 2014 07:50:40 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.8.68 with SMTP id p4ls80888laa.52.gmail; Thu, 20 Nov 2014 07:50:40 -0800 (PST) X-Received: by 10.112.201.138 with SMTP id ka10mr48851485lbc.20.1416498640562; Thu, 20 Nov 2014 07:50:40 -0800 (PST) Received: from mail-la0-f43.google.com (mail-la0-f43.google.com. [209.85.215.43]) by mx.google.com with ESMTPS id og3si2517840lbb.6.2014.11.20.07.50.40 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 20 Nov 2014 07:50:40 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.43 as permitted sender) client-ip=209.85.215.43; Received: by mail-la0-f43.google.com with SMTP id q1so2651525lam.2 for ; Thu, 20 Nov 2014 07:50:40 -0800 (PST) X-Received: by 10.112.235.196 with SMTP id uo4mr11665491lbc.66.1416498640394; Thu, 20 Nov 2014 07:50:40 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.184.201 with SMTP id ew9csp284697lbc; Thu, 20 Nov 2014 07:50:39 -0800 (PST) X-Received: by 10.52.81.129 with SMTP id a1mr6618061vdy.53.1416498638952; Thu, 20 Nov 2014 07:50:38 -0800 (PST) Received: from lists.xen.org (lists.xen.org. [50.57.142.19]) by mx.google.com with ESMTPS id m5si932063vck.101.2014.11.20.07.50.38 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Thu, 20 Nov 2014 07:50:38 -0800 (PST) Received-SPF: none (google.com: xen-devel-bounces@lists.xen.org does not designate permitted sender hosts) client-ip=50.57.142.19; Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XrTyb-0004c1-KH; Thu, 20 Nov 2014 15:48:57 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XrTyZ-0004bl-9p for xen-devel@lists.xen.org; Thu, 20 Nov 2014 15:48:55 +0000 Received: from [85.158.137.68] by server-17.bemta-3.messagelabs.com id 57/6B-11608-66D0E645; Thu, 20 Nov 2014 15:48:54 +0000 X-Env-Sender: Ian.Campbell@citrix.com X-Msg-Ref: server-10.tower-31.messagelabs.com!1416498530!12712812!1 X-Originating-IP: [66.165.176.63] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni42MyA9PiAzMDYwNDg=\n X-StarScan-Received: X-StarScan-Version: 6.12.4; banners=-,-,- X-VirusChecked: Checked Received: (qmail 9761 invoked from network); 20 Nov 2014 15:48:52 -0000 Received: from smtp02.citrix.com (HELO SMTP02.CITRIX.COM) (66.165.176.63) by server-10.tower-31.messagelabs.com with RC4-SHA encrypted SMTP; 20 Nov 2014 15:48:52 -0000 X-IronPort-AV: E=Sophos;i="5.07,424,1413244800"; d="scan'208";a="194847941" Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.80) with Microsoft SMTP Server id 14.3.181.6; Thu, 20 Nov 2014 10:48:48 -0500 Received: from cosworth.uk.xensource.com ([10.80.16.52] helo=localhost.localdomain ident=ianc) by ukmail1.uk.xensource.com with smtp (Exim 4.69) (envelope-from ) id 1XrTyS-0005kU-0G; Thu, 20 Nov 2014 15:48:49 +0000 Received: by localhost.localdomain (sSMTP sendmail emulation); Thu, 20 Nov 2014 15:48:47 +0000 From: Ian Campbell To: Date: Thu, 20 Nov 2014 15:48:47 +0000 Message-ID: <1416498527-32441-1-git-send-email-ian.campbell@citrix.com> X-Mailer: git-send-email 1.7.10.4 MIME-Version: 1.0 X-DLP: MIA1 Cc: wei.liu2@citrix.com, ian.jackson@eu.citrix.com, Ian Campbell Subject: [Xen-devel] [PATCH for-4.5 v2] libxc: don't leak buffer containing the uncompressed PV kernel X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Post: , List-Help: , List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: ian.campbell@citrix.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.43 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Archive: 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 --- v2: Correct handling in xc_try_lzo1x_decode. This is a bug fix and should go into 4.5. I have a sneaking suspicion this is going to need to backport a very long way... --- tools/libxc/include/xc_dom.h | 10 ++++-- tools/libxc/xc_dom_bzimageloader.c | 20 ++++++++++++ tools/libxc/xc_dom_core.c | 61 +++++++++++++++++++++++++++-------- tools/libxc/xc_dom_decompress_lz4.c | 5 +++ 4 files changed, 80 insertions(+), 16 deletions(-) diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h index 6ae6a9f..07d7224 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 2225699..964ebdc 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 baa62a1..ecbf981 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 490ec56..b6a33f2 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;