[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2] libxc: add LZ4 decompression support
On Tue, 2013-09-24 at 17:29 +0100, Jan Beulich wrote: > Since there's no shared or static library to link against, this simply > re-uses the hypervisor side code. However, I only audited the code > added here for possible security issues, not the referenced code in > the hypervisor tree. > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > --- > v2: Fold in the addition of a previously missing include in the Mini-OS > case - provided by Ian Campbell - to allow stubdom compilation to > succeed. > --- > I intentionally retained the tab indentation in the code cloned from > its hypervisor original (which in turn retained it as being a clone > of the Linux original), to ease diff-ing. > > --- a/tools/libxc/xc_dom_bzimageloader.c > +++ b/tools/libxc/xc_dom_bzimageloader.c > @@ -566,6 +566,8 @@ static int xc_try_lzo1x_decode( > > #else /* __MINIOS__ */ > > +#include <mini-os/byteorder.h> > + > int xc_try_bzip2_decode(struct xc_dom_image *dom, void **blob, size_t *size); > int xc_try_lzma_decode(struct xc_dom_image *dom, void **blob, size_t *size); > int xc_try_lzo1x_decode(struct xc_dom_image *dom, void **blob, size_t *size); > @@ -573,6 +575,124 @@ int xc_try_xz_decode(struct xc_dom_image > > #endif /* !__MINIOS__ */ > > +#define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS > +#define STATIC static > +#define u8 uint8_t > +#define u16 uint16_t > +#define u32 uint32_t > +#define u64 uint64_t > +#define INIT > +#define unlikely(x) (x) I think rather than pollute this file with this sort of thing (which might have unexpected consequences in the future) this would be better of placed in a separate file compiled for both regular and stub use. > + > +static inline uint_fast16_t le16_to_cpup(const unsigned char *buf) > +{ > + return buf[0] | (buf[1] << 8); > +} > + > +static inline uint_fast32_t le32_to_cpup(const unsigned char *buf) > +{ > + return le16_to_cpup(buf) | ((uint32_t)le16_to_cpup(buf + 2) << 16); > +} > + > +#include "../../xen/common/lz4/decompress.c" > + > +/* > + * Note: Uncompressed chunk size is used in the compressor side > + * (userspace side for compression). > + * It is hardcoded because there is not proper way to extract it > + * from the binary stream which is generated by the preliminary > + * version of LZ4 tool so far. > + */ > +#define LZ4_DEFAULT_UNCOMPRESSED_CHUNK_SIZE (8 << 20) > +#define ARCHIVE_MAGICNUMBER 0x184C2102 > + > +static int xc_try_lz4_decode( > + struct xc_dom_image *dom, void **blob, size_t *psize) > +{ > + int ret = -1; > + size_t uncomp_chunksize = LZ4_DEFAULT_UNCOMPRESSED_CHUNK_SIZE; > + unsigned char *inp = *blob, *output, *outp; > + ssize_t size = *psize - 4; > + size_t out_size, out_len, dest_len, chunksize; > + const char *msg; > + > + if (size < 4) { > + msg = "input too small"; > + goto exit_0; The exit_0 loop is after the error print, so these errors don't get printed. I don't really like exit_0 and exit_2 as names (what happened to exit_1?). "err" and "err_free" would be more usual. > + } > + > + out_size = out_len = get_unaligned_le32(inp + size); > + if (xc_dom_kernel_check_size(dom, out_len)) { > + msg = "Decompressed image too large"; > + goto exit_0; > + } > + > + output = malloc(out_len); > + if (!output) { > + msg = "Could not allocate output buffer"; > + goto exit_0; > + } > + outp = output; > + > + chunksize = get_unaligned_le32(inp); > + if (chunksize == ARCHIVE_MAGICNUMBER) { > + inp += 4; > + size -= 4; > + } else { > + msg = "invalid header"; > + goto exit_2; > + } > + > + for (;;) { > + if (size < 4) { > + msg = "missing data"; > + goto exit_2; > + } > + chunksize = get_unaligned_le32(inp); > + if (chunksize == ARCHIVE_MAGICNUMBER) { > + inp += 4; > + size -= 4; > + continue; > + } > + inp += 4; > + size -= 4; We know size is at least 4 from the check at the head of the loop, but now we subtracted 4 so size could be 0, but lz4_decompress doesn't take size as an argument so whatever it does it isn't basing the return value in &chunksize on it, so I assume it reads over the end of the buffer via inp. I didn't look to see what the minimum number of bytes which lz4_decompress will consume is, but I think we need a check of some description. > + > + if (out_len >= uncomp_chunksize) { > + dest_len = uncomp_chunksize; > + out_len -= dest_len; > + } else > + dest_len = out_len; > + ret = lz4_decompress(inp, &chunksize, outp, dest_len); > + if (ret < 0) { > + msg = "decoding failed"; > + goto exit_2; > + } > + > + outp += dest_len; > + size -= chunksize; > + > + if (size == 0) > + { > + *blob = output; > + *psize = out_size; > + return 0; > + } > + > + if (size < 0) { > + msg = "data corrupted"; > + goto exit_2; > + } > + > + inp += chunksize; > + } > + > +exit_2: > + free(output); > + DOMPRINTF("LZ4 decompression error: %s\n", msg); > +exit_0: > + return ret; > +} > + > struct setup_header { > uint8_t _pad0[0x1f1]; /* skip uninteresting stuff */ > uint8_t setup_sects; > @@ -733,6 +853,17 @@ static int xc_dom_probe_bzimage_kernel(s > return -EINVAL; > } > } > + else if ( check_magic(dom, "\x02\x21", 2) ) > + { > + ret = xc_try_lz4_decode(dom, &dom->kernel_blob, &dom->kernel_size); > + if ( ret < 0 ) > + { > + xc_dom_panic(dom->xch, XC_INVALID_KERNEL, > + "%s unable to LZ4 decompress kernel\n", > + __FUNCTION__); > + return -EINVAL; > + } > + } > else > { > xc_dom_panic(dom->xch, XC_INVALID_KERNEL, > --- a/xen/common/lz4/decompress.c > +++ b/xen/common/lz4/decompress.c > @@ -151,7 +151,7 @@ _output_error: > return (int) (-(((unsigned char *)ip) - source)); > } > > -#ifndef __XEN__ > +#if !defined(__XEN__) && !defined(__XEN_TOOLS__) > static int lz4_uncompress_unknownoutputsize(const char *source, char *dest, > int isize, size_t maxoutputsize) > { > @@ -294,7 +294,7 @@ exit_0: > return ret; > } > > -#ifndef __XEN__ > +#if !defined(__XEN__) && !defined(__XEN_TOOLS__) > int lz4_decompress_unknownoutputsize(const char *src, size_t src_len, > char *dest, size_t *dest_len) > { > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |