Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38423 )
Change subject: cbfs: Add metadata cache ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38423/10/src/lib/cbfs.c File src/lib/cbfs.c:
https://review.coreboot.org/c/coreboot/+/38423/10/src/lib/cbfs.c@28 PS10, Line 28: if (err == CB_CBFS_CACHE_FULL)
Sorry, I really don't see the relation between what I wrote and what you wrote here, I don't get the impression you understood what I was trying to say. Again, I am *not leaving any gaps, holes or known-but-closeable windows open* in my design. If you enable the CONFIG_TOCTOU_SAFETY option in Kconfig, the design *will be TOCTOU-safe*! Period. This is not a temporary incremental RO-only thing, this will hold true for the final design for both the RO and RW CBFS. If you verify the anchor in a TOCTOU-safe way (e.g. using fused SoC hashing) and you run this on a non-XIP platform, you will have an entirely TOCTOU-safe boot flow start to end, RO and RW, no other open windows remaining. Even when accessing a file that did not fit in the mcache, this implementation will do so in an entirely TOCTOU-safe manner. The mcache overflow issue does *not* threaten TOCTOU-safety or any other security guarantee in *any* way. I keep saying that but I don't get the impression you hear me to be honest.
I might be confused. I was under the impression that we weren't going to be enabling CONFIG_TOCTOU_SAFETY on Chrome OS. And I think we should. I'm probably misunderstanding.
What you're proposing requires every platform to carefully separate its CBFS files based on what stage they are needed in. Some files are needed in both pre-RAM and post-RAM stages, so then what -- either you have to store them twice or you have to provide extra APIs to allow one side to access the CBFS of the other, and place the cognitive burden of figuring out what to use where onto each platform maintainer. It also requires a bunch extra plumbing to set up yet another mcache instance and reserve space for that in memlayout and...
I don't think the implementation would need to be what you mentioned.
I disagree that this "needs to be addressed eventually", because my solution doesn't need this, not now and not in the future. My solution can solve this problem and still provide all the same security guarantees you want without adding any of this stuff. That is why I think it is better.
I think it depends on what we would be optimizing for on specific platforms in a pre-ram environment. Those resources are limited which inherently makes one need to optimize for something. If we're reliant upon the seeding full RW CBFS metadata in RO (and dependent on number RW files) then we're forcing our hand w.r.t. tradeoffs when one could fill mcache partially. I don't believe every platform is the same, and I'm worried we're not dealing with the limited resource problem up front. That's where I'm coming from, at least.