Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36300 )
Change subject: vboot: use vboot persistent context ......................................................................
Patch Set 12: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/common.c... PS1, Line 55: if (wd->buffer_offset == ALIGN_UP(sizeof(*wd), 16)) {
Perhaps we should just punt this for now, and deal with it along with cleaning up all the recovery_r […]
Yes. I don't think there's an immediate problem here. We're just discussing additional safeguards we could add if we wanted to. (Note that those safeguards already don't exist today: if you call vboot_get_recovery_reason_shared_data() before vboot_init_work_context() in the current ToT you're also accessing garbage data and potentially invalid addresses. So this patch is not regressing safety, although we could of course still add more in the future.)
https://review.coreboot.org/c/coreboot/+/36300/12/src/security/vboot/common.... File src/security/vboot/common.c:
https://review.coreboot.org/c/coreboot/+/36300/12/src/security/vboot/common.... PS12, Line 52: struct vb2_context **vboot_ctx_ptr = car_get_var_ptr(&vboot_ctx); Honestly, I find this harder to follow than the previous version. But might be a matter of taste.
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/c/coreboot/+/36300/1/src/security/vboot/misc.h@5... PS1, Line 58: void *vboot_get_workbuf(void);
Julius, could you comment on https://bugs.chromium. […]
Done