From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: [PATCH 4.0-testing 02/10] libxc: builder: limit maximum size of kernel/ramdisk. Date: Mon, 11 Feb 2013 13:12:45 +0000 Message-ID: <1360588373-779-2-git-send-email-ian.campbell@citrix.com> References: <1360588355.20449.34.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1360588355.20449.34.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org Cc: Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org From: Ian Jackson Allowing user supplied kernels of arbitrary sizes, especially during decompression, can swallow up dom0 memory leading to either virtual address space exhaustion in the builder process or allocation failures/OOM killing of both toolstack and unrelated processes. We disable these checks when building in a stub domain for pvgrub since this uses the guest's own memory and is isolated. Decompression of gzip compressed kernels and ramdisks has been safe since 14954:58205257517d (Xen 3.1.0 onwards). This is XSA-25 / CVE-2012-4544. Also make explicit checks for buffer overflows in various decompression routines. These were already ruled out due to other properties of the code but check them as a belt-and-braces measure. Signed-off-by: Ian Campbell Acked-by: Ian Jackson [ Includes 25589:60f09d1ab1fe for CVE-2012-2625 ] --- stubdom/grub/kexec.c | 4 ++ tools/libxc/xc_dom.h | 23 +++++++++- tools/libxc/xc_dom_bzimageloader.c | 44 ++++++++++++++++-- tools/libxc/xc_dom_core.c | 78 +++++++++++++++++++++++++++++-- tools/pygrub/src/pygrub | 55 +++++++++++++++++---- tools/python/xen/xm/messages/xen-xm.pot | 3 +- 6 files changed, 186 insertions(+), 21 deletions(-) diff --git a/stubdom/grub/kexec.c b/stubdom/grub/kexec.c index 821c71d..f3f1b7b 100644 --- a/stubdom/grub/kexec.c +++ b/stubdom/grub/kexec.c @@ -137,6 +137,10 @@ void kexec(void *kernel, long kernel_size, void *module, long module_size, char dom = xc_dom_allocate(cmdline, features); dom->allocate = kexec_allocate; + /* We are using guest owned memory, therefore no limits. */ + xc_dom_kernel_max_size(dom, 0); + xc_dom_ramdisk_max_size(dom, 0); + dom->kernel_blob = kernel; dom->kernel_size = kernel_size; diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h index 58d3f49..71ffa1e 100644 --- a/tools/libxc/xc_dom.h +++ b/tools/libxc/xc_dom.h @@ -36,6 +36,9 @@ struct xc_dom_image { void *ramdisk_blob; size_t ramdisk_size; + size_t max_kernel_size; + size_t max_ramdisk_size; + /* arguments and parameters */ char *cmdline; uint32_t f_requested[XENFEAT_NR_SUBMAPS]; @@ -158,6 +161,23 @@ void xc_dom_release_phys(struct xc_dom_image *dom); void xc_dom_release(struct xc_dom_image *dom); int xc_dom_mem_init(struct xc_dom_image *dom, unsigned int mem_mb); +/* Set this larger if you have enormous ramdisks/kernels. Note that + * you should trust all kernels not to be maliciously large (e.g. to + * exhaust all dom0 memory) if you do this (see CVE-2012-4544 / + * XSA-25). You can also set the default independently for + * ramdisks/kernels in xc_dom_allocate() or call + * xc_dom_{kernel,ramdisk}_max_size. + */ +#ifndef XC_DOM_DECOMPRESS_MAX +#define XC_DOM_DECOMPRESS_MAX (1024*1024*1024) /* 1GB */ +#endif + +int xc_dom_kernel_check_size(struct xc_dom_image *dom, size_t sz); +int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz); + +int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz); +int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz); + size_t xc_dom_check_gzip(void *blob, size_t ziplen); int xc_dom_do_gunzip(void *src, size_t srclen, void *dst, size_t dstlen); int xc_dom_try_gunzip(struct xc_dom_image *dom, void **blob, size_t * size); @@ -202,7 +222,8 @@ void xc_dom_log_memory_footprint(struct xc_dom_image *dom); void *xc_dom_malloc(struct xc_dom_image *dom, 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); + const char *filename, size_t * size, + const size_t max_size); char *xc_dom_strdup(struct xc_dom_image *dom, const char *str); /* --- alloc memory pool ------------------------------------------- */ diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c index aa5abc7..3aaf827 100644 --- a/tools/libxc/xc_dom_bzimageloader.c +++ b/tools/libxc/xc_dom_bzimageloader.c @@ -33,13 +33,19 @@ static int xc_try_bzip2_decode( char *out_buf; char *tmp_buf; int retval = -1; - int outsize; + unsigned int outsize; uint64_t total; stream.bzalloc = NULL; stream.bzfree = NULL; stream.opaque = NULL; + if ( dom->kernel_size == 0) + { + xc_dom_printf("BZIP2: Input is 0 size"); + return -1; + } + ret = BZ2_bzDecompressInit(&stream, 0, 0); if ( ret != BZ_OK ) { @@ -52,6 +58,17 @@ static int xc_try_bzip2_decode( * the input buffer to start, and we'll realloc as needed. */ outsize = dom->kernel_size; + + /* + * stream.avail_in and outsize are unsigned int, while kernel_size + * is a size_t. Check we aren't overflowing. + */ + if ( outsize != dom->kernel_size ) + { + xc_dom_printf("BZIP2: Input too large"); + goto bzip2_cleanup; + } + out_buf = malloc(outsize); if ( out_buf == NULL ) { @@ -84,13 +101,20 @@ static int xc_try_bzip2_decode( if ( stream.avail_out == 0 ) { /* Protect against output buffer overflow */ - if ( outsize > INT_MAX / 2 ) + if ( outsize > UINT_MAX / 2 ) { xc_dom_printf("BZIP2: output buffer overflow\n"); free(out_buf); goto bzip2_cleanup; } + if ( xc_dom_kernel_check_size(dom, outsize * 2) ) + { + xc_dom_printf("BZIP2: output too large"); + free(out_buf); + goto bzip2_cleanup; + } + tmp_buf = realloc(out_buf, outsize * 2); if ( tmp_buf == NULL ) { @@ -158,10 +182,15 @@ static int xc_try_lzma_decode( unsigned char *out_buf; unsigned char *tmp_buf; int retval = -1; - int outsize; + size_t outsize; const char *msg; ret = lzma_alone_decoder(&stream, 32*1024*1024); + if ( dom->kernel_size == 0) + { + xc_dom_printf("LZMA: Input is 0 size"); + return -1; + } if ( ret != LZMA_OK ) { xc_dom_printf("LZMA: Failed to init stream decoder\n"); @@ -237,13 +266,20 @@ static int xc_try_lzma_decode( if ( stream.avail_out == 0 ) { /* Protect against output buffer overflow */ - if ( outsize > INT_MAX / 2 ) + if ( outsize > SIZE_MAX / 2 ) { xc_dom_printf("LZMA: output buffer overflow\n"); free(out_buf); goto lzma_cleanup; } + if ( xc_dom_kernel_check_size(dom, outsize * 2) ) + { + xc_dom_printf("LZMA: output too large"); + free(out_buf); + goto lzma_cleanup; + } + tmp_buf = realloc(out_buf, outsize * 2); if ( tmp_buf == NULL ) { diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c index df8e83b..6770302 100644 --- a/tools/libxc/xc_dom_core.c +++ b/tools/libxc/xc_dom_core.c @@ -139,7 +139,8 @@ 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) + const char *filename, size_t * size, + const size_t max_size) { struct xc_dom_mem *block = NULL; int fd = -1; @@ -151,6 +152,13 @@ void *xc_dom_malloc_filemap(struct xc_dom_image *dom, lseek(fd, 0, SEEK_SET); *size = lseek(fd, 0, SEEK_END); + if ( max_size && *size > max_size ) + { + xc_dom_panic(XC_OUT_OF_MEMORY, + "tried to map file which is too large"); + goto err; + } + block = malloc(sizeof(*block)); if ( block == NULL ) goto err; @@ -202,6 +210,40 @@ char *xc_dom_strdup(struct xc_dom_image *dom, const char *str) } /* ------------------------------------------------------------------------ */ +/* decompression buffer sizing */ +int xc_dom_kernel_check_size(struct xc_dom_image *dom, size_t sz) +{ + /* No limit */ + if ( !dom->max_kernel_size ) + return 0; + + if ( sz > dom->max_kernel_size ) + { + xc_dom_panic(XC_INVALID_KERNEL, + "kernel image too large"); + return 1; + } + + return 0; +} + +int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz) +{ + /* No limit */ + if ( !dom->max_ramdisk_size ) + return 0; + + if ( sz > dom->max_ramdisk_size ) + { + xc_dom_panic(XC_INVALID_KERNEL, + "ramdisk image too large"); + return 1; + } + + return 0; +} + +/* ------------------------------------------------------------------------ */ /* read files, copy memory blocks, with transparent gunzip */ size_t xc_dom_check_gzip(void *blob, size_t ziplen) @@ -215,7 +257,7 @@ size_t xc_dom_check_gzip(void *blob, size_t ziplen) gzlen = blob + ziplen - 4; unziplen = gzlen[3] << 24 | gzlen[2] << 16 | gzlen[1] << 8 | gzlen[0]; - if ( (unziplen < 0) || (unziplen > (1024*1024*1024)) ) /* 1GB limit */ + if ( (unziplen < 0) || (unziplen > XC_DOM_DECOMPRESS_MAX) ) { xc_dom_printf ("%s: size (zip %zd, unzip %zd) looks insane, skip gunzip\n", @@ -266,6 +308,9 @@ int xc_dom_try_gunzip(struct xc_dom_image *dom, void **blob, size_t * size) if ( unziplen == 0 ) return 0; + if ( xc_dom_kernel_check_size(dom, unziplen) ) + return 0; + unzip = xc_dom_malloc(dom, unziplen); if ( unzip == NULL ) return -1; @@ -562,6 +607,10 @@ struct xc_dom_image *xc_dom_allocate(const char *cmdline, const char *features) goto err; memset(dom, 0, sizeof(*dom)); + + dom->max_kernel_size = XC_DOM_DECOMPRESS_MAX; + dom->max_ramdisk_size = XC_DOM_DECOMPRESS_MAX; + if ( cmdline ) dom->cmdline = xc_dom_strdup(dom, cmdline); if ( features ) @@ -582,10 +631,25 @@ struct xc_dom_image *xc_dom_allocate(const char *cmdline, const char *features) return NULL; } +int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz) +{ + xc_dom_printf("%s: kernel_max_size=%zx", __FUNCTION__, sz); + dom->max_kernel_size = sz; + return 0; +} + +int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz) +{ + xc_dom_printf("%s: ramdisk_max_size=%zx", __FUNCTION__, sz); + dom->max_ramdisk_size = sz; + return 0; +} + int xc_dom_kernel_file(struct xc_dom_image *dom, const char *filename) { xc_dom_printf("%s: filename=\"%s\"\n", __FUNCTION__, filename); - dom->kernel_blob = xc_dom_malloc_filemap(dom, filename, &dom->kernel_size); + dom->kernel_blob = xc_dom_malloc_filemap(dom, filename, &dom->kernel_size, + dom->max_kernel_size); if ( dom->kernel_blob == NULL ) return -1; return xc_dom_try_gunzip(dom, &dom->kernel_blob, &dom->kernel_size); @@ -595,7 +659,9 @@ int xc_dom_ramdisk_file(struct xc_dom_image *dom, const char *filename) { xc_dom_printf("%s: filename=\"%s\"\n", __FUNCTION__, filename); dom->ramdisk_blob = - xc_dom_malloc_filemap(dom, filename, &dom->ramdisk_size); + xc_dom_malloc_filemap(dom, filename, &dom->ramdisk_size, + dom->max_ramdisk_size); + if ( dom->ramdisk_blob == NULL ) return -1; // return xc_dom_try_gunzip(dom, &dom->ramdisk_blob, &dom->ramdisk_size); @@ -755,7 +821,11 @@ int xc_dom_build_image(struct xc_dom_image *dom) void *ramdiskmap; unziplen = xc_dom_check_gzip(dom->ramdisk_blob, dom->ramdisk_size); + if ( xc_dom_ramdisk_check_size(dom, unziplen) != 0 ) + unziplen = 0; + ramdisklen = unziplen ? unziplen : dom->ramdisk_size; + if ( xc_dom_alloc_segment(dom, &dom->ramdisk_seg, "ramdisk", 0, ramdisklen) != 0 ) goto err; diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub index e52df7b..93b0cac 100644 --- a/tools/pygrub/src/pygrub +++ b/tools/pygrub/src/pygrub @@ -28,6 +28,7 @@ import grub.LiloConf import grub.ExtLinuxConf PYGRUB_VER = 0.6 +FS_READ_MAX = 1024 * 1024 def enable_cursor(ison): if ison: @@ -407,7 +408,8 @@ class Grub: if self.__dict__.get('cf', None) is None: raise RuntimeError, "couldn't find bootloader config file in the image provided." f = fs.open_file(self.cf.filename) - buf = f.read() + # limit read size to avoid pathological cases + buf = f.read(FS_READ_MAX) del f self.cf.parse(buf) @@ -633,6 +635,37 @@ if __name__ == "__main__": def usage(): print >> sys.stderr, "Usage: %s [-q|--quiet] [-i|--interactive] [--output=] [--kernel=] [--ramdisk=] [--args=] [--entry=] " %(sys.argv[0],) + def copy_from_image(fs, file_to_read, file_type, output_directory, + not_really): + if not_really: + if fs.file_exists(file_to_read): + return "<%s:%s>" % (file_type, file_to_read) + else: + sys.exit("The requested %s file does not exist" % file_type) + try: + datafile = fs.open_file(file_to_read) + except Exception, e: + print >>sys.stderr, e + sys.exit("Error opening %s in guest" % file_to_read) + (tfd, ret) = tempfile.mkstemp(prefix="boot_"+file_type+".", + dir=output_directory) + dataoff = 0 + while True: + data = datafile.read(FS_READ_MAX, dataoff) + if len(data) == 0: + os.close(tfd) + del datafile + return ret + try: + os.write(tfd, data) + except Exception, e: + print >>sys.stderr, e + os.close(tfd) + os.unlink(ret) + del datafile + sys.exit("Error writing temporary copy of "+file_type) + dataoff += len(data) + try: opts, args = getopt.gnu_getopt(sys.argv[1:], 'qih::', ["quiet", "interactive", "help", "output=", @@ -712,18 +745,18 @@ if __name__ == "__main__": if not chosencfg["kernel"]: chosencfg = run_grub(file, entry, fs, incfg["args"]) - data = fs.open_file(chosencfg["kernel"]).read() - (tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.", - dir="/var/run/xend/boot") - os.write(tfd, data) - os.close(tfd) + bootcfg["kernel"] = copy_from_image(fs, chosencfg["kernel"], "kernel", + output_directory, not_really) if chosencfg["ramdisk"]: - data = fs.open_file(chosencfg["ramdisk"],).read() - (tfd, bootcfg["ramdisk"]) = tempfile.mkstemp(prefix="boot_ramdisk.", - dir="/var/run/xend/boot") - os.write(tfd, data) - os.close(tfd) + try: + bootcfg["ramdisk"] = copy_from_image(fs, chosencfg["ramdisk"], + "ramdisk", output_directory, + not_really) + except: + if not not_really: + os.unlink(bootcfg["kernel"]) + raise else: initrd = None diff --git a/tools/python/xen/xm/messages/xen-xm.pot b/tools/python/xen/xm/messages/xen-xm.pot index a600a69..3d381f1 100644 --- a/tools/python/xen/xm/messages/xen-xm.pot +++ b/tools/python/xen/xm/messages/xen-xm.pot @@ -8,10 +8,11 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2008-03-31 17:40+0100\n" +"POT-Creation-Date: 2013-02-07 10:25+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" +"Language: \n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=CHARSET\n" "Content-Transfer-Encoding: 8bit\n" -- 1.7.2.5