From patchwork Sat Nov 19 18:22:28 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Gunthorpe X-Patchwork-Id: 696874 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.sourceforge.net (lists.sourceforge.net [216.34.181.88]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tLjs31qzrz9t1P for ; Sun, 20 Nov 2016 05:22:47 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=sfs-ml-3.v29.ch3.sourceforge.com) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1c8AHj-0003J3-Pv; Sat, 19 Nov 2016 18:22:43 +0000 Received: from sog-mx-2.v43.ch3.sourceforge.com ([172.29.43.192] helo=mx.sourceforge.net) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1c8AHi-0003Iy-HG for tpmdd-devel@lists.sourceforge.net; Sat, 19 Nov 2016 18:22:42 +0000 Received-SPF: pass (sog-mx-2.v43.ch3.sourceforge.com: domain of obsidianresearch.com designates 184.70.90.242 as permitted sender) client-ip=184.70.90.242; envelope-from=jgunthorpe@obsidianresearch.com; helo=quartz.orcorp.ca; Received: from quartz.orcorp.ca ([184.70.90.242]) by sog-mx-2.v43.ch3.sourceforge.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) id 1c8AHe-0007iP-LF for tpmdd-devel@lists.sourceforge.net; Sat, 19 Nov 2016 18:22:42 +0000 Received: from jgg by quartz.orcorp.ca with local (Exim 4.84_2) (envelope-from ) id 1c8AHV-0006Rj-3e; Sat, 19 Nov 2016 11:22:29 -0700 Date: Sat, 19 Nov 2016 11:22:28 -0700 From: Jason Gunthorpe To: Jarkko Sakkinen Message-ID: <20161119182228.GA22775@obsidianresearch.com> References: <1479429004-7962-1-git-send-email-stefanb@linux.vnet.ibm.com> <1479429004-7962-2-git-send-email-stefanb@linux.vnet.ibm.com> <20161118155249.sdxp2qfjfzfw4tzt@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161118155249.sdxp2qfjfzfw4tzt@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -1.6 (-) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1c8AHe-0007iP-LF Cc: linux-security-module@vger.kernel.org, tpmdd-devel@lists.sourceforge.net Subject: Re: [tpmdd-devel] [PATCH 2/2] tpm: Fix error code handling after tpm_bios_log_setup X-BeenThere: tpmdd-devel@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list List-Id: Tpm Device Driver maintainance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces@lists.sourceforge.net 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 > > 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 Erm, what about rc == 0? And all the other problems? 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 :/ >From 8768bcb8cd2a5a17cc4d811a9298b20c3a2c0884 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe 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 Signed-off-by: Jason Gunthorpe Tested-by: Stefan Berger --- 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; - } + if (rc) + return rc; cnt = 0; chip->bios_dir[cnt] = securityfs_create_dir(name, NULL); diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c index 36df9df4c472b9..7dee42d7b5e05c 100644 --- a/drivers/char/tpm/tpm_of.c +++ b/drivers/char/tpm/tpm_of.c @@ -29,13 +29,16 @@ int tpm_read_log_of(struct tpm_chip *chip) struct tpm_bios_log *log; log = &chip->log; - if (chip->dev.parent->of_node) + if (chip->dev.parent && chip->dev.parent->of_node) np = chip->dev.parent->of_node; else return -ENODEV; sizep = of_get_property(np, "linux,sml-size", NULL); - if (sizep == NULL) + basep = of_get_property(np, "linux,sml-base", NULL); + if (sizep == NULL && basep == NULL) + return -ENODEV; + if (sizep == NULL || basep == NULL) return -EIO; if (*sizep == 0) { @@ -43,10 +46,6 @@ int tpm_read_log_of(struct tpm_chip *chip) return -EIO; } - basep = of_get_property(np, "linux,sml-base", NULL); - if (basep == NULL) - return -EIO; - log->bios_event_log = kmalloc(*sizep, GFP_KERNEL); if (!log->bios_event_log) return -ENOMEM;