Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32704 )
Change subject: security: Add common boot media write protection ......................................................................
Patch Set 6:
(11 comments)
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... File src/security/lockdown/Kconfig:
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 2: config SECURITY_BOOTMEDIA_LOCKDOWN
SECURITY_HAVE_BOOTMEDIA_LOCKDOWN
removed
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 6: support
supports
removed
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 18: LOCK_RO
Can we name this option with 'whole' in it so it reads straight forward in the code?
Done
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 22: media
medium?
Done
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 26: NOTE: If you trigger the chipset lockdown unconditionally,
I wouldn’t indent the note. `NOTE:` is enough “markup”.
Done
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 30: LOCK_NO_ACCESS
same as my comment above.
Done
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 30: NO_ACCESS
RW to be consistent with RO?
it's not read-writeable
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/Kconf... PS3, Line 36: The locking will take place during the chipset lockdown, which is
Add a blank line above?
I don't understand
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/bootm... File src/security/lockdown/bootmedia.c:
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/bootm... PS3, Line 40: "whole bootmedia\n");
Won’t this be printed several time?
removed the loop
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/bootm... PS3, Line 49: }
Add an else branch to inform the user about an error?
Unified the error handling
https://review.coreboot.org/c/coreboot/+/32704/3/src/security/lockdown/bootm... PS3, Line 53: Didn't
Didn’t or couldn’t … […]
Isn't printed any more if no bootmedia protection is selected