"Kevin O'Connor" <kevin@koconnor.net> wrote on 11/20/2015 12:26:08 PM:

>
> On Fri, Nov 20, 2015 at 05:09:07PM +0000, Stefan Berger wrote:
> > "Kevin O'Connor" <kevin@koconnor.net> wrote on 11/20/2015 11:12:53 AM:
> > > I don't understand the error handling in tpm_extend_acpi_log() and
> > > tpm_extend().  Why does a log overflow in tpm_extend_acpi_log()
> > > shutdown the tpm chip (via tpm_set_failure() )?  In particular,
> > > tpm_extend_acpi_log can be called from clients via the 16bit BIOS
> > > interface, and it's the only way a client could cause the tpm chip to
> > > shutdown.  Why does tpm_extend() call reset_acpi_log() on failure?  It
> > > seems odd that a failure in communication with the TPM chip would
> > > result in an ACPI log reset - no other TPM chip failure does that.
> >
> > The intention here was to invalidate the log that is supposed to be
> > written along with PCR extensions, resulting in attestation not being
> > possible due to a failure in the extend.
>
> Okay - so there isn't a specific need to handle a failure in
> tpm_extend_acpi_log() differently from a failure in tpm_extend()?
> That is, the goal is just to prevent attestation after a failed
> request?

tpm_shutdown can be removed; tpm_extend could also call tpm_set_failure() instead of tpm_shutdown()

>
> > > Is it expected that the tcpa ACPI table could move or be modified at
> > > runtime?  The code rescans for the table twice on every call to
> > > tpm_extend_acpi_log() - if it can't move or be modified then I think
> > > it would be simpler to cache the values.
> >
> > I will have a look whether we can cache that. I guess for as long as it's
> > scanned for 'early' we are not write-protected, yet. You may have seen I
> > am also rescanning the log every time something is appended to it. Here
> > the reason was that we are running in ROM mode and I cannot write to
> > static variables anymore that would allow to set the pointer to the last
> > entry. So back when I wrote this I handled the ACPI table in the same way
> > - rescanning.
>
> Oh, so the code could assume that the tcpa table is not moved nor
> modified _and_ that the BIOS is also the only writer to the log?

Unless there is way that the bootloader could give back control to the BIOS, the BIOS would be the only writer to the log.

>
> BTW, to make a global variable read/writable at runtime, one can add
> the VARLOW attribute to the variable.

Ok.

Regards,
    Stefan