On 11/16/2016 03:07 PM, Jason Gunthorpe wrote:
> On Wed, Nov 16, 2016 at 12:07:23PM -0500, Stefan Berger wrote:
>> The culprit seems to be 'tpm: fix the missing .owner in
>> tpm_bios_measurements_ops'
> That is unlikely, it is probably the patch before which calls read_log
> unconditionally now. That suggests the crashing is a little random..
I ran the vtpm driver test suite (with -j32) a few times at that patch
and it didn't crash. It crashes severely with later patches applied.
Here's the current experimental patch that fixes these problems:
iff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index 0cb43ef..a73295a 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -56,6 +56,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
log = &chip->log;
+ if (!chip->acpi_dev_handle)
+ return 0;
+
// So ACPI is not supported on this device, but ACPI support is compiled
in. I am returning 0 here, assuming it's not an OF device and the
corresponding OF function need not be called (see below).
/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
status = acpi_get_table(ACPI_SIG_TCPA, 1,
(struct acpi_table_header **)&buff);
diff --git a/drivers/char/tpm/tpm_eventlog.c
b/drivers/char/tpm/tpm_eventlog.c
index fb603a7..12b0356 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -380,7 +380,8 @@ static int tpm_read_log(struct tpm_chip *chip)
if ((rc == 0) || (rc == -ENOMEM))
return rc;
- rc = tpm_read_log_of(chip);
+ if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
+ rc = tpm_read_log_of(chip);
// I am not sure how to handle this case, in case we get here, which
would only be on an OF device (following 'return 0;' above), but we
don't want to attempt to read the log there, either. I think the most
straight-forward way would be to gate this whole function with a flag
that only the vtpm proxy driver has: TPM_CHIP_FLAG_NO_FIRMWARE_LOG.
return rc;
Stefan
>
> tpm_read_log_acpi should check if the chip has a acpi_dev_handle
> before running, but it also shouldn't crash - so there are two bugs
> here I guess.. Please do that instead of the checking the virual flag.
>
> Jarkko, you know acpi better, we switched ppi to search starting from
> the acpi_dev_handle for its data - can we do the same for event log?
>
>> The crash looks like this:
>> [ 173.608722] [<ffffffff8140ca11>] dump_stack+0x63/0x82
>> [ 173.608722] [<ffffffff8106b99f>] iounmap.part.1+0x7f/0x90
>> [ 173.608722] [<ffffffff8106b9dc>] iounmap+0x2c/0x30
>> [ 173.608722] [<ffffffff81496c75>] acpi_os_map_cleanup.part.10+0x31/0x3e
>> [ 173.608722] [<ffffffff8179629c>] acpi_os_unmap_iomem+0xcb/0xd2
>> [ 173.608722] [<ffffffffa00e1b28>] read_log+0xc8/0x19e [tpm]
> This seems really strange ACPI should not crash like this - yes it
> will wrongly read the log for the system into the vtpm, but that
> *should* work.
>
> Are you doing anything special with this test like high parallism or
> something? Any chance you can look at little more? The tpm code looks
> OK to me, the map and unmap are properly paired - but the bad address
> from the iounmap suggests the refcounting in acpi is not working or
> something along those lines?
>
> Jason
>
------------------------------------------------------------------------------
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
