linux-kernel - Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 18 Dec 2017 13:29:01 +0100
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        Jason Gunthorpe <jgg@...pe.ca>
Cc:     James Ettle <james@...le.org.uk>, linux-integrity@...r.kernel.org,
        azhar.shaikh@...el.com, linux-kernel@...r.kernel.org,
        james.l.morris@...cle.com
Subject: Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on
 Braswell system

On 12/18/2017 01:22 PM, Javier Martinez Canillas wrote:

[snip]

> 
> James,
> 
> Can you please test the following (untested) patch on top of the other two
> mentioned patches to see if it makes a difference for you?
> 

I should had tried to at least compile the patch :)

Updated patch below:

>From 370d45a34dc8914066a995a3a6d6df1953ea9f60 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@...hat.com>
Date: Mon, 18 Dec 2017 12:56:28 +0100
Subject: [PATCH v2] tpm: only attempt to disable the LPC CLKRUN if is already
 enabled

Commit 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
added logic in the TPM TIS driver to disable the Low Pin Count CLKRUN
signal during TPM transactions.

Unfortunately this breaks other devices that are attached to the LPC bus
like for example PS/2 mouse and keyboards.

One flaw with the logic is that it assumes that the CLKRUN is always
enabled, and so it unconditionally enables it after a TPM transaction.

But it could be that the CLKRUN signal was already disabled in the LPC
bus and so after the driver probes, the signal will remain enabled which
may break other devices transactions since the clocks will be restarted
by the CLKRUN# signal.

Fixes: 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
---
 drivers/char/tpm/tpm_tis_core.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index e7bd2e750f69..5f2b1fc2194f 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -688,7 +688,8 @@ static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value)
 	struct tpm_tis_data *data = dev_get_drvdata(&chip->dev);
 	u32 clkrun_val;
 
-	if (!IS_ENABLED(CONFIG_X86) || !is_bsw())
+	if (!IS_ENABLED(CONFIG_X86) || !is_bsw() ||
+	    !data->ilb_base_addr)
 		return;
 
 	if (value) {
@@ -746,7 +747,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		      const struct tpm_tis_phy_ops *phy_ops,
 		      acpi_handle acpi_dev_handle)
 {
-	u32 vendor, intfcaps, intmask;
+	u32 vendor, intfcaps, intmask, clkrun_val;
 	u8 rid;
 	int rc, probe;
 	struct tpm_chip *chip;
@@ -772,6 +773,14 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 					ILB_REMAP_SIZE);
 		if (!priv->ilb_base_addr)
 			return -ENOMEM;
+
+		clkrun_val = ioread32(priv->ilb_base_addr + LPC_CNTRL_OFFSET);
+		/* Check if CLKRUN# is already not enabled in the LPC bus */
+		if (!(clkrun_val & LPC_CLKRUN_EN)) {
+			priv->flags |= TPM_TIS_CLK_ENABLE;
+			iounmap(priv->ilb_base_addr);
+			priv->ilb_base_addr = NULL;
+		}
 	}
 
 	if (chip->ops->clk_enable != NULL)
@@ -868,7 +877,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	}
 
 	rc = tpm_chip_register(chip);
-	if (rc && is_bsw())
+	if (rc && is_bsw() && priv->ilb_base_addr)
 		iounmap(priv->ilb_base_addr);
 
 	if (chip->ops->clk_enable != NULL)
-- 
2.14.3