Age | Commit message (Collapse) | Author |
|
cb_err_t was meant to be used in place of `enum cb_err` in all
situations, but the choice to use a typedef here seems to be
controversial. We should not be arbitrarily using two different
identifiers for the same thing across the codebase, so since there are
no use cases for serializing enum cb_err at the moment (which would be
the primary reason to typedef a fixed-width integer instead), remove
cb_err_t again for now.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Iaec36210d129db26d51f0a105d3de070c03b686b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62600
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
|
|
This patch fixes a few minor CBFS parsing edge cases that could lead to
unintended behavior: the CBFS attribute parser could have run into an
infinite loop if an attribute's length was (accidentally or maliciously)
invalid. A length of 0 would have caused it to read the same attribute
over and over again without making forward progress, while a very large
length could have caused an overflow that makes it go backwards to find
the next attribute. Also, the filename was not guaranteed to be
null-terminated which could have resulted in out-of-bounds reads on a
few error messages.
Finally, clarify the validity guarantees for CBFS header fields offered
by cbfs_walk() in the comment explaining cbfs_mdata.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ie569786e5bec355b522f6580f53bdd8b16a4d726
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57569
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jakub Czapiga <jacz@semihalf.com>
|
|
CBFS file with lenth of (UINT32_MAX - cbfs_file.offset + 1) causes
overflow, making cbfs_walk() being stuck in an infinite loop, and
checking the same file. This patch makes cbfs_walk() skip file headers
with incorrect data_offset or data_length.
Signed-off-by: Jakub Czapiga <jacz@semihalf.com>
Change-Id: I70020e347087cbd8134a1a60177fa9eef63fb7bd
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57525
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
|
|
The new CBFS stack was written to try to isolate cases of single file
corruption as far as possible and still make other files avaialble (at
least as long as verification is disabled and they can still be found at
all). For most cases of header corruption, it will just continue trying
to parse the next file. However, in cases where parts of the file extend
beyond the end of the rdev, we have been relying on the range checking
of the rdev API rather than doing it explicitly.
This is fine in general, but it causes the problem that these errors
cannot be distinguished from I/O errors, and I/O errors always make the
whole cbfs_walk() fail. That means we will not return a successful
result from cbfs_mcache_build(), and leads to an odd discrepancy in how
certain kinds of corrupted CBFSes are treated with and without mcache.
This patch adds an explicit range check to make the behavior consistent.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ice2b6960284bd0c19be35b0607e5e32791e7a64c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57271
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jakub Czapiga <jacz@semihalf.com>
|
|
This patch adds file data hashing for CONFIG_CBFS_VERIFICATION. With
this, all CBFS accesses using the new CBFS APIs (cbfs_load/_map/_alloc
and variants) will be fully verified when verification is enabled. (Note
that some use of legacy APIs remains and thus the CBFS_VERIFICATION
feature is not fully finished.)
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ic9fff279f69cf3b7c38a0dc2ff3c970eaa756aa8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52084
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
|
|
Attribute tags are defined as hexadecimal constants, not decimal, so it
makes more sense to print them like that in error messages as well.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I3a5a6a8c9b8d24e57633595fc47221a483d8593a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48836
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
|
|
This patch reduces some code duplication in cbfstool by switching it to
use the CBFS data structure definitions in commonlib rather than its own
private copy. In addition, replace a few custom helpers related to hash
algorithms with the official vboot APIs of the same purpose.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I22eae1bcd76d85fff17749617cfe4f1de55603f4
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41117
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Wim Vervoorn <wvervoorn@eltan.com>
|
|
This patch adapts cbfs_load() and cbfs_map() to use the new CBFS API
directly, rather than through cbfs_boot_locate(). For cbfs_load() this
means that attribute metadata does not need to be read twice.
Change-Id: I754cc34b1c1471129e15475aa0f1891e02439a02
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39305
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
|
|
This patch adds a new CBFS implementation that is intended to replace
the existing commonlib/cbfs.c. The new implementation is designed to
meet a bunch of current and future goals that in aggregate make it
easier to start from scratch than to adapt the exisiting implementation:
1. Be BSD-licensed so it can evetually be shared with libpayload.
2. Allow generating/verifying a metadata hash for future CBFS per-file
verification (see [1][2]).
3. Be very careful about reading (not mmaping) all data only once, to be
suitable for eventual TOCTOU-safe verification.
4. Make it possible to efficiently implement all current and future
firmware use cases (both with and without verification).
The main primitive is the cbfs_walk() function which will traverse a
CBFS and call a callback for every file. cbfs_lookup() uses this to
implement the most common use case of finding a file so that it can be
read. A host application using this code (e.g. coreboot, libpayload,
cbfstool) will need to provide a <cbfs_glue.h> header to provide the
glue to access the respective CBFS storage backend implementation.
This patch merely adds the code, the next patch will integrate it into
coreboot.
[1]: https://www.youtube.com/watch?v=Hs_EhewBgtM
[2]: https://osfc.io/uploads/talk/paper/47/The_future_of_firmware_verification_in_coreboot.pdf
(Note: In early discussions the metadata hash was called "master hash".)
Change-Id: Ica64c1751fa37686814c0247460c399261d5814c
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38421
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|