Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31606 )
Change subject: libpayload: cbfs: Require input size and output size for cbfs_decompress ......................................................................
libpayload: cbfs: Require input size and output size for cbfs_decompress
Currently, cbfs_decompress() calls ulzma() and ulz4f() for LZMA/LZ4 decompression. These two functions don't accept input/output size as parameters. We can make cbfs_decompress more robust by calling ulzman() and ulz4fn() instead. This could prevent us from overflowing destination buffer.
BUG=none BRANCH=none TEST=boot into kernel on Kukui with COMPRESSED_PAYLOAD_LZMA / COMPRESSED_PAYLOAD_LZ4.
Change-Id: Ibe617825bd000ed618791d8e3c5f65bbbd5f7e33 Signed-off-by: You-Cheng Syu youcheng@google.com Reviewed-on: https://review.coreboot.org/c/31606 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Daisuke Nojiri dnojiri@chromium.org --- M payloads/libpayload/include/cbfs_core.h M payloads/libpayload/libcbfs/cbfs.c M payloads/libpayload/libcbfs/cbfs_core.c 3 files changed, 17 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Daisuke Nojiri: Looks good to me, approved
diff --git a/payloads/libpayload/include/cbfs_core.h b/payloads/libpayload/include/cbfs_core.h index 364f6c4..a707154 100644 --- a/payloads/libpayload/include/cbfs_core.h +++ b/payloads/libpayload/include/cbfs_core.h @@ -253,8 +253,9 @@ void *cbfs_get_file_content(struct cbfs_media *media, const char *name, int type, size_t *sz);
-/* returns decompressed size on success, 0 on failure */ -int cbfs_decompress(int algo, void *src, void *dst, int len); +/* Returns decompressed size on success, 0 on failure. */ +size_t cbfs_decompress(int algo, const void *src, size_t srcn, void *dst, + size_t dstn);
/* returns a pointer to CBFS master header, or CBFS_HEADER_INVALID_ADDRESS * on failure */ diff --git a/payloads/libpayload/libcbfs/cbfs.c b/payloads/libpayload/libcbfs/cbfs.c index 577fb20..d679e67 100644 --- a/payloads/libpayload/libcbfs/cbfs.c +++ b/payloads/libpayload/libcbfs/cbfs.c @@ -114,8 +114,9 @@ final_size = cbfs_decompress(stage->compression, ((unsigned char *) stage) + sizeof(struct cbfs_stage), + stage->len, (void *) (uintptr_t) stage->load, - stage->len); + stage->memlen); if (!final_size) { entry = -1; goto out; diff --git a/payloads/libpayload/libcbfs/cbfs_core.c b/payloads/libpayload/libcbfs/cbfs_core.c index 91f2603..4ecda30 100644 --- a/payloads/libpayload/libcbfs/cbfs_core.c +++ b/payloads/libpayload/libcbfs/cbfs_core.c @@ -48,6 +48,7 @@ * */
+#include <libpayload.h> #include <cbfs.h> #include <string.h> #include <sysinfo.h> @@ -256,7 +257,8 @@ return NULL;
ret = malloc(*size); - if (ret != NULL && !cbfs_decompress(algo, data, ret, *size)) { + if (ret != NULL && + !cbfs_decompress(algo, data, on_media_size, ret, *size)) { free(ret); ret = NULL; } @@ -321,24 +323,27 @@ return NULL; }
-int cbfs_decompress(int algo, void *src, void *dst, int len) +size_t cbfs_decompress(int algo, const void *src, size_t srcn, void *dst, + size_t dstn) { + size_t len; switch (algo) { case CBFS_COMPRESS_NONE: + len = MIN(srcn, dstn); memcpy(dst, src, len); return len; #ifdef CBFS_CORE_WITH_LZMA case CBFS_COMPRESS_LZMA: - return ulzma(src, dst); + return ulzman(src, srcn, dst, dstn); #endif #ifdef CBFS_CORE_WITH_LZ4 case CBFS_COMPRESS_LZ4: - return ulz4f(src, dst); + return ulz4fn(src, srcn, dst, dstn); #endif default: - ERROR("tried to decompress %d bytes with algorithm #%x," - "but that algorithm id is unsupported.\n", len, - algo); + ERROR("tried to decompress %zu bytes with algorithm " + "#%x, but that algorithm id is unsupported.\n", + srcn, algo); return 0; } }