Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41448 )
Change subject: cbfs: add cbfs_boot_locate_file_get_compression_info() ......................................................................
cbfs: add cbfs_boot_locate_file_get_compression_info()
There's no current way to obtain the decompression information (algo and size) with the current API. Therefore, split cbfs_boot_load_file() up and add cbfs_boot_locate_file_get_compression_info() so other callers can obtain the same infromation.
BUG=b:155322763,b:150746858,b:152909132
Change-Id: I605b036a50ed26ad6897b5b86a37897766169e6b Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/include/cbfs.h M src/lib/cbfs.c 2 files changed, 21 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/41448/1
diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 29621c6..172b4b1 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -23,6 +23,11 @@ /* Locate file in a specific region of fmap. Return 0 on success. < 0 on error*/ int cbfs_locate_file_in_region(struct cbfsf *fh, const char *region_name, const char *name, uint32_t *type); +/* Locate file by name filling in the cbfs_type and compression information. + Returns 0 on success, < 0 on error. */ +int cbfs_boot_locate_file_get_compression_info(struct cbfsf *fh, + const char *name, uint32_t *cbfs_type, + uint32_t *compression_algo, size_t *decompressed_size); /* Load an arbitrary type file from CBFS into a buffer. Returns amount of * loaded bytes on success or 0 on error. File will get decompressed as * necessary. Same decompression requirements as diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index a3294de..c6767b0 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -189,6 +189,19 @@ return cbfs_boot_map_with_leak(name, CBFS_TYPE_OPTIONROM, NULL); }
+int cbfs_boot_locate_file_get_compression_info(struct cbfsf *fh, + const char *name, uint32_t *cbfs_type, + uint32_t *compression_algo, size_t *decompressed_size) +{ + if (cbfs_boot_locate(fh, name, cbfs_type) < 0) + return -1; + + if (cbfsf_decompression_info(fh, compression_algo, decompressed_size) < 0) + return -1; + + return 0; +} + size_t cbfs_boot_load_file(const char *name, void *buf, size_t buf_size, uint32_t type) { @@ -196,13 +209,11 @@ uint32_t compression_algo; size_t decompressed_size;
- if (cbfs_boot_locate(&fh, name, &type) < 0) + if (cbfs_boot_locate_file_get_compression_info(&fh, name, &type, + &compression_algo, &decompressed_size) < 0) return 0;
- if (cbfsf_decompression_info(&fh, &compression_algo, - &decompressed_size) - < 0 - || decompressed_size > buf_size) + if (decompressed_size > buf_size) return 0;
return cbfs_load_and_decompress(&fh.data, 0, region_device_sz(&fh.data),
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41448 )
Change subject: cbfs: add cbfs_boot_locate_file_get_compression_info() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG@10 PS1, Line 10: size) with the current API. Therefore, split cbfs_boot_load_file() up Can't you just run cbfs_boot_locate() and then cbfsf_decompression_info() on the resulting file handle? I don't think there's any rule against using <commonlib/cbfs.h> APIs from other files, we're using cbfsf_file_type() all over the place. (Just trying to reduce churn on the CBFS code while I'm trying to change all of it.)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41448 )
Change subject: cbfs: add cbfs_boot_locate_file_get_compression_info() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG@10 PS1, Line 10: size) with the current API. Therefore, split cbfs_boot_load_file() up
Can't you just run cbfs_boot_locate() and then cbfsf_decompression_info() on the resulting file hand […]
I can do that. I don't think it saves much since the sequence in this change would be the same at the caller sites.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41448 )
Change subject: cbfs: add cbfs_boot_locate_file_get_compression_info() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG@10 PS1, Line 10: size) with the current API. Therefore, split cbfs_boot_load_file() up
I can do that. […]
I just don't think it's worth adding right now. I'd prefer to avoid platform-specific code dealing with things like decompression directly wherever possible anyway (things like that tend to be a headache for TOCTOU safety). It would be best if you could just use cbfs_boot_load_file(() for this but I guess the adding to CBMEM doesn't work with that. Maybe we should have a cbfs_load_to_cbmem(name, cbmem_id) or something like that. Haven't fully thought through all the use cases yet.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41448 )
Change subject: cbfs: add cbfs_boot_locate_file_get_compression_info() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41448/1//COMMIT_MSG@10 PS1, Line 10: size) with the current API. Therefore, split cbfs_boot_load_file() up
I just don't think it's worth adding right now. I'd prefer to avoid platform-specific code dealing with things like decompression directly wherever possible anyway (things like that tend to be a headache for TOCTOU safety). It would be best if you could just use cbfs_boot_load_file(() for this but I guess the adding to CBMEM doesn't work with that. Maybe we should have a cbfs_load_to_cbmem(name, cbmem_id) or something like that. Haven't fully thought through all the use cases yet.
See the 2 new patches prior to this original set. This one we can abandon.
Aaron Durbin has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41448 )
Change subject: cbfs: add cbfs_boot_locate_file_get_compression_info() ......................................................................
Abandoned
different approach was taken
-
Aaron Durbin (Code Review) -
Julius Werner (Code Review)