[coreboot-gerrit] New patch to review for coreboot: lzma: Port size-checking ulzman() version to coreboot

Julius Werner (jwerner@chromium.org) gerrit at coreboot.org
Mon Feb 8 23:42:01 CET 2016


Julius Werner (jwerner at chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13637

-gerrit

commit 0e64a6dcedc642001df5480df7bd1cc44d11cfc8
Author: Julius Werner <jwerner at chromium.org>
Date:   Mon Feb 8 11:46:22 2016 -0800

    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 <jwerner at chromium.org>
---
 src/include/lib.h    |  3 ++-
 src/lib/lzma.c       | 19 ++++++++++++++-----
 src/lib/lzmadecode.c |  2 +-
 src/lib/rmodule.c    |  2 +-
 src/lib/selfboot.c   |  8 ++++----
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/include/lib.h b/src/include/lib.h
index ab1f067..03a484b 100644
--- a/src/include/lib.h
+++ b/src/include/lib.h
@@ -21,7 +21,8 @@
 #include <types.h>
 
 /* Defined in src/lib/lzma.c */
-unsigned long ulzma(unsigned char *src, unsigned char *dst);
+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 c04f4a4..9c5f58e 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,10 @@ 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 (outSize > dstn)
+		outSize = dstn;
+	if (LzmaDecodeProperties(&state.Properties, properties,
+				 LZMA_PROPERTIES_SIZE) != LZMA_RESULT_OK) {
 		printk(BIOS_WARNING, "lzma: Incorrect stream properties.\n");
 		return 0;
 	}
@@ -47,8 +51,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 +60,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 ada7226..4f6a677 100644
--- a/src/lib/lzmadecode.c
+++ b/src/lib/lzmadecode.c
@@ -31,7 +31,7 @@
 
 /* Use 32-bit reads whenever possible to avoid bad flash performance.  */
 #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 13eb324..628195c 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 60cda6a..6d86159 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,



More information about the coreboot-gerrit mailing list