Re: [PATCH] tpm: efi: reject a final events table with an oversized preboot log

[Jarkko Sakkinen]

On Mon, Nov 09, 2026 at 04:43:23PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 09, 2026 at 09:49:34AM +0800, Guo Zihao wrote:
> > On Fri, Nov 06, 2026 at 03:24:39PM +0000, Jarkko Sakkinen wrote:
> > > Ditto.
> > >
> > > For all four address validation vs bug fix intersection. I did not
> > > check throughly enough to be exactly sure of all four.
> > 
> > For this one I'd like to argue the bug fix case, since the data being
> > validated is not something the TPM reports over a TCG-defined
> > protocol: both sizes come from EFI configuration tables written by
> > firmware, and nothing in the TCG spec constrains what firmware puts
> > there.
> > 
> > The firmware data is already treated as untrusted in the same
> > function: it bails out on an empty log (log_tbl->size == 0), on a
> > missing final log address, and on a wrong log format version. The
> > unchecked subtraction is the odd one out.
> > 
> > The consequence is also plain memory corruption rather than a
> > graceful rejection: final_events_log_size goes negative, the
> > following devm_krealloc() gets a wrapped size, and the memcpy()
> > appending the final events log runs with a negative length.
> > 
> > And the out-of-bounds write was introduced by 166a2809d65b ("tpm:
> > Don't duplicate events from the final event log in the TCG2 log"):
> > before that commit nothing subtracted the preboot size, so a
> > malformed table was harmless. The Fixes: tag refers to that
> > regression, not to the malformed table itself.
> > 
> > Ard also had a look at v2 and only asked for a pr_warn() on the
> > rejection path, which is in.
> > 
> > So I'd like to keep the Fixes: tag here, but I'm happy to drop it if
> > you disagree - your tree, your call.
> > 
> > Thanks.
> 
> Let's take quantitative approach instead.
> 
> OK, so does this actually happen? I'm weighting this based on that
> mainly. Usually bus corruption is not very common when it comes to
> TPMs because they are usually either soldered or in the firmware.
> 
> Is there something I'm missing out? I don't think bus interposer
> type of security scenario is common enough to use it as a rationale.

I do think that the change is for better. I'm disregarding its value
by saying this :-)

There's so many bug incoming that we need to be just somewhat rigid on
what we consider as a bug fix per se and what not.

Br, Jarkko