On Sun, Nov 20, 2016 at 09:46:25AM +0000, Jarkko Sakkinen wrote: > On Sat, Nov 19, 2016 at 11:22:28AM -0700, Jason Gunthorpe wrote: > > On Fri, Nov 18, 2016 at 07:52:49AM -0800, Jarkko Sakkinen wrote: > > > On Thu, Nov 17, 2016 at 07:30:04PM -0500, Stefan Berger wrote: > > > > tpm_bios_log_setup() may return -ENODEV in case no log was > > > > found. In this case we do not need to fail the device. > > > > > > > > Signed-off-by: Stefan Berger <[email protected]> > > > > drivers/char/tpm/tpm-chip.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > > > > index 3f27753..2d6530b 100644 > > > > +++ b/drivers/char/tpm/tpm-chip.c > > > > @@ -346,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip) > > > > tpm_sysfs_add_device(chip); > > > > > > > > rc = tpm_bios_log_setup(chip); > > > > - if (rc == -ENODEV) > > > > + if (rc != -ENODEV) > > > > return rc; > > > > > > > > tpm_add_ppi(chip); > > > > > > CC to linux-security-module > > > > > > LGTM > > > > > > Reviewed-by: Jarkko Sakkinen <[email protected]> > > > > Erm, what about rc == 0? And all the other problems? > > Sorry my bad. I was not thinking clearly. > > This whole situation looks like a mess. I gave a lot of thought on this > during my plane trips. > > > Here, use this (untested) should take care of everything on this > > topic.. > > > > The two things I haven't seen explained are the sysfs unregister crash > > and the acpi iounmap crash :/ > > Yup. The reason I'm not weighting that yet so much is that I do not know > the environment. > > > > > From 8768bcb8cd2a5a17cc4d811a9298b20c3a2c0884 Mon Sep 17 00:00:00 2001 > > From: Jason Gunthorpe <[email protected]> > > Date: Sat, 19 Nov 2016 11:18:28 -0700 > > Subject: [PATCH] tpm: Fix handling of missing event log > > > > The event log is an optional firmware feature, if the firmware > > does not support it then the securityfs files should not be created > > and no other notification given. > > > > - Uniformly return -ENODEV from the tpm_bios_log_setup cone if > > no event log is detected. > > - Check in ACPI if this node was discovered via ACPI. > > - Improve the check in OF to make sure there is a parent and to > > fail detection if the two log properties are not declared > > - Pass through all other error codes instead of filtering just some > > > > Signed-off-by: Jason Gunthorpe <[email protected]> > > --- > > drivers/char/tpm/tpm-chip.c | 2 +- > > drivers/char/tpm/tpm_acpi.c | 8 +++++++- > > drivers/char/tpm/tpm_eventlog.c | 26 +++++++++++++------------- > > drivers/char/tpm/tpm_of.c | 11 +++++------ > > 4 files changed, 26 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > > index 3f27753d96aab5..7a4869151d3b90 100644 > > --- a/drivers/char/tpm/tpm-chip.c > > +++ b/drivers/char/tpm/tpm-chip.c > > @@ -346,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip) > > tpm_sysfs_add_device(chip); > > > > rc = tpm_bios_log_setup(chip); > > - if (rc == -ENODEV) > > + if (rc != 0 && rc != -ENODEV) > > return rc; > > > > tpm_add_ppi(chip); > > diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c > > index 0cb43ef5f79a6e..99366bf64f3359 100644 > > --- a/drivers/char/tpm/tpm_acpi.c > > +++ b/drivers/char/tpm/tpm_acpi.c > > @@ -56,12 +56,18 @@ int tpm_read_log_acpi(struct tpm_chip *chip) > > > > log = &chip->log; > > > > + /* Unfortuntely ACPI does not associate the event log with a specific > > + * TPM, like PPI. Thus all ACPI TPMs will read the same log. > > + */ > > + if (!chip->acpi_dev_handle) > > + return -ENODEV; > > + > > /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ > > status = acpi_get_table(ACPI_SIG_TCPA, 1, > > (struct acpi_table_header **)&buff); > > > > if (ACPI_FAILURE(status)) > > - return -EIO; > > + return -ENODEV; > > > > switch(buff->platform_class) { > > case BIOS_SERVER: > > diff --git a/drivers/char/tpm/tpm_eventlog.c > > b/drivers/char/tpm/tpm_eventlog.c > > index fb603a74cbd29e..2a15b866ac257a 100644 > > --- a/drivers/char/tpm/tpm_eventlog.c > > +++ b/drivers/char/tpm/tpm_eventlog.c > > @@ -377,14 +377,21 @@ static int tpm_read_log(struct tpm_chip *chip) > > } > > > > rc = tpm_read_log_acpi(chip); > > - if ((rc == 0) || (rc == -ENOMEM)) > > + if (rc != -ENODEV) > > return rc; > > > > - rc = tpm_read_log_of(chip); > > - > > - return rc; > > + return tpm_read_log_of(chip); > > } > > > > +/* > > + * tpm_bios_log_setup() - Read the event log from the firmware > > + * @chip: TPM chip to use. > > + * > > + * If an event log is found then the securityfs files are setup to > > + * export it to userspace, otherwise nothing is done. > > + * > > + * Returns -ENODEV if the firmware has no event log. > > + */ > > int tpm_bios_log_setup(struct tpm_chip *chip) > > { > > const char *name = dev_name(&chip->dev); > > @@ -395,15 +402,8 @@ int tpm_bios_log_setup(struct tpm_chip *chip) > > return 0; > > > > rc = tpm_read_log(chip); > > - /* > > - * read_log failure means event log is not supported except for ENOMEM. > > - */ > > - if (rc < 0) { > > - if (rc == -ENOMEM) > > - return -ENODEV; > > - else > > - return rc; > > - } > > WTF. I really have to be much more focused when I looked this. That > is more than wrong... Too much multitasking last couple of weeks. That's > my excuse... > > I can consider putting the patch set to the next release but I really > would want yet another version with change log what fixes were done and > why.
I mean one more version of the patch set. Otherwise, I probably just postpone the whole patch set to 4.10. /Jarkko ------------------------------------------------------------------------------ _______________________________________________ tpmdd-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
