[Skiboot] [PATCH 3/3] libstb/tpm: block access to unknown i2c devs on the tpm bus

Oliver O'Halloran oohall at gmail.com
Tue Dec 3 16:46:19 AEDT 2019


Our favourite TPM is capable of listening on multiple I2C bus addresses
and although this feature is supposed to be disabled by default we have
some systems in the wild where the TPM appears to be listening on these
secondary addresses.

The secondary addresses are also susceptible to the bus-lockup problem
that we see with certain traffic patterns to the "main" TPM address.
We don't know what addresses the TPM might be listening on it's best to
take a conservitve approach and only allow traffic to I2C bus addresses
that we are explicitly told about by firmware.

This is only required on the TPM bus, so this patch extends the existing
TPM workaround to also check that a DT node exists for any I2C bus
address the OS wants to talk to. If there isn't one, we don't forward
the I2C request to the bus and return an I2C timeout error to the OS.

Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
---
 libstb/drivers/tpm_i2c_nuvoton.c | 47 +++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/libstb/drivers/tpm_i2c_nuvoton.c b/libstb/drivers/tpm_i2c_nuvoton.c
index ef32b79fd6f7..83ec1c81e485 100644
--- a/libstb/drivers/tpm_i2c_nuvoton.c
+++ b/libstb/drivers/tpm_i2c_nuvoton.c
@@ -493,12 +493,46 @@ static struct tpm_driver tpm_i2c_nuvoton_driver = {
 static int nuvoton_tpm_quirk(void *data, struct i2c_request *req, int *rc)
 {
 	struct tpm_dev *tpm_device = data;
+	struct dt_node *dev;
+	uint16_t addr;
+	bool found;
 
-	/* If we're doing i2cdetect on the TPM, pretent we just NACKed
-	 * it due to errata in nuvoton firmware where if we let this
-	 * request go through, it would steal the bus and you'd end up
-	 * in a nice world of pain.
+	/*
+	 * The nuvoton TPM firmware has a problem where a single byte read or
+	 * zero byte write to one of its I2C addresses causes the TPM to lock
+	 * up the bus. Once locked up the bus can only be recovered by power
+	 * cycling the TPM. Unfortunately, we don't have the ability to
+	 * power cycle the TPM because allowing it to be reset at runtime
+	 * would undermine the TPM's security model (we can reset it and
+	 * send it whatever measurements we like to unlock it's secrets).
+	 * So the best we can do here is try avoid triggering the problem
+	 * in the first place.
+	 *
+	 * For a bit of added fun the TPM also appears to check for traffic
+	 * on a few different I2C bus addresses. It does this even when not
+	 * configured to respond on those addresses so you can trigger the
+	 * bug by sending traffic... somwhere. To work around this we block
+	 * sending I2C requests on the TPM's bus unless the DT explicitly
+	 * tells us there is a device there.
 	 */
+
+	/* first, check if this a known address */
+	addr = req->dev_addr;
+	found = false;
+
+	dt_for_each_child(req->bus->dt_node, dev) {
+		if (dt_prop_get_u32(dev, "reg") == addr) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		*rc = OPAL_I2C_TIMEOUT;
+		return 1;
+	}
+
+	/* second, check if it's a bad transaction to the TPM */
 	if (tpm_device->bus_id == req->bus->opal_id &&
 	    tpm_device->i2c_addr == req->dev_addr &&
 	    ((req->op == I2C_READ && req->rw_len == 1) ||
@@ -517,6 +551,7 @@ void tpm_i2c_nuvoton_probe(void)
 	struct tpm_dev *tpm_device = NULL;
 	struct dt_node *node = NULL;
 	struct i2c_bus *bus;
+	const char *name;
 
 	dt_for_each_compatible(dt_root, node, "nuvoton,npct650") {
 		if (!dt_node_is_enabled(node))
@@ -562,6 +597,10 @@ void tpm_i2c_nuvoton_probe(void)
 		assert(bus->check_quirk == NULL);
 		bus->check_quirk = nuvoton_tpm_quirk;
 		bus->check_quirk_data = tpm_device;
+		name = dt_prop_get(node, "ibm,port-name");
+
+		prlog(PR_NOTICE, "NUVOTON: TPM I2C workaround applied to %s\n",
+				  name);
 
 		/*
 		 * Tweak for linux. It doesn't have a driver compatible
-- 
2.21.0



More information about the Skiboot mailing list