Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32705 )
Change subject: security/lockdown: Write-protect WP_RO ......................................................................
Patch Set 6:
(4 comments)
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
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?
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. You should have a helper that fills out a region_device based on the Kconfig:
static int helper(struct region_device *rdev) { if (CONFIG(BOOTMEDIA_LOCK_RO) || CONFIG(BOOTMEDIA_LOCK_NO_ACCESS)) { const struct region_device *boot_rdev = bootdevice_ro(); return rdev_chain_full(rdev, boot_rdev); } else if (CONFIG(BOOTMEDIA_LOCK_VBOOT_RO)) return fmap_locate_area_as_rdev("WP_RO", rdev); }
return -1; }
And add another helper for going through the wp_prot sequence for ones you care about:
static int another_helper(const struct region_device *rdev, const int *wp_prot, size_t wp_num) { for (size_t i = 0; i < wp_num; i++) { boot_device_wp_region(rdev, wp_prot[i]) } }
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?