Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32705 )
Change subject: security/lockdown: Write-protect WP_RO ......................................................................
Patch Set 9:
(7 comments)
https://review.coreboot.org/c/coreboot/+/32705/5/src/include/boot_device.h File src/include/boot_device.h:
https://review.coreboot.org/c/coreboot/+/32705/5/src/include/boot_device.h@7... PS5, Line 78: Looks
Locks
Done
https://review.coreboot.org/c/coreboot/+/32705/5/src/security/lockdown/Kconf... File src/security/lockdown/Kconfig:
https://review.coreboot.org/c/coreboot/+/32705/5/src/security/lockdown/Kconf... PS5, Line 61: chip
nit: should probably say "boot media", not "chip"
Done
https://review.coreboot.org/c/coreboot/+/32705/6/src/security/lockdown/Kconf... File src/security/lockdown/Kconfig:
https://review.coreboot.org/c/coreboot/+/32705/6/src/security/lockdown/Kconf... PS6, Line 26: programmer
controller
Done
https://review.coreboot.org/c/coreboot/+/32705/6/src/security/lockdown/Kconf... PS6, Line 27: chipset lockdown
Then what about the LOCK_IN_VERSTAGE and LOCK_IN_RAMSTAGE options?
Done
https://review.coreboot.org/c/coreboot/+/32705/5/src/security/lockdown/lockd... File src/security/lockdown/lockdown.c:
https://review.coreboot.org/c/coreboot/+/32705/5/src/security/lockdown/lockd... PS5, Line 76: security_lockdown_bootmedia
Is there an expectation to add this to a BS entry some place else? Otherwise you could drop the void […]
Done
https://review.coreboot.org/c/coreboot/+/32705/6/src/security/lockdown/lockd... File src/security/lockdown/lockdown.c:
https://review.coreboot.org/c/coreboot/+/32705/6/src/security/lockdown/lockd... PS6, Line 56: for (size_t i = 0; i < ARRAY_SIZE(wp_prot); i++) {
This is the same sequence as above in the BOOTMEDIA_LOCK_RO path. […]
Done
https://review.coreboot.org/c/coreboot/+/32705/6/src/security/lockdown/lockd... PS6, Line 75: security_lockdown_bootmedia
This is calling a symbol that doesn't exist?
Done