[PATCH] tpm_crb

* [PATCH] tpm_crb - workaround broken ACPI tables
@ 2019-07-11 12:29 Safford, David (GE Global Research, US)
  2019-07-11 14:10 ` Jarkko Sakkinen
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Safford, David (GE Global Research, US) @ 2019-07-11 12:29 UTC (permalink / raw)
  To: linux-integrity, jarkko.sakkinen, Wiseman,
	Monty (GE Global Research, US)

Most x86 desktops and laptops have firmware TPMs which support the
CRB interface. Unfortunately, the linux tpm_crb driver depends on
perfectly correct ACPI tables, and there are a *lot* of systems out
there with broken tpm_crb entries. (Not one of my five tpm_crb
systems works with the existing driver.) While it is good to
encourage vendors to fix their firmware, many refuse ("It works on
Windows"), leaving users in the lurch.

This patch adds a kernel parameter "tpm_crb.force=1" that works
around the problem in every case I have tested so far. Basically
it does two things:
	- it trusts the cmd and resp addresses in the CRB registers
	- it ignores all alleged IO resource conflicts

Both workarounds make sense. If there really were an address conflict,
or if the register values really were wrong, the device would not be
working at all. And testing with this patch has shown that in every
case (so far), the problem has been bogus ACPI entries.

This patch is against the upstream 5.2 kernel.

Signed-off-by: David Safford <david.safford@ge.com>

--- a/drivers/char/tpm/tpm_crb.c	2019-07-07 18:41:56.000000000 -0400
+++ b/drivers/char/tpm/tpm_crb.c	2019-07-10 11:12:37.269766833 -0400
@@ -108,6 +108,12 @@
 	u32 smc_func_id;
 };
 
+static bool force;
+#ifdef CONFIG_X86
+module_param(force, bool, 0444);
+MODULE_PARM_DESC(force, "Force register provided mappings to overcome broken acpi tables");
+#endif
+
 static bool crb_wait_for_reg_32(u32 __iomem *reg, u32 mask, u32 value,
 				unsigned long timeout)
 {
@@ -460,9 +466,12 @@
 	if (start != new_res.start)
 		return (void __iomem *) ERR_PTR(-EINVAL);
 
-	if (!resource_contains(io_res, &new_res))
-		return devm_ioremap_resource(dev, &new_res);
-
+	if (!resource_contains(io_res, &new_res)){
+		if (force)
+			return ioremap(start, size);
+		else
+			return devm_ioremap_resource(dev, &new_res);
+	}
 	return priv->iobase + (new_res.start - io_res->start);
 }
 
@@ -480,6 +489,11 @@
 	if (start + size - 1 <= io_res->end)
 		return size;
 
+	if (force) {
+		dev_err(dev, FW_BUG "ACPI cmd size broken. Using %llX\n", size);
+		return size;
+	}
+
 	dev_err(dev,
 		FW_BUG "ACPI region does not cover the entire command/response buffer. %pr vs %llx %llx\n",
 		io_res, start, size);
@@ -512,8 +526,11 @@
 		dev_err(dev, FW_BUG "TPM2 ACPI table does not define a memory resource\n");
 		return -EINVAL;
 	}
+	if (force)
+		priv->iobase = ioremap(io_res.start, io_res.end - io_res.start + 1);
+	else
+		priv->iobase = devm_ioremap_resource(dev, &io_res);
 
-	priv->iobase = devm_ioremap_resource(dev, &io_res);
 	if (IS_ERR(priv->iobase))
 		return PTR_ERR(priv->iobase);


^ permalink raw reply	[flat|nested] 17+ messages in thread