Attention is currently required from: Felix Singer, Julius Werner, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56343 )
Change subject: security/lockdown: Allow to lock the controller's full address space ......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Sorry, I don't really understand what this does. […]
You are probably right.
It's mostly simplifying and precaution, yes. The current code is too convoluted compared to the kind of security we are used to (originally maybe 5 lines in FILO). It seems (I only had a quick look at the code) that CONFIG_ROM_SIZE is taken into account. What happens if that Kconfig is not correct? What happens if the bootmedia code changes and some other mechanism is used? We would catch errors in QA, but the whole thing is just easy to break. We also need to check in FILO if the flash is protected. This can be a one-liner if the whole address space is protected or something convoluted if we need to know the flash size there too.
The whole thing is a workaround for FSP btw. I guess of the triple (Intel FSP, security, upstream) one has to pick two, but can't have it all; unless one has enough leverage ofc. (I guess it was no coincidence that FSP stopped evolving in the coreboot/security direction once it was vboot compatible ^^).
I see two options: 1. Just turn the LOCK_WHOLE_RO behavior into what we need; i.e. not taking the flash size into account. 2. keep our solution downstream or move it into the mainboard ports.