From a25b5d257dbfbff808b19bf8c48565435e6bef9d Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Mon, 8 Feb 2016 11:46:22 -0800 Subject: [PATCH] lzma: Port size-checking ulzman() version to coreboot We've had a second version of ulzma() that would check the input and output buffer sizes in libpayload for a while now. Since it's generally never a bad idea to double-check for overruns, let's port it to coreboot and use it where applicable. (This requires a small fix in the four byte at a time read optimization we only have in coreboot, since it made the stream counter hit the end a little earlier than the algorithm liked and could trigger an assertion.) BRANCH=None BUG=None TEST=Booted Oak, Jerry and Falco. Change-Id: Id566b31dfa896ea1b991badf5a6ad9d075aef987 Signed-off-by: Julius Werner Reviewed-on: https://review.coreboot.org/13637 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin --- src/include/lib.h | 5 +++-- src/lib/lzma.c | 17 ++++++++++++----- src/lib/lzmadecode.c | 7 +++++-- src/lib/rmodule.c | 2 +- src/lib/selfboot.c | 8 ++++---- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/include/lib.h b/src/include/lib.h index ab1f067ee3..737ab36b83 100644 --- a/src/include/lib.h +++ b/src/include/lib.h @@ -20,8 +20,9 @@ #include #include -/* Defined in src/lib/lzma.c */ -unsigned long ulzma(unsigned char *src, unsigned char *dst); +/* Defined in src/lib/lzma.c. Returns decompressed size or 0 on error. */ +size_t ulzma(const void *src, void *dst); +size_t ulzman(const void *src, size_t srcn, void *dst, size_t dstn); /* Defined in src/lib/ramtest.c */ void ram_check(unsigned long start, unsigned long stop); diff --git a/src/lib/lzma.c b/src/lib/lzma.c index c04f4a4040..5566cd5dfc 100644 --- a/src/lib/lzma.c +++ b/src/lib/lzma.c @@ -16,9 +16,10 @@ #include "lzmadecode.h" -unsigned long ulzma(unsigned char * src, unsigned char * dst) +size_t ulzman(const void *src, size_t srcn, void *dst, size_t dstn) { unsigned char properties[LZMA_PROPERTIES_SIZE]; + const int data_offset = LZMA_PROPERTIES_SIZE + 8; UInt32 outSize; SizeT inProcessed; SizeT outProcessed; @@ -26,7 +27,7 @@ unsigned long ulzma(unsigned char * src, unsigned char * dst) CLzmaDecoderState state; SizeT mallocneeds; MAYBE_STATIC unsigned char scratchpad[15980]; - unsigned char *cp; + const unsigned char *cp; /* Note: these timestamps aren't useful for memory-mapped media (x86) */ timestamp_add_now(TS_START_ULZMA); @@ -37,7 +38,8 @@ unsigned long ulzma(unsigned char * src, unsigned char * dst) * byte and re-construct. */ cp = src + LZMA_PROPERTIES_SIZE; outSize = cp[3] << 24 | cp[2] << 16 | cp[1] << 8 | cp[0]; - if (LzmaDecodeProperties(&state.Properties, properties, LZMA_PROPERTIES_SIZE) != LZMA_RESULT_OK) { + if (LzmaDecodeProperties(&state.Properties, properties, + LZMA_PROPERTIES_SIZE) != LZMA_RESULT_OK) { printk(BIOS_WARNING, "lzma: Incorrect stream properties.\n"); return 0; } @@ -47,8 +49,8 @@ unsigned long ulzma(unsigned char * src, unsigned char * dst) return 0; } state.Probs = (CProb *)scratchpad; - res = LzmaDecode(&state, src + LZMA_PROPERTIES_SIZE + 8, (SizeT)0xffffffff, &inProcessed, - dst, outSize, &outProcessed); + res = LzmaDecode(&state, src + data_offset, srcn - data_offset, + &inProcessed, dst, outSize, &outProcessed); if (res != 0) { printk(BIOS_WARNING, "lzma: Decoding error = %d\n", res); return 0; @@ -56,3 +58,8 @@ unsigned long ulzma(unsigned char * src, unsigned char * dst) timestamp_add_now(TS_END_ULZMA); return outProcessed; } + +size_t ulzma(const void *src, void *dst) +{ + return ulzman(src, ~(size_t)0, dst, ~(size_t)0); +} diff --git a/src/lib/lzmadecode.c b/src/lib/lzmadecode.c index ada72260f2..fbf15964a6 100644 --- a/src/lib/lzmadecode.c +++ b/src/lib/lzmadecode.c @@ -29,9 +29,12 @@ #define kBitModelTotal (1 << kNumBitModelTotalBits) #define kNumMoveBits 5 -/* Use 32-bit reads whenever possible to avoid bad flash performance. */ +/* Use 32-bit reads whenever possible to avoid bad flash performance. Fall back + * to byte reads for last 4 bytes since RC_TEST returns an error when BufferLim + * is *reached* (not surpassed!), meaning we can't allow that to happen while + * there are still bytes to decode from the algorithm's point of view. */ #define RC_READ_BYTE (look_ahead_ptr < 4 ? look_ahead.raw[look_ahead_ptr++] \ - : ((((uintptr_t) Buffer & 3) || ((SizeT) (BufferLim - Buffer) < 4)) ? (*Buffer++) \ + : ((((uintptr_t) Buffer & 3) || ((SizeT) (BufferLim - Buffer) <= 4)) ? (*Buffer++) \ : ((look_ahead.dw = *(UInt32 *)Buffer), (Buffer += 4), (look_ahead_ptr = 1), look_ahead.raw[0]))) #define RC_INIT2 Code = 0; Range = 0xFFFFFFFF; \ diff --git a/src/lib/rmodule.c b/src/lib/rmodule.c index 13eb324aa8..628195cafa 100644 --- a/src/lib/rmodule.c +++ b/src/lib/rmodule.c @@ -295,7 +295,7 @@ int rmodule_stage_load(struct rmod_stage_load *rsl) if (map == NULL) return -1; - fsize = ulzma(map, rmod_loc); + fsize = ulzman(map, stage.len, rmod_loc, stage.memlen); rdev_munmap(fh, map); diff --git a/src/lib/selfboot.c b/src/lib/selfboot.c index 60cda6ae9e..6d86159ddd 100644 --- a/src/lib/selfboot.c +++ b/src/lib/selfboot.c @@ -378,12 +378,12 @@ static int load_self_segments( /* Copy data from the initial buffer */ if (ptr->s_filesz) { unsigned char *middle, *end; - size_t len; - len = ptr->s_filesz; + size_t len = ptr->s_filesz; + size_t memsz = ptr->s_memsz; switch(ptr->compression) { case CBFS_COMPRESS_LZMA: { printk(BIOS_DEBUG, "using LZMA\n"); - len = ulzma(src, dest); + len = ulzman(src, len, dest, memsz); if (!len) /* Decompression Error. */ return 0; break; @@ -397,7 +397,7 @@ static int load_self_segments( printk(BIOS_INFO, "CBFS: Unknown compression type %d\n", ptr->compression); return -1; } - end = dest + ptr->s_memsz; + end = dest + memsz; middle = dest + len; printk(BIOS_SPEW, "[ 0x%08lx, %08lx, 0x%08lx) <- %08lx\n", (unsigned long)dest,