tpm: efi: reject a final events table with an oversized preboot log
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi Jarkko,
No bus interposer is involved: the threat here is firmware shipping a self-contradictory final events table (preboot size larger than the table itself), not tampering on the wire. So the quantitative question - does this actually happen - does have data.
This area of efi.c has already needed fixes for real firmware tables four times:
- 05c8c1ff81ed ("efi/tpm: Don't traverse an event log with no events"): some machines return an unpopulated template table whose contents are just garbage.
- b4f1874c6216 ("tpm: check event log version before reading final events"): several Dell models froze at boot because the firmware event log and final events table did not match each other.
Fixes: includes 166a2809d65b.
- 80a6c216b16d ("tpm/eventlog: Don't abort tpm_read_log on faulty ACPI address"): a Framework laptop with an INSYDE BIOS shipped an un-mappable TPM2 ACPI table.
- 48cff270b037 ("tpm: efi: Use local variable for calculating final log size"): this one is the closest. The same subtraction went negative and hit __memcpy with a wrapped size, with a real oops in the commit message. It carries Fixes: 166a2809d65b and went to stable.
So the corrupted-table input and the negative-size memcpy path have both been observed in the wild, just not through this exact combination yet.
I'm fine either way on the tag. If you still consider this one hardening, I'll send a v3 without Fixes - the change itself is not affected by that.
Thanks.
Jarkko Sakkinen
maintainerQuoted 33 lines · Guo Zihao
Hi Jarkko,
No bus interposer is involved: the threat here is firmware shipping a self-contradictory final events table (preboot size larger than the table itself), not tampering on the wire. So the quantitative question - does this actually happen - does have data.
This area of
efi.chas already needed fixes for real firmware tables four times:-
05c8c1ff81ed("efi/tpm: Don't traverse an event log with no events"): some machines return an unpopulated template table whose contents are just garbage.-
b4f1874c6216("tpm: check event log version before reading final events"): several Dell models froze at boot because the firmware event log and final events table did not match each other.
Fixes: includes166a2809d65b.-
80a6c216b16d("tpm/eventlog: Don't aborttpm_read_logon faulty ACPI address"): a Framework laptop with an INSYDE BIOS shipped an un-mappable TPM2 ACPI table.-
48cff270b037("tpm: efi: Use local variable for calculating final log size"): this one is the closest. The same subtraction went negative and hit__memcpywith a wrapped size, with a real oops in the commit message. It carries Fixes:166a2809d65band went to stable.So the corrupted-table input and the negative-size memcpy path have both been observed in the wild, just not through this exact combination yet.
Quoted 5 lines
I'm fine either way on the tag. If you still consider this one hardening, I'll send a v3 without Fixes - the change itself is not affected by that.
Thanks.
OK, I think you are making a fair argument.
This is not just pure hardening given the various possibilities for pure software (firmware) mishaps.
Maybe v3 with a reminder of why this crossed the border?
That said, I think I agree with you now.
Br, Jarkko