Attention is currently required from: Julius Werner.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78808?usp=email )
Change subject: security/vboot: Avoid using invalid vb2_context pointer ......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1: The main point of this patch is to prevent the device continuing to boot when `vb2api_reinit()` fails (for example when the vboot shared data version isn't the same in RO and RW). In that case, `vboot_ctx` won't be properly initialized. Our vboot code heavily relies on `vboot_ctx`. If the boot flow continues with an invalid `vboot_ctx`, weird security issues may happen (such as entering manual recovery in RW due to incorrect `vboot_ctx->boot_mode`?), which I think we'll want to avoid. Therefore what I want here is just an always-fatal version of assertion (similar to `VERIFY` in Visual C++).
In general, assert() is the check you're supposed to use when you "know" that something must be true (because of the way the rest of the code is written)
I totally agree with you about that. I think the debate here is about the definition of "the rest of code". I'd probably interpret it as "the rest of code within the same coreboot stage", instead of "the rest of code within the firmware image".
On a quick look, most of the other `assert` calls in coreboot seem to be simple checks for input arguments. So I think the cases here are quite different.