Backport 25984 ([rom] Examine the immutable section and skip if invalid) #28798
Conversation
pamaury
requested review from
engdoreis,
jwnrt and
timothytrippel
and removed request for
a team
pamaury
force-pushed
the
backport_25984
branch
3 times, most recently
from
6888d95 to
d0aef9f
Compare
jwnrt
left a comment
jwnrt
left a comment
There was a problem hiding this comment.
Approving the backport but have some concerns about the use of kHardenedBoolFalse in a pointer.
| static rom_error_t rom_verify_immutable_section( | ||
| rom_error_t verify_result, const manifest_t *manifest, | ||
| uintptr_t *imm_section_entry_point) { | ||
| *imm_section_entry_point = kHardenedBoolFalse; |
There was a problem hiding this comment.
I'm not entirely comfortable with using a sentinal value in the middle of a pointer, although I don't think that the current value of kHardenedBoolFalse is a valid address.
I would prefer to have a separate valid boolean outside of the address if possible.
There was a problem hiding this comment.
The reason to use the bitpattern kHardenedBoolFalse is to avoid using an all-zeros pattern to represent the same kind of nothing that NULL would represent. The all-zeros and all-ones patterns are relatively easy for an attacker to create as a glitch, whereas kHardenedBoolFalse is comparatively difficult.
That it is not a valid address is kind-of the point: an access to that address will result in a fault. This is exactly the same thing that would happen attempting to access everyone else's favorite invalid address (NULL).
The pattern of using an alternate to NULL is common in much of the ROM and ROM_EXT code.
Add tests that check if a valid signed ROM_EXT with an invalid immutable section can boot. The correct behavior is that the ROM should determine that the immutable section is invalid and try the other slot. Signed-off-by: Chris Frantz <[email protected]> (cherry picked from commit f17d30f)
1. Refactor immutable section verification into a separate function. 2. When booting, examine the immutable section and fail the boot with an error if the immutable section is invalid. If the failure occurs in the primary slot, the ROM should attempt to boot the secondary slot. Signed-off-by: Chris Frantz <[email protected]> (cherry picked from commit 1561505)
pamaury
force-pushed
the
backport_25984
branch
from
d0aef9f to
23e236d
Compare
3 participants
Backport #25984.
Note: several conflicts due to the fact that #26427 was applied after #25984 on earlgrey_1.0.0 but backported to master in #27133 already