[PATCH v3 0/2] Add driver for Xaptum ENF Access card (XAP-EA-00x)

[PATCH v3 0/2] Add driver for Xaptum ENF Access card (XAP-EA-00x)

[David R. Bild]

This series add a driver for the Xaptum ENF Access card line
(XAP-EA-00x), a series of mini PCI-e cards containing a TPM 2.0 chip
used to authenticate IoT devices and gateways.

The hardware is essentially a USB-SPI bridge and an SPI TPM 2.0
chip. The first patch registers the bridge as an SPI controller and
the TPM as an SPI device. The second patch performs the TPM platform
initialization that would normally be done by the BIOS.

The TPM portions need review from the TPM maintainers, before Greg can
accept it in the USB tree.

Changes since v2:
 * Depend on (not select) SPI and TPM support
 * Remove noisy logging
 * Do not explicitly specify MODULE_ALIAS
 * Use KBUILD_MODNAME macro for module name
 * Clean up formatting & indentation

David R. Bild (2):
  usb: misc: xapea00x: add driver for Xaptum ENF Access Card
  usb: misc: xapea00x: perform platform initialization of TPM

 MAINTAINERS                                 |   6 +
 drivers/usb/misc/Kconfig                    |   2 +
 drivers/usb/misc/Makefile                   |   1 +
 drivers/usb/misc/xapea00x/Kconfig           |  14 +
 drivers/usb/misc/xapea00x/Makefile          |   8 +
 drivers/usb/misc/xapea00x/xapea00x-bridge.c | 380 +++++++++++
 drivers/usb/misc/xapea00x/xapea00x-core.c   | 433 +++++++++++++
 drivers/usb/misc/xapea00x/xapea00x-spi.c    | 196 ++++++
 drivers/usb/misc/xapea00x/xapea00x-tpm.c    | 952 ++++++++++++++++++++++++++++
 drivers/usb/misc/xapea00x/xapea00x.h        |  75 +++
 10 files changed, 2067 insertions(+)
 create mode 100644 drivers/usb/misc/xapea00x/Kconfig
 create mode 100644 drivers/usb/misc/xapea00x/Makefile
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-bridge.c
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-core.c
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-spi.c
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-tpm.c
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x.h

-- 
2.16.3

[PATCH v3 1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card

[David R. Bild]

This commit adds a driver for the Xaptum ENF Access Card, a TPM2.0
hardware module for authenticating IoT devices and gateways.

The card consists of a SPI TPM 2.0 chip and a USB-SPI bridge. This
driver configures the bridge, registers the bridge as an SPI
controller, and adds the TPM 2.0 as an SPI device.  The in-kernel TPM
2.0 driver is then automatically loaded to configure the TPM and
expose it to userspace.

Signed-off-by: David R. Bild <david.bild@xaptum.com>
---
 MAINTAINERS                                 |   6 +
 drivers/usb/misc/Kconfig                    |   2 +
 drivers/usb/misc/Makefile                   |   1 +
 drivers/usb/misc/xapea00x/Kconfig           |  14 +
 drivers/usb/misc/xapea00x/Makefile          |   7 +
 drivers/usb/misc/xapea00x/xapea00x-bridge.c | 380 ++++++++++++++++++++++++++
 drivers/usb/misc/xapea00x/xapea00x-core.c   | 408 ++++++++++++++++++++++++++++
 drivers/usb/misc/xapea00x/xapea00x-spi.c    | 196 +++++++++++++
 drivers/usb/misc/xapea00x/xapea00x.h        |  75 +++++
 9 files changed, 1089 insertions(+)
 create mode 100644 drivers/usb/misc/xapea00x/Kconfig
 create mode 100644 drivers/usb/misc/xapea00x/Makefile
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-bridge.c
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-core.c
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-spi.c
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x.h

diff --git a/MAINTAINERS b/MAINTAINERS
index b1ccabd0dbc3..77d35444ef1c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14796,6 +14796,12 @@ L:	linux-wireless@vger.kernel.org
 S:	Maintained
 F:	drivers/net/wireless/rndis_wlan.c
 
+USB XAPEA00X DRIVER
+M:	David R. Bild <david.bild@xaptum.com>
+L:	linux-usb@vger.kernel.org
+S:	Maintained
+F:	drivers/usb/misc/xapea00x/
+
 USB XHCI DRIVER
 M:	Mathias Nyman <mathias.nyman@intel.com>
 L:	linux-usb@vger.kernel.org
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 68d2f2cd17dd..747d7f03fb84 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -275,3 +275,5 @@ config USB_CHAOSKEY
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called chaoskey.
+
+source "drivers/usb/misc/xapea00x/Kconfig"
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 109f54f5b9aa..f3583501547c 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_USB_HSIC_USB4604)		+= usb4604.o
 obj-$(CONFIG_USB_CHAOSKEY)		+= chaoskey.o
 
 obj-$(CONFIG_USB_SISUSBVGA)		+= sisusbvga/
+obj-$(CONFIG_USB_XAPEA00X)		+= xapea00x/
 obj-$(CONFIG_USB_LINK_LAYER_TEST)	+= lvstest.o
diff --git a/drivers/usb/misc/xapea00x/Kconfig b/drivers/usb/misc/xapea00x/Kconfig
new file mode 100644
index 000000000000..bea57c8cca53
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/Kconfig
@@ -0,0 +1,14 @@
+config USB_XAPEA00X
+	tristate "Xaptum ENF Access card support (XAP-EA-00x)"
+	depends on USB_SUPPORT && SPI && TCG_TPM
+	select TCG_TIS_SPI
+	help
+	  Say Y here if you want to support the Xaptum ENF Access
+	  modules (XAP-EA-00x) in the USB or Mini PCI-e form
+	  factors. The XAP-EA-00x module exposes a TPM 2.0 as
+	  /dev/tpmX to use for authenticating with the Xaptum ENF.
+
+	  To compile this driver as a module, choose M here. The
+	  module will be called xapea00x.
+
+	  If unsure, say M.
diff --git a/drivers/usb/misc/xapea00x/Makefile b/drivers/usb/misc/xapea00x/Makefile
new file mode 100644
index 000000000000..c4bcd7524c31
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the xapea00x driver.
+#
+obj-$(CONFIG_USB_XAPEA00X) += xapea00x.o
+
+xapea00x-y += xapea00x-core.o xapea00x-bridge.o
diff --git a/drivers/usb/misc/xapea00x/xapea00x-bridge.c b/drivers/usb/misc/xapea00x/xapea00x-bridge.c
new file mode 100644
index 000000000000..7071431dea96
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-bridge.c
@@ -0,0 +1,380 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ *  TPM 2.0-based hardware module for authenticating IoT devices and
+ *  gateways.
+ *
+ *  Copyright (c) 2017-2018 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+#define XAPEA00X_BR_CMD_READ			0x00
+#define XAPEA00X_BR_CMD_WRITE			0x01
+#define XAPEA00X_BR_CMD_WRITEREAD		0x02
+
+#define XAPEA00X_BR_BREQTYP_SET		0x40
+
+#define XAPEA00X_BR_BREQ_SET_GPIO_VALUES	0x21
+#define XAPEA00X_BR_BREQ_SET_GPIO_CS		0x25
+#define XAPEA00X_BR_BREQ_SET_SPI_WORD		0x31
+
+#define XAPEA00X_BR_USB_TIMEOUT		1000 // msecs
+
+#define XAPEA00X_BR_CS_DISABLED		0x00
+
+/*******************************************************************************
+ * Bridge USB transfers
+ */
+
+struct xapea00x_br_bulk_command {
+	__u16  reserved1;
+	__u8   command;
+	__u8   reserved2;
+	__le32 length;
+} __attribute__((__packed__));
+
+/**
+ * xapea00x_br_prep_bulk_command - Prepares the bulk command header with
+ * the supplied values.
+ * @hdr: pointer to header to prepare
+ * @command: the command id for the command
+ * @length: length in bytes of the command data
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static void xapea00x_br_prep_bulk_command(struct xapea00x_br_bulk_command *hdr,
+					  u8 command, int length)
+{
+	hdr->reserved1 = 0;
+	hdr->command   = command;
+	hdr->reserved2 = 0;
+	hdr->length    = __cpu_to_le32(length);
+}
+
+/**
+ * xapea00x_br_bulk_write - Issues a bulk write to the bridge chip.
+ * @dev: pointer to the device to write to
+ * @command: the command started by this write (WRITE, READ, WRITE_READ)
+ * @data: pointer to the data to write. Must be DMA capable (e.g.,
+ *	  kmalloc-ed, not stack).
+ * @len: length in bytes of the data to write
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_br_bulk_write(struct xapea00x_device *dev,
+				  struct xapea00x_br_bulk_command *header,
+				  const void *data, int len)
+{
+	u8 *buf;
+	unsigned int pipe;
+	int buf_len, actual_len, retval;
+
+	buf_len = sizeof(struct xapea00x_br_bulk_command) + len;
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf) {
+		retval = -ENOMEM;
+		goto out;
+	}
+
+	memcpy(buf, header, sizeof(struct xapea00x_br_bulk_command));
+	memcpy(buf + sizeof(struct xapea00x_br_bulk_command), data, len);
+
+	pipe = usb_sndbulkpipe(dev->udev, dev->bulk_out->bEndpointAddress);
+	retval = usb_bulk_msg(dev->udev, pipe, buf, buf_len, &actual_len,
+			      XAPEA00X_BR_USB_TIMEOUT);
+	if (retval)
+		goto free_buf;
+
+free_buf:
+	kzfree(buf);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_br_bulk_read - Issues a bulk read to the bridge chip.
+ * @dev: pointer to the device to read from
+ * @data: pointer to the data read. Must be DMA capable (e.g.,
+ *	  kmalloc-ed, not stack).
+ * @len: length in bytes of the data to read
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_br_bulk_read(struct xapea00x_device *dev, void *data,
+				 int len)
+{
+	unsigned int pipe;
+	void *buf;
+	int actual_len, retval;
+
+	buf = kzalloc(len, GFP_KERNEL);
+	if (!buf) {
+		retval = -ENOMEM;
+		goto out;
+	}
+
+	pipe = usb_rcvbulkpipe(dev->udev, dev->bulk_in->bEndpointAddress);
+	retval = usb_bulk_msg(dev->udev, pipe, buf, len, &actual_len,
+			      XAPEA00X_BR_USB_TIMEOUT);
+
+	if (retval)
+		goto free_buf;
+
+	memcpy(data, buf, actual_len);
+
+free_buf:
+	kzfree(buf);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_br_ctrl_write - Issues a send control transfer to the bridge
+ * chip.
+ * @dev: pointer to the device to write to
+ * @bRequest: the command
+ * @wValue: the command value
+ * @wIndex: the command index
+ * @data: pointer to the command data
+ * @len: length in bytes of the command data
+ *
+ * The possible bRequest, wValue, and wIndex values and data format
+ * are specified in the hardware datasheet.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_br_ctrl_write(struct xapea00x_device *dev, u8 bRequest,
+				  u16 wValue, u16 wIndex, u8 *data, u16 len)
+{
+	unsigned int pipe;
+	void *buf;
+	int retval;
+
+	buf = kzalloc(len, GFP_KERNEL);
+	if (!buf) {
+		retval = -ENOMEM;
+		goto out;
+	}
+	memcpy(buf, data, len);
+
+	pipe = usb_sndctrlpipe(dev->udev, 0);
+	retval = usb_control_msg(dev->udev, pipe, bRequest,
+				 XAPEA00X_BR_BREQTYP_SET, wValue, wIndex,
+				 buf, len, XAPEA00X_BR_USB_TIMEOUT);
+	if (retval < 0)
+		goto free_buf;
+
+	retval = 0;
+
+free_buf:
+	kzfree(buf);
+
+out:
+	return retval;
+}
+
+/*******************************************************************************
+ * Bridge configuration commands
+ */
+
+/**
+ * xapea00x_br_set_gpio_value - Sets the value on the specified pin of
+ * the bridge chip.
+ * @dev: pointer to the device containing the bridge whose pin to set
+ * @pin: the number of the pin to set
+ * @value: the value to set the pin to, 0 or 1
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_br_set_gpio_value(struct xapea00x_device *dev, u8 pin,
+				      u8 value)
+{
+	u8 data[4] = { 0, 0, 0, 0 };
+
+	switch (pin) {
+	case 10:
+	case  9:
+	case  8:
+	case  7:
+	case  6:
+		data[0] = value << (pin - 4);
+		data[2] = 1 << (pin - 4);
+		break;
+	case  5:
+		data[0] = value;
+		data[2] = 1;
+		break;
+	case  4:
+	case  3:
+	case  2:
+	case  1:
+	case  0:
+		data[1] = value << (pin + 3);
+		data[3] = 1 << (pin + 3);
+		break;
+	}
+	return xapea00x_br_ctrl_write(dev, XAPEA00X_BR_BREQ_SET_GPIO_VALUES,
+				      0, 0, data, 4);
+}
+
+/**
+ * xapea00x_br_set_gpio_cs - Sets the chip select control on the specified
+ * pin of the bridge chip.
+ * @dev: pointer to the device containing the bridge whose cs to set
+ * @pin: the number of the pin to set
+ * @control: the chip select control value for the pin, 0, 1, or 2
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_br_set_gpio_cs(struct xapea00x_device *dev, u8 pin,
+				   u8 control)
+{
+	u8 data[2] = { pin, control };
+
+	return xapea00x_br_ctrl_write(dev, XAPEA00X_BR_BREQ_SET_GPIO_CS,
+				      0, 0, data, 2);
+}
+
+/*******************************************************************************
+ * Bridge configuration commands
+ */
+/**
+ * xapea00x_br_disable_cs - disable the built-in chip select
+ * capability of the specified channel. It does not support holding
+ * the CS active between SPI transfers, a feature required for the
+ * TPM. Instead, we manually control the CS pin as a GPIO.
+ * @dev: pointer to the device containing the bridge whose cs to disable
+ * @channel: the SPI channel whose cs to disable
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful 0. Otherwise a negative error number.
+ */
+int xapea00x_br_disable_cs(struct xapea00x_device *dev, u8 channel)
+{
+	return xapea00x_br_set_gpio_cs(dev, channel,
+				       XAPEA00X_BR_CS_DISABLED);
+}
+
+/**
+ * xapea00x_br_assert_cs - assert the chip select pin for the
+ * specified channel.
+ * @dev: pointer to the device containing the bridge who cs to assert
+ * @channel: the SPI channel whose cs to assert
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful 0. Otherwise a negative error number.
+ */
+int xapea00x_br_assert_cs(struct xapea00x_device *dev, u8 channel)
+{
+	return xapea00x_br_set_gpio_value(dev, channel, 0);
+}
+
+/**
+ * xapea00x_br_deassert_cs - deassert the chip select pin for the
+ * specified channel.
+ * @dev: pointer to the device containing the bridge who cs to deassert
+ * @channel: the SPI channel whose cs to deassert
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful 0. Otherwise a negative error number.
+ */
+int xapea00x_br_deassert_cs(struct xapea00x_device *dev, u8 channel)
+{
+	return xapea00x_br_set_gpio_value(dev, channel, 1);
+}
+
+/*******************************************************************************
+ * Bridge SPI reads and writes
+ */
+/**
+ * xeapea00x_spi_read - Performs a read from the active channel
+ * @dev: pointer to the device to perform the read
+ * @rx_buf: pointer to the buffer to read the data into.  Must be
+ *	    DMA-capable (e.g., kmalloc-ed, not stack).
+ * @len: length in bytes of the data to read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_br_spi_read(struct xapea00x_device *dev, void *rx_buf, int len)
+{
+	struct xapea00x_br_bulk_command header;
+	int retval;
+
+	xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_READ, len);
+	retval = xapea00x_br_bulk_write(dev, &header, NULL, 0);
+	if (retval)
+		goto out;
+
+	retval = xapea00x_br_bulk_read(dev, rx_buf, len);
+
+out:
+	return retval;
+}
+
+/**
+ *xapea00x_br_spi_write - Performs a write to the active channel
+ * @dev: pointer to the device to perform the write
+ * @tx_buf: pointer to the data to write. Must be DMA-capable (e.g.,
+ *	    kmalloc-ed, not stack).
+ * @len: length in bytes of the data to write
+ */
+int xapea00x_br_spi_write(struct xapea00x_device *dev, const void *tx_buf,
+			  int len)
+{
+	struct xapea00x_br_bulk_command header;
+	int retval;
+
+	xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_WRITE, len);
+	retval = xapea00x_br_bulk_write(dev, &header, tx_buf, len);
+
+	return retval;
+}
+
+/**
+ * xapea00x_br_spi_write_read - Performs a simultaneous write and read on
+ * the active channel
+ * @dev: pointer to the device to perform the write/read
+ * @tx_buf: pointer to the data to write. Must be DMA-capable (e.g.,
+ *	    kmalloc-ed, not stack).
+ * @rx_buf: pointer to the buffer to read the data into. Must be
+ *	    DMA-capable (e.g., kmalloc-ed, not stack).
+ * @len: length in bytes of the data to write/read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_br_spi_write_read(struct xapea00x_device *dev, const void *tx_buf,
+			       void *rx_buf, int len)
+{
+	struct xapea00x_br_bulk_command header;
+	int retval;
+
+	xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_WRITEREAD, len);
+	retval = xapea00x_br_bulk_write(dev, &header, tx_buf, len);
+	if (retval)
+		goto out;
+
+	retval = xapea00x_br_bulk_read(dev, rx_buf, len);
+
+out:
+	return retval;
+}
diff --git a/drivers/usb/misc/xapea00x/xapea00x-core.c b/drivers/usb/misc/xapea00x/xapea00x-core.c
new file mode 100644
index 000000000000..885bcda9c01d
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-core.c
@@ -0,0 +1,408 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ *  TPM 2.0-based hardware module for authenticating IoT devices and
+ *  gateways.
+ *
+ *  Copyright (c) 2017-2018 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+#define XAPEA00X_TPM_MODALIAS	"tpm_tis_spi"
+
+#define kref_to_xapea00x(k) container_of(k, struct xapea00x_device, kref)
+
+static void xapea00x_delete(struct kref *kref)
+{
+	struct xapea00x_device *dev = kref_to_xapea00x(kref);
+
+	usb_put_dev(dev->udev);
+	kfree(dev);
+}
+
+/*******************************************************************************
+ * SPI master functions
+ */
+
+/**
+ * xapea00x_spi_setup - Setup the SPI channel for the TPM.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_setup(struct spi_device *spi)
+{
+	struct xapea00x_device *dev;
+	int retval;
+
+	dev = spi_master_get_devdata(spi->master);
+
+	mutex_lock(&dev->usb_mutex);
+	if (!dev->interface) {
+		retval = -ENODEV;
+		goto out;
+	}
+
+	/* Verify that this is the TPM device */
+	if (spi->chip_select != 0) {
+		retval = -EINVAL;
+		goto err;
+	}
+
+	/*
+	 * Disable auto chip select for the TPM channel.
+	 * Must be done after setting the SPI parameters.
+	 */
+	retval = xapea00x_br_disable_cs(dev, 0);
+	if (retval)
+		goto err;
+
+	/* De-assert chip select for the TPM channel. */
+	retval = xapea00x_br_deassert_cs(dev, 0);
+	if (retval)
+		goto err;
+
+	goto out;
+
+err:
+	dev_err(&dev->interface->dev,
+		"configuring SPI channel failed with %d\n", retval);
+
+out:
+	mutex_unlock(&dev->usb_mutex);
+	return retval;
+}
+
+/**
+ * xapea00x_spi_transfer - Execute a single SPI transfer.
+ * @dev: pointer to the device to do the transfer on
+ * @tx_buf: pointer to the data to send, if not NULL
+ * @rx_buf: pointer to the buffer to store the received data, if not NULL
+ * @len: length in bytes of the data to send/receive
+ * @cs_hold: If non-zero, the chip select will remain asserted
+ * @delay_usecs: If nonzero, how long to delay after last bit transfer
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_spi_transfer(struct xapea00x_device *dev,
+			  const void *tx_buf, void *rx_buf, u32 len,
+			  int cs_hold, u16 delay_usecs)
+{
+	int retval;
+
+	/* Assert chip select */
+	retval = xapea00x_br_assert_cs(dev, 0);
+	if (retval)
+		goto out;
+
+	/* empty transfer */
+	if (!tx_buf && !rx_buf)
+		retval = 0;
+	/* read transfer */
+	else if (!tx_buf)
+		retval = xapea00x_br_spi_read(dev, rx_buf, len);
+	/* write transfer */
+	else if (!rx_buf)
+		retval = xapea00x_br_spi_write(dev, tx_buf, len);
+	/* write_read transfer */
+	else
+		retval = xapea00x_br_spi_write_read(dev, tx_buf, rx_buf, len);
+
+	/* Deassert chip select, if requested */
+	if (!cs_hold)
+		retval = xapea00x_br_deassert_cs(dev, 0);
+
+	/* Delay for the requested time */
+	udelay(delay_usecs);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_spi_transfer_one_message - Execute a full SPI message.
+ * @master: The SPI master on which to execute the message.
+ * @msg: The SPI message to execute.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative erorr number.
+ */
+static int xapea00x_spi_transfer_one_message(struct spi_master *master,
+					     struct spi_message *msg)
+{
+	struct xapea00x_device *dev;
+	struct spi_transfer *xfer;
+	int is_last, retval;
+
+	dev = spi_master_get_devdata(master);
+
+	mutex_lock(&dev->usb_mutex);
+	if (!dev->interface) {
+		retval = -ENODEV;
+		goto out;
+	}
+
+	/* perform all transfers */
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		is_last = list_is_last(&xfer->transfer_list, &msg->transfers);
+
+		/* Transfer message */
+		retval = xapea00x_spi_transfer(dev, xfer->tx_buf,
+					       xfer->rx_buf, xfer->len,
+					       is_last == xfer->cs_change,
+					       xfer->delay_usecs);
+		if (retval)
+			goto out;
+
+		msg->actual_length += xfer->len;
+	}
+
+	retval = 0;
+
+out:
+	msg->status = retval;
+	spi_finalize_current_message(master);
+
+	mutex_unlock(&dev->usb_mutex);
+	return retval;
+}
+
+/**
+ * xapea00x_spi_probe - Register and configure the SPI master.
+ * @dev: the device whose SPI master to register
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_probe(struct xapea00x_device *dev)
+{
+	struct spi_master *spi_master;
+	int retval;
+
+	spi_master = spi_alloc_master(&dev->interface->dev, sizeof(void *));
+	if (!spi_master) {
+		retval = -ENOMEM;
+		goto err_out;
+	}
+
+	spi_master_set_devdata(spi_master, dev);
+
+	spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
+	spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
+
+	spi_master->bus_num = -1; /* dynamically assigned */
+	spi_master->num_chipselect = 1;
+	spi_master->mode_bits = SPI_MODE_0;
+
+	spi_master->flags = 0;
+	spi_master->setup = xapea00x_spi_setup;
+	spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
+
+	retval = spi_register_master(spi_master);
+
+	if (retval)
+		goto free_spi;
+
+	dev->spi_master = spi_master;
+
+	return 0;
+
+free_spi:
+	spi_master_put(spi_master);
+	dev->spi_master = NULL;
+
+err_out:
+	return retval;
+}
+
+struct xapea00x_async_probe {
+	struct work_struct work;
+	struct xapea00x_device *dev;
+};
+
+#define work_to_probe(w) container_of(w, struct xapea00x_async_probe, work)
+
+/**
+ * xapea00x_init_async_probe - initialize an async probe with the
+ * specified values.
+ * @probe: pointer to the async_probe to initialize
+ * @dev: pointer to the device to probe
+ * @f: pointer to the probe function
+ */
+static void xapea00x_init_async_probe(struct xapea00x_async_probe *probe,
+				      struct xapea00x_device *dev,
+				      void (*f)(struct work_struct *work))
+{
+	INIT_WORK(&probe->work, f);
+	probe->dev = dev;
+
+	kref_get(&dev->kref);
+	spi_master_get(dev->spi_master);
+}
+
+/**
+ * xapea00x_cleanup_async_probe - clean up the internals of the async
+ * probe. Call this method after the probe has completed.
+ *
+ * The caller is responsible for freeing the probe itself, if
+ * dynamically allocated.
+ *
+ * @probe: pointer to the async_probe to clean up
+ */
+static void xapea00x_cleanup_async_probe(struct xapea00x_async_probe *probe)
+{
+	spi_master_put(probe->dev->spi_master);
+	kref_put(&probe->dev->kref, xapea00x_delete);
+}
+
+static struct spi_board_info tpm_board_info = {
+	.modalias	= XAPEA00X_TPM_MODALIAS,
+	.max_speed_hz	= 43 * 1000 * 1000, // Hz
+	.chip_select	= 0,
+	.mode		= SPI_MODE_0
+};
+
+/**
+ * xapea00x_tpm_probe - Register and initialize the TPM device
+ * @work: the work struct contained by the xapea00x device
+ *
+ * Context: !in_interrupt()
+ */
+static void xapea00x_tpm_probe(struct work_struct *work)
+{
+	struct xapea00x_async_probe *probe = work_to_probe(work);
+	struct xapea00x_device *dev = probe->dev;
+	struct spi_master *spi_master = dev->spi_master;
+	struct spi_device *tpm;
+	int retval;
+
+	tpm = spi_new_device(spi_master, &tpm_board_info);
+	mutex_lock(&dev->usb_mutex);
+	if (!dev->interface) {
+		retval = -ENODEV;
+		goto out;
+	}
+	if (!tpm) {
+		retval = -ENODEV;
+		dev_err(&dev->interface->dev,
+			"unable to add spi device for TPM\n");
+		goto err;
+	}
+
+	dev->tpm = tpm;
+	goto out;
+
+err:
+	dev_err(&dev->interface->dev,
+		"TPM initialization failed with %d\n", retval);
+
+out:
+	mutex_unlock(&dev->usb_mutex);
+	xapea00x_cleanup_async_probe(probe);
+	kzfree(probe);
+}
+
+/*******************************************************************************
+ * USB driver structs and functions
+ */
+
+static const struct usb_device_id xapea00x_devices[] = {
+	{ USB_DEVICE(USB_VENDOR_ID_SILABS, USB_PRODUCT_ID_XAPEA001) },
+	{ USB_DEVICE(USB_VENDOR_ID_XAPTUM, USB_PRODUCT_ID_XAPEA002) },
+	{ USB_DEVICE(USB_VENDOR_ID_XAPTUM, USB_PRODUCT_ID_XAPEA003) },
+	{ }
+};
+MODULE_DEVICE_TABLE(usb, xapea00x_devices);
+
+static int xapea00x_probe(struct usb_interface *interface,
+			  const struct usb_device_id *id)
+{
+	struct xapea00x_device *dev;
+	struct xapea00x_async_probe *probe;
+	int retval;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	kref_init(&dev->kref);
+	mutex_init(&dev->usb_mutex);
+
+	/* ---------------------- USB ------------------------ */
+	dev->interface = interface;
+	dev->udev = usb_get_dev(interface_to_usbdev(interface));
+
+	dev->vid = __le16_to_cpu(dev->udev->descriptor.idVendor);
+	dev->pid = __le16_to_cpu(dev->udev->descriptor.idProduct);
+
+	retval = usb_find_common_endpoints(interface->cur_altsetting,
+					   &dev->bulk_in, &dev->bulk_out,
+					   NULL, NULL);
+	if (retval) {
+		dev_err(&interface->dev,
+			"could not find both bulk-in and bulk-out endpoints\n");
+		goto free_dev;
+	}
+
+	usb_set_intfdata(interface, dev);
+
+	/* ---------------------- SPI Master ------------------------ */
+	retval = xapea00x_spi_probe(dev);
+	if (retval) {
+		dev_err(&interface->dev, "could not initialize SPI master\n");
+		goto free_dev;
+	}
+
+	/* ---------------------- TPM SPI Device ------------------------ */
+	probe = kzalloc(sizeof(*probe), GFP_KERNEL);
+	if (!probe) {
+		retval = -ENOMEM;
+		goto free_spi;
+	}
+	xapea00x_init_async_probe(probe, dev, xapea00x_tpm_probe);
+
+	schedule_work(&probe->work);
+
+	/* ---------------------- Finished ------------------------ */
+	return 0;
+
+free_spi:
+	spi_unregister_master(dev->spi_master);
+
+free_dev:
+	kref_put(&dev->kref, xapea00x_delete);
+
+	dev_err(&interface->dev, "device failed with %d\n", retval);
+	return retval;
+}
+
+static void xapea00x_disconnect(struct usb_interface *interface)
+{
+	struct xapea00x_device *dev = usb_get_intfdata(interface);
+
+	usb_set_intfdata(interface, NULL);
+	spi_unregister_master(dev->spi_master);
+
+	mutex_lock(&dev->usb_mutex);
+	dev->interface = NULL;
+	mutex_unlock(&dev->usb_mutex);
+
+	kref_put(&dev->kref, xapea00x_delete);
+}
+
+static struct usb_driver xapea00x_driver = {
+	.name		= KBUILD_MODNAME,
+	.probe		= xapea00x_probe,
+	.disconnect	= xapea00x_disconnect,
+	.id_table	= xapea00x_devices
+};
+
+module_usb_driver(xapea00x_driver);
+
+MODULE_AUTHOR("David R. Bild <david.bild@xaptum.com>");
+MODULE_DESCRIPTION("Xaptum XAP-EA-00x ENF Access card");
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/misc/xapea00x/xapea00x-spi.c b/drivers/usb/misc/xapea00x/xapea00x-spi.c
new file mode 100644
index 000000000000..d71e79029639
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-spi.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ *  TPM 2.0-based hardware module for authenticating IoT devices and
+ *  gateways.
+ *
+ *  Copyright (c) 2017-2018 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+/*******************************************************************************
+ * SPI master functions
+ */
+
+/**
+ * xapea00x_spi_setup - Setup the SPI channel for the TPM.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_setup(struct spi_device *spi)
+{
+	struct xapea00x_device *dev;
+	int retval;
+
+	dev = spi_master_get_devdata(spi->master);
+
+	/* Verify that this is the TPM device */
+	if (spi->chip_select != 0) {
+		retval = -EINVAL;
+		goto err;
+	}
+
+	/* Set the SPI parameters for the TPM channel. */
+	retval = xapea00x_br_set_spi_word(dev, 0, XAPEA00X_TPM_SPI_WORD);
+	if (retval)
+		goto err;
+
+	/*
+	 * Disable auto chip select for the TPM channel.
+	 * Must be done after setting the SPI parameters.
+	 */
+	retval = xapea00x_br_set_gpio_cs(dev, 0, XAPEA00X_GPIO_CS_DISABLED);
+	if (retval)
+		goto err;
+
+	/* De-assert chip select for the TPM channel. */
+	retval = xapea00x_br_set_gpio_value(dev, 0, 1);
+	if (retval)
+		goto err;
+
+	return 0;
+
+err:
+	dev_err(&dev->interface->dev,
+		"configuring SPI channel failed with %d\n", retval);
+	return retval;
+}
+
+/**
+ * xapea00x_spi_transfer - Execute a single SPI transfer.
+ * @dev: pointer to the device to do the transfer on
+ * @tx_buf: pointer to the data to send, if not NULL
+ * @rx_buf: pointer to the buffer to store the received data, if not NULL
+ * @len: length in bytes of the data to send/receive
+ * @cs_hold: If non-zero, the chip select will remain asserted
+ * @delay_usecs: If nonzero, how long to delay after last bit transfer
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_spi_transfer(struct xapea00x_device *dev,
+			  const void *tx_buf, void *rx_buf, u32 len,
+			  int cs_hold, u16 delay_usecs)
+{
+	int retval;
+
+	/* Assert chip select */
+	retval = xapea00x_br_set_gpio_value(dev, 0, 0);
+	if (retval)
+		goto out;
+
+	/* empty transfer */
+	if (!tx_buf && !rx_buf)
+		retval = 0;
+	/* read transfer */
+	else if (!tx_buf)
+		retval = xapea00x_br_spi_read(dev, rx_buf, len);
+	/* write transfer */
+	else if (!rx_buf)
+		retval = xapea00x_br_spi_write(dev, tx_buf, len);
+	/* write_read transfer */
+	else
+		retval = xapea00x_br_spi_write_read(dev, tx_buf, rx_buf, len);
+
+	/* Deassert chip select, if requested */
+	if (!cs_hold)
+		retval = xapea00x_br_set_gpio_value(dev, 0, 1);
+
+	/* Delay for the requested time */
+	udelay(delay_usecs);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_spi_transfer_one_message - Execute a full SPI message.
+ * @master: The SPI master on which to execute the message.
+ * @msg: The SPI message to execute.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative erorr number.
+ */
+static int xapea00x_spi_transfer_one_message(struct spi_master *master,
+					     struct spi_message *msg)
+{
+	struct xapea00x_device *dev;
+	struct spi_transfer *xfer;
+	int is_last, retval;
+
+	dev = spi_master_get_devdata(master);
+
+	/* perform all transfers */
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		is_last = list_is_last(&xfer->transfer_list, &msg->transfers);
+
+		/* Transfer message */
+		retval = xapea00x_spi_transfer(dev, xfer->tx_buf,
+					       xfer->rx_buf, xfer->len,
+					       is_last == xfer->cs_change,
+					       xfer->delay_usecs);
+		if (retval)
+			goto out;
+
+		msg->actual_length += xfer->len;
+	}
+
+	retval = 0;
+
+out:
+	msg->status = retval;
+	spi_finalize_current_message(master);
+	return retval;
+}
+
+/**
+ * xapea00x_spi_probe - Register and configure the SPI master.
+ * @dev: the device whose SPI master to register
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_probe(struct xapea00x_device *dev)
+{
+	struct spi_master *spi_master;
+	int retval;
+
+	spi_master = spi_alloc_master(&dev->udev->dev, sizeof(void *));
+	if (!spi_master) {
+		retval = -ENOMEM;
+		goto err_out;
+	}
+
+	spi_master_set_devdata(spi_master, dev);
+
+	spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
+	spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
+
+	spi_master->bus_num = -1; /* dynamically assigned */
+	spi_master->num_chipselect = XAPEA00X_NUM_CS;
+	spi_master->mode_bits = SPI_MODE_0;
+
+	spi_master->flags = 0;
+	spi_master->setup = xapea00x_spi_setup;
+	spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
+
+	retval = spi_register_master(spi_master);
+
+	if (retval)
+		goto free_spi;
+
+	dev->spi_master = spi_master;
+
+	return 0;
+
+free_spi:
+	spi_master_put(spi_master);
+	dev->spi_master = NULL;
+
+err_out:
+	return retval;
+}
diff --git a/drivers/usb/misc/xapea00x/xapea00x.h b/drivers/usb/misc/xapea00x/xapea00x.h
new file mode 100644
index 000000000000..4771a9af71e3
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x.h
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ *  TPM 2.0-based hardware module for authenticating IoT devices and
+ *  gateways.
+ *
+ *  Copyright (c) 2017-2018 Xaptum, Inc.
+ */
+
+#ifndef _XAPEA00X_H
+#define _XAPEA00X_H
+
+#include <linux/kernel.h>
+#include <linux/kref.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#define USB_VENDOR_ID_SILABS           0x10c4
+#define USB_VENDOR_ID_XAPTUM           0x2FE0
+
+#define USB_PRODUCT_ID_XAPEA001        0x8BDE
+#define USB_PRODUCT_ID_XAPEA002        0x8BDE
+#define USB_PRODUCT_ID_XAPEA003        0x8BEE
+
+struct xapea00x_device {
+	struct kref kref;
+
+	struct usb_device *udev;
+	/*
+	 * The interface pointer will be set NULL when the device
+	 * disconnects.  Accessing it safe only while holding the
+	 * usb_mutex.
+	 */
+	struct usb_interface *interface;
+	/*
+	 * Th usb_mutex must be held while synchronous USB requests are
+	 * in progress. It is acquired during disconnect to be sure
+	 * that there is not an outstanding request.
+	 */
+	struct mutex usb_mutex;
+
+	struct usb_endpoint_descriptor *bulk_in;
+	struct usb_endpoint_descriptor *bulk_out;
+
+	u16 pid;
+	u16 vid;
+
+	struct spi_master *spi_master;
+	struct spi_device *tpm;
+};
+
+/* Public bridge functions */
+int xapea00x_br_disable_cs(struct xapea00x_device *dev, u8 channel);
+int xapea00x_br_assert_cs(struct xapea00x_device *dev, u8 channel);
+int xapea00x_br_deassert_cs(struct xapea00x_device *dev, u8 channel);
+
+int xapea00x_br_spi_read(struct xapea00x_device *dev, void *rx_buf, int len);
+int xapea00x_br_spi_write(struct xapea00x_device *dev, const void *tx_buf,
+			  int len);
+int xapea00x_br_spi_write_read(struct xapea00x_device *dev, const void *tx_buf,
+			       void *rx_buf, int len);
+
+/* Shared SPI function */
+int xapea00x_spi_transfer(struct xapea00x_device *dev,
+			  const void *tx_buf, void *rx_buf, u32 len,
+			  int cs_hold, u16 delay_usecs);
+
+/* Shared TPM functions */
+int xapea00x_tpm_platform_initialize(struct xapea00x_device *dev);
+
+#endif /* _XAPEA00X_H */
-- 
2.16.3

Re: [PATCH v3 1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card

[Oliver Neukum]

Am Freitag, den 04.05.2018, 08:00 -0500 schrieb  David R. Bild :
> 
> +config USB_XAPEA00X
> +	tristate "Xaptum ENF Access card support (XAP-EA-00x)"
> +	depends on USB_SUPPORT && SPI && TCG_TPM

You'd have to know how the device internally works. It would be better
to select SPI.

> +static int xapea00x_br_bulk_write(struct xapea00x_device *dev,
> +				  struct xapea00x_br_bulk_command *header,
> +				  const void *data, int len)
> +{
> +	u8 *buf;
> +	unsigned int pipe;
> +	int buf_len, actual_len, retval;
> +
> +	buf_len = sizeof(struct xapea00x_br_bulk_command) + len;
> +	buf = kzalloc(buf_len, GFP_KERNEL);
> +	if (!buf) {
> +		retval = -ENOMEM;
> +		goto out;
> +	}
> +
> +	memcpy(buf, header, sizeof(struct xapea00x_br_bulk_command));
> +	memcpy(buf + sizeof(struct xapea00x_br_bulk_command), data, len);
> +
> +	pipe = usb_sndbulkpipe(dev->udev, dev->bulk_out->bEndpointAddress);
> +	retval = usb_bulk_msg(dev->udev, pipe, buf, buf_len, &actual_len,
> +			      XAPEA00X_BR_USB_TIMEOUT);
> +	if (retval)
> +		goto free_buf;

WTF?

> +free_buf:
> +	kzfree(buf);
> +
> +out:
> +	return retval;
> +}
> +

[..]
> +static int xapea00x_spi_setup(struct spi_device *spi)
> +{
> +	struct xapea00x_device *dev;
> +	int retval;
> +
> +	dev = spi_master_get_devdata(spi->master);
> +
> +	mutex_lock(&dev->usb_mutex);
> +	if (!dev->interface) {
> +		retval = -ENODEV;
> +		goto out;
> +	}
> +
> +	/* Verify that this is the TPM device */
> +	if (spi->chip_select != 0) {
> +		retval = -EINVAL;
> +		goto err;
> +	}
> +
> +	/*
> +	 * Disable auto chip select for the TPM channel.
> +	 * Must be done after setting the SPI parameters.
> +	 */
> +	retval = xapea00x_br_disable_cs(dev, 0);
> +	if (retval)
> +		goto err;
> +
> +	/* De-assert chip select for the TPM channel. */
> +	retval = xapea00x_br_deassert_cs(dev, 0);
> +	if (retval)
> +		goto err;
> +
> +	goto out;
> +
> +err:
> +	dev_err(&dev->interface->dev,
> +		"configuring SPI channel failed with %d\n", retval);

That could be a simple 'if' statement.
> +
> +out:
> +	mutex_unlock(&dev->usb_mutex);
> +	return retval;
> +}
> +
> 

[..]
> +/**
> + * xapea00x_spi_probe - Register and configure the SPI master.
> + * @dev: the device whose SPI master to register
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_spi_probe(struct xapea00x_device *dev)
> +{
> +	struct spi_master *spi_master;
> +	int retval;
> +
> +	spi_master = spi_alloc_master(&dev->interface->dev, sizeof(void *));
> +	if (!spi_master) {
> +		retval = -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	spi_master_set_devdata(spi_master, dev);
> +
> +	spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
> +	spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
> +
> +	spi_master->bus_num = -1; /* dynamically assigned */
> +	spi_master->num_chipselect = 1;
> +	spi_master->mode_bits = SPI_MODE_0;
> +
> +	spi_master->flags = 0;
> +	spi_master->setup = xapea00x_spi_setup;
> +	spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
> +
> +	retval = spi_register_master(spi_master);
> +
> +	if (retval)
> +		goto free_spi;
> +
> +	dev->spi_master = spi_master;

Race condition.

> +
> +	return 0;
> +
> +free_spi:
> +	spi_master_put(spi_master);
> +	dev->spi_master = NULL;
> +
> +err_out:
> +	return retval;
> +}
> +
> +struct xapea00x_async_probe {
> +	struct work_struct work;
> +	struct xapea00x_device *dev;
> +};
> +
> +#define work_to_probe(w) container_of(w, struct xapea00x_async_probe, work)
> +
> +/**
> + * xapea00x_init_async_probe - initialize an async probe with the
> + * specified values.
> + * @probe: pointer to the async_probe to initialize
> + * @dev: pointer to the device to probe
> + * @f: pointer to the probe function
> + */
> +static void xapea00x_init_async_probe(struct xapea00x_async_probe *probe,
> +				      struct xapea00x_device *dev,
> +				      void (*f)(struct work_struct *work))
> +{
> +	INIT_WORK(&probe->work, f);
> +	probe->dev = dev;
> +
> +	kref_get(&dev->kref);
> +	spi_master_get(dev->spi_master);
> +}
> +
> +/**
> + * xapea00x_cleanup_async_probe - clean up the internals of the async
> + * probe. Call this method after the probe has completed.
> + *
> + * The caller is responsible for freeing the probe itself, if
> + * dynamically allocated.
> + *
> + * @probe: pointer to the async_probe to clean up
> + */
> +static void xapea00x_cleanup_async_probe(struct xapea00x_async_probe *probe)
> +{
> +	spi_master_put(probe->dev->spi_master);
> +	kref_put(&probe->dev->kref, xapea00x_delete);
> +}
> +
> +static struct spi_board_info tpm_board_info = {
> +	.modalias	= XAPEA00X_TPM_MODALIAS,
> +	.max_speed_hz	= 43 * 1000 * 1000, // Hz

Are you hardcoding HZ ?

> +	.chip_select	= 0,
> +	.mode		= SPI_MODE_0
> +};

	Regards
		Oliver

Re: [PATCH v3 1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card

[David R. Bild]

On Mon, May 7, 2018 at 4:58 AM, Oliver Neukum <oneukum@suse.com> wrote:
>
> Am Freitag, den 04.05.2018, 08:00 -0500 schrieb  David R. Bild :
> >
> > +config USB_XAPEA00X
> > +     tristate "Xaptum ENF Access card support (XAP-EA-00x)"
> > +     depends on USB_SUPPORT && SPI && TCG_TPM
>
> You'd have to know how the device internally works. It would be better
> to select SPI.


That was my logic to Greg too. I'll change the SPI back to select.

> [..]
> > +/**
> > + * xapea00x_spi_probe - Register and configure the SPI master.
> > + * @dev: the device whose SPI master to register
> > + *
> > + * Return: If successful, 0. Otherwise a negative error number.
> > + */
> > +static int xapea00x_spi_probe(struct xapea00x_device *dev)
> > +{
> > +     struct spi_master *spi_master;
> > +     int retval;
> > +
> > +     spi_master = spi_alloc_master(&dev->interface->dev, sizeof(void *));
> > +     if (!spi_master) {
> > +             retval = -ENOMEM;
> > +             goto err_out;
> > +     }
> > +
> > +     spi_master_set_devdata(spi_master, dev);
> > +
> > +     spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
> > +     spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
> > +
> > +     spi_master->bus_num = -1; /* dynamically assigned */
> > +     spi_master->num_chipselect = 1;
> > +     spi_master->mode_bits = SPI_MODE_0;
> > +
> > +     spi_master->flags = 0;
> > +     spi_master->setup = xapea00x_spi_setup;
> > +     spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
> > +
> > +     retval = spi_register_master(spi_master);
> > +
> > +     if (retval)
> > +             goto free_spi;
> > +
> > +     dev->spi_master = spi_master;
>
> Race condition.
>

What race condition do you see?  (I appreciate the review, but need
some more specific help here.)

>
> > +
> > +     return 0;
> > +
> > +free_spi:
> > +     spi_master_put(spi_master);
> > +     dev->spi_master = NULL;
> > +
> > +err_out:
> > +     return retval;
> > +}
> > +
>
> > +
> > +static struct spi_board_info tpm_board_info = {
> > +     .modalias       = XAPEA00X_TPM_MODALIAS,
> > +     .max_speed_hz   = 43 * 1000 * 1000, // Hz
>
> Are you hardcoding HZ ?


Yes. But it's the max speed of the SPI clock which is actually
specified in real HZ. It's not kernel "HZ".

Thanks for the other comments too; I'll incorporate into the next version.

Best,
David

Re: [PATCH v3 1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card

[Oliver Neukum]

Am Montag, den 07.05.2018, 08:31 -0500 schrieb  David R. Bild :
> > > +     spi_master->flags = 0;
> > > +     spi_master->setup = xapea00x_spi_setup;
> > > +     spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
> > > +
> > > +     retval = spi_register_master(spi_master);
> > > +
> > > +     if (retval)
> > > +             goto free_spi;
> > > +
> > > +     dev->spi_master = spi_master;
> > 
> > Race condition.
> > 
> 
> What race condition do you see?  (I appreciate the review, but need
> some more specific help here.)

Hi,

you have registered the master. So it is functional, but if any
callback goes for dev->spi_master at that point, it will read an
incorrect value.

	HTH
		Oliver

[PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
for performing initialization of the TPM.  For these modules, the host
kernel is the platform, so we perform the initialization in the driver
before registering the TPM with the kernel TPM subsystem.

The initialization consists of issuing the TPM startup command,
running the TPM self-test, and setting the TPM platform hierarchy
authorization to a random, unsaved value so that it can never be used
after the driver has loaded.

Signed-off-by: David R. Bild <david.bild@xaptum.com>
---
 drivers/usb/misc/xapea00x/Makefile        |   3 +-
 drivers/usb/misc/xapea00x/xapea00x-core.c |  25 +
 drivers/usb/misc/xapea00x/xapea00x-tpm.c  | 952 ++++++++++++++++++++++++++++++
 3 files changed, 979 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-tpm.c

diff --git a/drivers/usb/misc/xapea00x/Makefile b/drivers/usb/misc/xapea00x/Makefile
index c4bcd7524c31..aa3f8803cdf5 100644
--- a/drivers/usb/misc/xapea00x/Makefile
+++ b/drivers/usb/misc/xapea00x/Makefile
@@ -4,4 +4,5 @@
 #
 obj-$(CONFIG_USB_XAPEA00X) += xapea00x.o
 
-xapea00x-y += xapea00x-core.o xapea00x-bridge.o
+xapea00x-y += xapea00x-core.o xapea00x-bridge.o xapea00x-tpm.o
+
diff --git a/drivers/usb/misc/xapea00x/xapea00x-core.c b/drivers/usb/misc/xapea00x/xapea00x-core.c
index 885bcda9c01d..53e82f8b38f3 100644
--- a/drivers/usb/misc/xapea00x/xapea00x-core.c
+++ b/drivers/usb/misc/xapea00x/xapea00x-core.c
@@ -280,6 +280,31 @@ static void xapea00x_tpm_probe(struct work_struct *work)
 	struct spi_device *tpm;
 	int retval;
 
+	mutex_lock(&dev->usb_mutex);
+	if (!dev->interface) {
+		retval = -ENODEV;
+		goto out;
+	}
+	/*
+	 * This driver is the "platform" in TPM terminology. Before
+	 * passing control of the TPM to the Linux TPM subsystem, do
+	 * the TPM initialization normally done by the platform code
+	 * (e.g., BIOS).
+	 */
+	retval = xapea00x_tpm_platform_initialize(dev);
+	if (retval) {
+		dev_err(&dev->interface->dev,
+			"unable to do TPM platform initialization: %d\n",
+			retval);
+		goto err;
+	}
+
+	/*
+	 * Now register the TPM with the Linux TPM subsystem.  This
+	 * may call through to xapea00x_spi_transfer_one_message(), so
+	 * don't hold usb_mutex here.
+	 */
+	mutex_unlock(&dev->usb_mutex);
 	tpm = spi_new_device(spi_master, &tpm_board_info);
 	mutex_lock(&dev->usb_mutex);
 	if (!dev->interface) {
diff --git a/drivers/usb/misc/xapea00x/xapea00x-tpm.c b/drivers/usb/misc/xapea00x/xapea00x-tpm.c
new file mode 100644
index 000000000000..27159043ce3c
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-tpm.c
@@ -0,0 +1,952 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ *  TPM 2.0-based hardware module for authenticating IoT devices and
+ *  gateways.
+ *
+ *  Copyright (c) 2017-2018 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+#define TPM_RETRY			50
+#define TPM_TIMEOUT			5    // msecs
+#define TPM_TIMEOUT_RANGE_US		300  // usecs
+
+#define TIS_SHORT_TIMEOUT		750  // msecs
+#define TIS_LONG_TIMEOUT		2000 // msecs
+
+#define TIS_MAX_BUF			1024 // byte
+#define TIS_HEADER_LEN			10   // byte
+
+#define TPM2_TIMEOUT_A			750  // msecs
+#define TPM2_TIMEOUT_B			2000 // msecs
+#define TPM2_TIMEOUT_C			200  // msecs
+#define TPM2_TIMEOUT_D			30   // msecs
+
+#define TPM_ACCESS_0			0x0000
+#define TPM_STS_0			0x0018
+#define TPM_DATA_FIFO_0		0x0024
+
+#define TPM2_ST_NO_SESSIONS		0x8001
+#define TPM2_ST_SESSIONS		0x8002
+
+#define TPM2_CC_STARTUP		0x0144
+#define TPM2_CC_SHUTDOWN		0x0145
+#define TPM2_CC_SELF_TEST		0x0143
+#define TPM2_CC_GET_RANDOM		0x017B
+#define TPM2_CC_HIERARCHY_CHANGE_AUTH	0x0129
+#define TPM2_CC_DICT_ATTACK_LOCK_RST	0x0139
+
+#define TPM_RC_SUCCESS			0x000
+#define TPM_RC_INITIALIZE		0x100
+
+enum tis_access {
+	TPM_ACCESS_VALID		= 0x80,
+	TPM_ACCESS_ACTIVE_LOCALITY	= 0x20,
+	TPM_ACCESS_REQUEST_PENDING	= 0x04,
+	TPM_ACCESS_REQUEST_USE		= 0x02
+};
+
+enum tis_status {
+	TPM_STS_VALID		= 0x80,
+	TPM_STS_COMMAND_READY	= 0x40,
+	TPM_STS_GO		= 0x20,
+	TPM_STS_DATA_AVAIL	= 0x10,
+	TPM_STS_DATA_EXPECT	= 0x08,
+	TPM_STS_SELF_TEST_DONE	= 0x04,
+	TPM_STS_RESPONSE_RETRY	= 0x02
+};
+
+struct tpm_tis_command {
+	__be16 tag;
+	__be32 size;
+	__be32 code;
+	    u8 body[0];
+} __attribute__((__packed__));
+
+/*******************************************************************************
+ * TPM TIS functions
+ */
+
+/**
+ * xapea00x_tpm_msleep - sleep for at least the specified time.
+ * @msecs: minimum duration to sleep for in milliseconds
+ */
+static void xapea00x_tpm_msleep(int msecs)
+{
+	usleep_range(msecs * 1000,
+		     msecs * 1000 + TPM_TIMEOUT_RANGE_US);
+}
+
+/**
+ * xapea00x_tpm_transfer - execute an SPI transfer.
+ * @dev: pointer to the device
+ * @addr: the TPM TIS register address
+ * @in: if a write or write_read transfer, the data to write
+ * @out: if a read or write_read  transfer, the buffer to read data into
+ * @len: the number of bytes to transfer
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transfer(struct xapea00x_device *dev,
+				 u32 addr, u8 *in, u8 *out, u16 len)
+{
+	u8 header[4];
+	int i, retval;
+
+	header[0] = (in ? 0x80 : 0x00) | (len - 1);
+	header[1] = 0xd4;
+	header[2] = addr >> 8;
+	header[3] = addr;
+
+	retval = xapea00x_spi_transfer(dev, header, header, 4, 1, 0);
+	if (retval)
+		goto out;
+
+	/* handle SPI wait states */
+	if ((header[3] & 0x01) == 0x00) {
+		header[0] = 0;
+
+		for (i = 0; i < TPM_RETRY; i++) {
+			retval = xapea00x_spi_transfer(dev, header, header, 1,
+						       1, 0);
+			if (retval)
+				goto out;
+			if ((header[0] & 0x01) == 00)
+				break;
+		}
+
+		if (i == TPM_RETRY) {
+			retval = -ETIMEDOUT;
+			goto out;
+		}
+	}
+
+	retval = xapea00x_spi_transfer(dev, out, in, len, 0, 0);
+	if (retval)
+		goto out;
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_read_bytes - read data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @result: buffer to in which to place the read data
+ * @len: the number of bytes to read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_read_bytes(struct xapea00x_device *dev, u32 addr,
+				   void *result, u16 len)
+{
+	return xapea00x_tpm_transfer(dev, addr, result, NULL, len);
+}
+
+/**
+ * xapea00x_tpm_write_bytes - write data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to write to
+ * @data: pointer to the data to write
+ * @len: the number of bytes to read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_write_bytes(struct xapea00x_device *dev, u32 addr,
+				    void *data, u16 len)
+{
+	return xapea00x_tpm_transfer(dev, addr, NULL, data, len);
+}
+
+/**
+ * xapea00x_tpm_read8 - read one byte of data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @result: pointer to the destination
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_read8(struct xapea00x_device *dev, u32 addr, u8 *result)
+{
+	return xapea00x_tpm_read_bytes(dev, addr, result, 1);
+}
+
+/**
+ * xapea00x_tpm_write8 - write one byte of data to the TPM
+ * @dev: pointer to the device
+ * @addr: the register to write to
+ * @data:  the byte to write
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_write8(struct xapea00x_device *dev, u32 addr, u8 data)
+{
+	return xapea00x_tpm_write_bytes(dev, addr, &data, 1);
+}
+
+/**
+ * xapea00x_tpm_read32 - read one integer of data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @result: pointer to the destination
+ *
+ * The method performs any required endianness conversion on the
+ * result.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_read32(struct xapea00x_device *dev, u32 addr,
+			       u32 *result)
+{
+	__le32 result_le;
+	int retval;
+
+	retval = xapea00x_tpm_read_bytes(dev, addr, &result_le,
+					 sizeof(result_le));
+	if (retval)
+		goto out;
+
+	*result = __le32_to_cpu(result_le);
+	retval = 0;
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_write32 - write one integer of data to the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @data: the integer to write
+ *
+ * The method performs any required endianness conversion on the
+ * data.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_write32(struct xapea00x_device *dev, u32 addr, u32 data)
+{
+	__le32 data_le;
+
+	data_le = __cpu_to_le32(data);
+	return xapea00x_tpm_write_bytes(dev, addr, &data_le, sizeof(data_le));
+}
+
+/**
+ * xapea00x_tpm_wait_reg8 - waits for the specified flags on the
+ * register to be set.
+ * @dev: pointer to the device
+ * @addr: the register to check
+ * @flags: mask of the flags to check
+ * @timeout_msecs: maximum amount of time to wait in milliseconds
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_wait_reg8(struct xapea00x_device *dev,
+				  u8 addr, u8 flags,
+				  int timeout_msecs)
+{
+	unsigned long stop = jiffies + msecs_to_jiffies(timeout_msecs);
+	u8 reg;
+	int retval;
+
+	do {
+		retval = xapea00x_tpm_read8(dev, addr, &reg);
+		if (retval)
+			goto out;
+
+		if ((reg & flags) == flags) {
+			retval = 0;
+			goto out;
+		}
+
+		xapea00x_tpm_msleep(TPM_TIMEOUT);
+	} while (time_before(jiffies, stop));
+
+	retval = -ETIMEDOUT;
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_request_locality0 - sets the active locality to 0
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_request_locality0(struct xapea00x_device *dev)
+{
+	int retval;
+
+	retval = xapea00x_tpm_write8(dev, TPM_ACCESS_0, TPM_ACCESS_REQUEST_USE);
+	if (retval)
+		goto out;
+
+	retval = xapea00x_tpm_wait_reg8(dev, TPM_ACCESS_0,
+					TPM_ACCESS_ACTIVE_LOCALITY,
+					TPM2_TIMEOUT_A);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_release_locality0 - release the active locality
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0.  Otherwise a negative error code.
+ */
+static int xapea00x_tpm_release_locality0(struct xapea00x_device *dev)
+{
+	return xapea00x_tpm_write8(dev, TPM_ACCESS_0,
+				   TPM_ACCESS_ACTIVE_LOCALITY);
+}
+
+/**
+ * xapea00x_tpm_burst_count - fetch the number of bytes of data the
+ * TPM can currently handle in one burst.
+ * @dev: pointer to the device
+ * @counter: pointer to the destination for the count
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_burst_count(struct xapea00x_device *dev, u32 *count)
+{
+	u32 reg;
+	int retval;
+
+	retval = xapea00x_tpm_read32(dev, TPM_STS_0, &reg);
+	if (retval)
+		goto out;
+
+	*count = (reg >> 8) & 0xFFFF;
+	retval = 0;
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_send - send the command to the TPM and execute it.
+ * @dev: pointer to the device
+ * @buf: the buffer containing the command
+ * @len: size of the buffer in bytes.
+ *
+ * N.B., the command may not fill the entire buffer. This function
+ * parses the command to determine its actual size.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_send(struct xapea00x_device *dev, void *buf, u32 len)
+{
+	struct tpm_tis_command *cmd = buf;
+	u32 size, burst;
+	int retval;
+
+	/* wait for TPM to be ready for command */
+	retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_COMMAND_READY,
+					TPM2_TIMEOUT_B);
+	if (retval)
+		goto err;
+
+	/* extract size of from header */
+	size = __be32_to_cpu(cmd->size);
+
+	if (size > len) {
+		retval = -EINVAL;
+		goto err;
+	}
+
+	/* Write the command */
+	while (size > 0) {
+		xapea00x_tpm_burst_count(dev, &burst);
+		burst = min(burst, size);
+
+		retval = xapea00x_tpm_write_bytes(dev, TPM_DATA_FIFO_0, buf,
+						  burst);
+		if (retval)
+			goto cancel;
+
+		retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_VALID,
+						TPM2_TIMEOUT_C);
+		if (retval)
+			goto cancel;
+
+		buf += burst;
+		size -= burst;
+	}
+
+	/* Do it */
+	retval = xapea00x_tpm_write8(dev, TPM_STS_0, TPM_STS_GO);
+	if (retval)
+		goto cancel;
+
+	return 0;
+
+cancel:
+	/* Attempt to cancel */
+	xapea00x_tpm_write8(dev, TPM_STS_0, TPM_STS_COMMAND_READY);
+
+err:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_recv - recv a command response from the TPM.
+ * @dev: pointer to the device
+ * @buf: the buffer in which to store the response
+ * @len: size of the buffer in bytes.
+ *
+ * N.B., the result may not fill the entire buffer. The caller must
+ * parse the response header to determine its actual size.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_recv(struct xapea00x_device *dev, void *buf, u32 len)
+{
+	struct tpm_tis_command *cmd = buf;
+	u32 burst;
+	u32 size;
+	int retval;
+
+	/* wait for TPM to have data available */
+	retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_DATA_AVAIL,
+					TPM2_TIMEOUT_C);
+	if (retval)
+		goto cancel;
+
+	/* read the header */
+	if (len < TIS_HEADER_LEN) {
+		retval = -EINVAL;
+		goto cancel;
+	}
+
+	retval = xapea00x_tpm_read_bytes(dev, TPM_DATA_FIFO_0, buf,
+					 TIS_HEADER_LEN);
+	if (retval)
+		goto cancel;
+
+	/* extract size of body from header */
+	size = __be32_to_cpu(cmd->size);
+	if (len < size) {
+		retval = -EINVAL;
+		goto cancel;
+	}
+
+	size -= TIS_HEADER_LEN;
+	buf   = &cmd->body;
+
+	/* read the body */
+	while (size > TIS_HEADER_LEN) {
+		xapea00x_tpm_burst_count(dev, &burst);
+		burst = min(burst, size);
+
+		retval = xapea00x_tpm_read_bytes(dev, TPM_DATA_FIFO_0, buf,
+						 burst);
+		if (retval)
+			goto cancel;
+
+		size -= burst;
+		buf += burst;
+	}
+
+	/* wait for valid */
+	retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_VALID,
+					TPM2_TIMEOUT_C);
+	if (retval)
+		goto err;
+
+	return 0;
+
+cancel:
+	xapea00x_tpm_write32(dev, TPM_STS_0, TPM_STS_COMMAND_READY);
+
+err:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_transmit - transmit one command to the TPM and receive
+ * the response.
+ * @dev: pointer to the device
+ * @buf: the buffer containing the command and to place the response in.
+ * @len: size of the buffer in bytes.
+ *
+ * N.B., the command and result may not fill the entire buffer. The
+ * caller must parse the response header to determine its actual size.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transmit(struct xapea00x_device *dev, void *buf,
+				 u32 len)
+{
+	int retval;
+
+	retval = xapea00x_tpm_request_locality0(dev);
+	if (retval)
+		goto out;
+
+	retval = xapea00x_tpm_send(dev, buf, len);
+	if (retval)
+		goto release;
+
+	retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_DATA_AVAIL,
+					TIS_LONG_TIMEOUT);
+	if (retval)
+		goto cancel;
+
+	retval = xapea00x_tpm_recv(dev, buf, len);
+	if (retval)
+		goto release;
+
+	retval = 0;
+	goto release;
+
+cancel:
+	xapea00x_tpm_write32(dev, TPM_STS_0, TPM_STS_COMMAND_READY);
+
+release:
+	xapea00x_tpm_release_locality0(dev);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_transmit_cmd - build and transmit one command to the
+ * TPM and receive the response.
+ * @dev: pointer to the device
+ * @tag: the TPM command header tag
+ * @cc: the TPM command header code
+ * @body: pointer to the command body
+ * @body_len: size in bytes of the command body
+ * @rc: pointer to the destination for the result code
+ * @result: pointer to the destination for the result body. If NULL,
+ *          the result body will be discarded.
+ * @result_len: size in bytes of the result buffer
+ * @actual_len: size in bytes of the result body. May be NULL is
+ *              result is NULL.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transmit_cmd(struct xapea00x_device *dev,
+				     u16 tag, u32 cc, void *body, u32 body_len,
+				     u32 *rc, void *result, u32 result_len,
+				     u32 *actual_len)
+{
+	struct tpm_tis_command *cmd;
+	void *buf;
+	int buflen, cmdlen, retval;
+
+	buflen = TIS_MAX_BUF + 4;
+	cmdlen = buflen - 2; /* reserve 2 bytes for realignment */
+
+	if (body_len + TIS_HEADER_LEN > cmdlen) {
+		retval = -E2BIG;
+		pr_notice("transmit_cmd: body_len + TIS_HEADER_LEN > cmdlen (%d)",
+			  cmdlen);
+		goto out;
+	}
+
+	buf = kzalloc(buflen, GFP_KERNEL);
+	if (!buf) {
+		retval = -ENOMEM;
+		goto out;
+	}
+	cmd = buf + 2; /* ensure all fields are properly aligned */
+
+	/* Build the command */
+	cmd->tag  = __cpu_to_be16(tag);
+	cmd->size = __cpu_to_be32(TIS_HEADER_LEN + body_len);
+	cmd->code = __cpu_to_be32(cc);
+	memcpy(&cmd->body, body, body_len);
+
+	/* Execute the command */
+	retval = xapea00x_tpm_transmit(dev, cmd, cmdlen);
+	if (retval)
+		goto free;
+
+	/* Extract result code */
+	*rc = __be32_to_cpu(cmd->code);
+
+	/* Copy the response data */
+	if (result) {
+		*actual_len = __be32_to_cpu(cmd->size) - TIS_HEADER_LEN;
+		if (*actual_len > result_len) {
+			retval = -E2BIG;
+			goto free;
+		}
+		memcpy(result, &cmd->body, *actual_len);
+	}
+
+	retval = 0;
+
+free:
+	memset(buf, 0, buflen);
+	kzfree(buf);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_transmit_cmd_simple - build and transmit one command to the
+ * TPM and discard the respone body.
+ * @dev: pointer to the device
+ * @tag: the TPM command header tag
+ * @cc: the TPM command header code
+ * @body: pointer to the command body
+ * @len: size in bytes of the command body
+ * @rc: pointer to the destination for the result code
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transmit_cmd_simple(struct xapea00x_device *dev,
+					    u16 tag, u32 cc,
+					    void *body, u32 len, u32 *rc)
+{
+	return xapea00x_tpm_transmit_cmd(dev, tag, cc, body, len, rc, NULL, 0,
+					 NULL);
+}
+
+/*******************************************************************************
+ * TPM commands
+ */
+
+/**
+ * xapea00x_tpm_startup - executes the TPM2_Startup command.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_startup(struct xapea00x_device *dev)
+{
+	u8 body[2] = { 0x00, 0x00 };
+	u32 rc;
+	int retval;
+
+	retval = xapea00x_tpm_transmit_cmd_simple(dev, TPM2_ST_NO_SESSIONS,
+						  TPM2_CC_STARTUP, body,
+						  sizeof(body), &rc);
+	if (retval)
+		goto out;
+
+	if (rc != TPM_RC_SUCCESS && rc != TPM_RC_INITIALIZE) {
+		retval = -EIO;
+		goto out;
+	}
+
+	retval = 0;
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_self_test - executes the TPM2_SelfTest command.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_self_test(struct xapea00x_device *dev)
+{
+	u8 body[1] = { 0x01 };
+	u32 rc;
+	int retval;
+
+	retval = xapea00x_tpm_transmit_cmd_simple(dev, TPM2_ST_NO_SESSIONS,
+						  TPM2_CC_SELF_TEST, body,
+						  sizeof(body), &rc);
+	if (retval)
+		goto out;
+
+	if (rc != TPM_RC_SUCCESS) {
+		retval = -EIO;
+		goto out;
+	}
+
+	retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_SELF_TEST_DONE,
+					TIS_LONG_TIMEOUT);
+	if (retval) {
+		retval = -EIO;
+		goto out;
+	}
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_dict_attack_lock_reset - executes the
+ * TPM2_DictionaryAttackLockReset command.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int
+xapea00x_tpm_dict_attack_lock_reset(struct xapea00x_device *dev)
+{
+	u8 body[17] = { 0x40, 0x00, 0x00, 0x0A, // TPM_RH_LOCKOUT
+			0x00, 0x00, 0x00, 0x09, // authorizationSize
+			0x40, 0x00, 0x00, 0x09, // TPM_RS_PW
+			0x00, 0x00,		// nonce size
+						// nonce
+			0x01,			// session attributes
+			0x00, 0x00		// payload size
+						// payload
+		      };
+	u32 rc;
+	int retval;
+
+	retval = xapea00x_tpm_transmit_cmd_simple(dev,
+						  TPM2_ST_SESSIONS,
+						  TPM2_CC_DICT_ATTACK_LOCK_RST,
+						  body, sizeof(body), &rc);
+	if (retval)
+		goto out;
+
+	if (rc != TPM_RC_SUCCESS) {
+		retval = -EIO;
+		goto out;
+	}
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_get_random - executes the TPM2_GetRandom command.
+ * @dev: pointer to the device
+ * @len: number of bytes to request
+ * @bytes: pointer to the destination
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_get_random(struct xapea00x_device *dev, u16 len,
+				   void *bytes)
+{
+	__be16 body;
+	u8 *buf;
+	u32 buf_len, result_len;
+	u32 rc;
+	int retval;
+
+	buf_len = len + 2;
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf) {
+		retval = -ENOMEM;
+		goto out;
+	}
+
+	while (len > 0) {
+		body = __cpu_to_be16(len);
+
+		retval = xapea00x_tpm_transmit_cmd(dev, TPM2_ST_NO_SESSIONS,
+						   TPM2_CC_GET_RANDOM,
+						   &body, sizeof(body),
+						   &rc, buf, buf_len,
+						   &result_len);
+
+		if (retval)
+			goto free;
+
+		if (rc != TPM_RC_SUCCESS) {
+			retval = -EIO;
+			goto free;
+		}
+
+		result_len = __be16_to_cpu(*(__be16 *)buf);
+		if (result_len > len) {
+			retval = -E2BIG;
+			goto free;
+		}
+
+		memcpy(bytes, buf + 2, result_len);
+		len -= result_len;
+	}
+
+	retval = 0;
+
+free:
+	memset(buf, 0, buf_len);
+	kzfree(buf);
+
+out:
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_randomize_platform_auth - sets the platform
+ * authorization to a random password and then discards it.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_randomize_platform_auth(struct xapea00x_device *dev)
+{
+	u8 password[16];
+	u8 body[35] = { 0x40, 0x00, 0x00, 0x0C, // TPM_RH_PLATFORM
+			0x00, 0x00, 0x00, 0x09, // authorizationSize
+			0x40, 0x00, 0x00, 0x09, // TPM_RS_PW
+			0x00, 0x00,		// nonce size
+						// nonce
+			0x01,			// session attributes
+			0x00, 0x00,		// old auth payload size
+						// old auth payload
+			0x00, 0x10,		// new auth payload size
+			0x00, 0x00, 0x00, 0x00, // new auth payload
+			0x00, 0x00, 0x00, 0x00, // new auth payload
+			0x00, 0x00, 0x00, 0x00, // new auth payload
+			0x00, 0x00, 0x00, 0x00	// new auth payload
+		      };
+	u32 rc;
+	int retval;
+
+	retval = xapea00x_tpm_get_random(dev, sizeof(password), password);
+	if (retval) {
+		dev_err(&dev->interface->dev,
+			"TPM get random failed with %d\n", retval);
+		goto out;
+	}
+
+	memcpy(body + 19, password, sizeof(password));
+
+	retval = xapea00x_tpm_transmit_cmd_simple(dev, TPM2_ST_SESSIONS,
+						  TPM2_CC_HIERARCHY_CHANGE_AUTH,
+						  &body, sizeof(body), &rc);
+	if (retval)
+		goto out;
+
+	if (rc != TPM_RC_SUCCESS) {
+		retval = -EIO;
+		pr_notice("HierarchyChangeAuth result code: %d\n", rc);
+		goto out;
+	}
+
+	retval = 0;
+
+out:
+	memset(password, 0, sizeof(password));
+	memset(body, 0, sizeof(body));
+	return retval;
+}
+
+/**
+ * xapea00x_tpm_platform_initialize - performs the minimal
+ * initialization of the TPM normally performed by the platform code
+ * (e.g., BIOS). This consists of executing the TPM startup and
+ * self-test commands and setting the platform authorization password.
+ *
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+int xapea00x_tpm_platform_initialize(struct xapea00x_device *dev)
+{
+	int retval;
+
+	/* wait for TPM to be ready */
+	retval =  xapea00x_tpm_wait_reg8(dev, TPM_ACCESS_0, TPM_ACCESS_VALID,
+					 TPM2_TIMEOUT_A);
+	if (retval)
+		goto out;
+
+	/* issue TPM2_CC_STARTUP command */
+	retval = xapea00x_tpm_startup(dev);
+	if (retval) {
+		dev_err(&dev->interface->dev, "TPM startup failed with %d\n",
+			retval);
+		goto out;
+	}
+
+	/* issue TPM2_SELF_TEST command */
+	retval = xapea00x_tpm_self_test(dev);
+	if (retval) {
+		dev_err(&dev->interface->dev, "TPM self-test failed with %d\n",
+			retval);
+		goto out;
+	}
+
+	/*
+	 * The TPM will enter dictionary lockout mode if turned off
+	 * too many times without a proper shutdown. For the
+	 * "thumb-drive"-esque demo devices, this happens whenever it
+	 * is unplugged. Dictionary attacks against the demo devices
+	 * (XAP-EA-00{1,2}) don't matter, so reset the lockout on every
+	 * boot. Production devices (XAP-EA-003) are internal mPCI-e
+	 * devices that should not be hot-plugged, so do not need to be
+	 * reset.
+	 */
+	if (dev->pid == USB_PRODUCT_ID_XAPEA001 ||
+	    dev->pid == USB_PRODUCT_ID_XAPEA002) {
+		retval = xapea00x_tpm_dict_attack_lock_reset(dev);
+		if (retval) {
+			dev_err(&dev->interface->dev,
+				"Resetting TPM lockout failed with %d\n",
+				retval);
+			goto out;
+		}
+	}
+
+	/* set the platform authorization to random bytes */
+	retval = xapea00x_tpm_randomize_platform_auth(dev);
+	if (retval) {
+		dev_err(&dev->interface->dev,
+			"Setting TPM platform auth failed with %d\n",
+			retval);
+		goto out;
+	}
+
+	retval = 0;
+
+out:
+	return retval;
+}
-- 
2.16.3

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jason Gunthorpe]

On Fri, May 04, 2018 at 08:00:22AM -0500, David R. Bild wrote:
> Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
> for performing initialization of the TPM.  For these modules, the host
> kernel is the platform, so we perform the initialization in the driver
> before registering the TPM with the kernel TPM subsystem.
> 
> The initialization consists of issuing the TPM startup command,
> running the TPM self-test, and setting the TPM platform hierarchy
> authorization to a random, unsaved value so that it can never be used
> after the driver has loaded.

The tpm driver already does most of this stuff automatically, why
duplicate it there and why is it coded in a way that doesn't use the
existing TPM services to do it?

Make no sense to me.

Jason

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Fri, May 4, 2018 at 2:06 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Fri, May 04, 2018 at 08:00:22AM -0500, David R. Bild wrote:
> > Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
> > for performing initialization of the TPM.  For these modules, the host
> > kernel is the platform, so we perform the initialization in the driver
> > before registering the TPM with the kernel TPM subsystem.
>
> The tpm driver already does most of this stuff automatically, why
> duplicate it there and why is it coded in a way that doesn't use the
> existing TPM services to do it?

I didn't want to have to duplicate all that functionality and was
disappointed when that became the only option (due to the two reasons
outlined below) for supporting existing kernels with an out-of-tree
module.

Bringing the module in-tree opens the option of reworking some of the
TPM subsystem to support this use case.  I'm open to concrete
suggestions on how to do so.

1) The first reason is that I don't think the necessary pieces are
currently made available for reuse. I'd love to not repeat that code,
but

- some required structs and functions are declared in private headers
(drivers/char/tpm/*.h instead of include/linux/tpm.h).
- many of the required functions are not exported.

If the TPM maintainers are open to more of the API being "public", I
can look into preparing patches that export the necessary operations.

2) The second reason is that the initialization done by the driver is
work that should be done by platform, before the kernel ever sees the
TPM.

In particular, it sets the credentials for the platform hierarchy.
The platform hierarchy is essentially the "root" account of the TPM,
so it's critical that those credentials be set before the TPM is
exposed to user-space.  (The platform credentials aren't persisted in
the TPM and must be set by the platform on every boot.)  If the driver
registers the TPM before doing initialization, there's a chance that
something else could access the TPM before the platform credentials
get set.

The TPM system doesn't appear (to me; please correct me) to be
designed to support the following start-up sequence:

a) Driver registers the device with the TPM subsystem
b) Driver uses the TPM subsystem to perform platform initialization
(what the BIOS should have done...)
c) Driver tells the TPM subsystem to now perform the kernel
initialization of the TPM
d) TPM subsystem does its initialization of the TPM
e) TPM is exposed to userspace, added as hwrng, etc.

It would only support the sequence  [a, d, e, b] and we'd just have to
hope no one accesses the TPM between steps 'e' and 'b'.

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Fri, May 4, 2018 at 2:56 PM, David R. Bild <david.bild@xaptum.com> wrote:
> 2) The second reason is that the initialization done by the driver is
> work that should be done by platform, before the kernel ever sees the
> TPM.
>
> In particular, it sets the credentials for the platform hierarchy.
> The platform hierarchy is essentially the "root" account of the TPM,
> so it's critical that those credentials be set before the TPM is
> exposed to user-space.  (The platform credentials aren't persisted in
> the TPM and must be set by the platform on every boot.)  If the driver
> registers the TPM before doing initialization, there's a chance that
> something else could access the TPM before the platform credentials
> get set.

Setting the platform hierarchy password to a random discarded value
(and the dictionary lockout reset) is really the only special work
done here. The other steps (startup, self test, etc.) are done by the
TPM subsystem if needed.

So easy option would be for the TPM subsystem to set the platform
hierarchy password to a random value during device registration, if
needed.  It could either

a) check if the platform hierarchy authorization is null and then
always set a random password

or

b) take a flag when the device is registered indicating whether to do this.

This wouldn't require a significant change to the TPM subsystem
internals and would let me drop nearly the entire second patch from
this series.  (I think the dictionary lockout reset can be done via
the already exported "tpm_send(...)" function.)

Any thoughts on this approach?

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jason Gunthorpe]

On Fri, May 04, 2018 at 03:19:21PM -0500, David R. Bild wrote:
> On Fri, May 4, 2018 at 2:56 PM, David R. Bild <david.bild@xaptum.com> wrote:
> > 2) The second reason is that the initialization done by the driver is
> > work that should be done by platform, before the kernel ever sees the
> > TPM.
> >
> > In particular, it sets the credentials for the platform hierarchy.
> > The platform hierarchy is essentially the "root" account of the TPM,
> > so it's critical that those credentials be set before the TPM is
> > exposed to user-space.  (The platform credentials aren't persisted in
> > the TPM and must be set by the platform on every boot.)  If the driver
> > registers the TPM before doing initialization, there's a chance that
> > something else could access the TPM before the platform credentials
> > get set.
> 
> Setting the platform hierarchy password to a random discarded value
> (and the dictionary lockout reset) is really the only special work
> done here. The other steps (startup, self test, etc.) are done by the
> TPM subsystem if needed.
> 
> So easy option would be for the TPM subsystem to set the platform
> hierarchy password to a random value during device registration, if
> needed.  It could either

This would probably make more sense, I'm not opposed at least

> This wouldn't require a significant change to the TPM subsystem
> internals and would let me drop nearly the entire second patch from
> this series.  (I think the dictionary lockout reset can be done via
> the already exported "tpm_send(...)" function.)

Sounds like a much better approach to me.

Jason

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Sun, May 06, 2018 at 09:02:29AM -0600, Jason Gunthorpe wrote:
> On Fri, May 04, 2018 at 03:19:21PM -0500, David R. Bild wrote:
> > Setting the platform hierarchy password to a random discarded value
> > (and the dictionary lockout reset) is really the only special work
> > done here. The other steps (startup, self test, etc.) are done by the
> > TPM subsystem if needed.
> > 
> > So easy option would be for the TPM subsystem to set the platform
> > hierarchy password to a random value during device registration, if
> > needed.  It could either
> 
> This would probably make more sense, I'm not opposed at least
> 
> > This wouldn't require a significant change to the TPM subsystem
> > internals and would let me drop nearly the entire second patch from
> > this series.  (I think the dictionary lockout reset can be done via
> > the already exported "tpm_send(...)" function.)
> 
> Sounds like a much better approach to me.
> 
> Jason

Yes this part but I have absolutely zero understanding about what was
explained before this part (the *longer* part in the email).

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
> On Fri, May 4, 2018 at 2:06 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Fri, May 04, 2018 at 08:00:22AM -0500, David R. Bild wrote:
> > > Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
> > > for performing initialization of the TPM.  For these modules, the host
> > > kernel is the platform, so we perform the initialization in the driver
> > > before registering the TPM with the kernel TPM subsystem.
> >
> > The tpm driver already does most of this stuff automatically, why
> > duplicate it there and why is it coded in a way that doesn't use the
> > existing TPM services to do it?
> 
> I didn't want to have to duplicate all that functionality and was
> disappointed when that became the only option (due to the two reasons
> outlined below) for supporting existing kernels with an out-of-tree
> module.
> 
> Bringing the module in-tree opens the option of reworking some of the
> TPM subsystem to support this use case.  I'm open to concrete
> suggestions on how to do so.
> 
> 1) The first reason is that I don't think the necessary pieces are
> currently made available for reuse. I'd love to not repeat that code,
> but
> 
> - some required structs and functions are declared in private headers
> (drivers/char/tpm/*.h instead of include/linux/tpm.h).
> - many of the required functions are not exported.
> 
> If the TPM maintainers are open to more of the API being "public", I
> can look into preparing patches that export the necessary operations.
> 
> 2) The second reason is that the initialization done by the driver is
> work that should be done by platform, before the kernel ever sees the
> TPM.

This is too speculative to give any confirmitive promises. Do not fully
understand the reasoning. For example: why should I care about
out-of-tree modules? I can look code changes but the text above contains
too many words to nail anything down. I'm confused.

> In particular, it sets the credentials for the platform hierarchy.
> The platform hierarchy is essentially the "root" account of the TPM,
> so it's critical that those credentials be set before the TPM is
> exposed to user-space.  (The platform credentials aren't persisted in
> the TPM and must be set by the platform on every boot.)  If the driver
> registers the TPM before doing initialization, there's a chance that
> something else could access the TPM before the platform credentials
> get set.

Maybe. Not sure yet where to draw the line eg should TSS2 daemon to do
it for example.

James? Philip?

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[James Bottomley]

On Tue, 2018-05-08 at 13:55 +0300, Jarkko Sakkinen wrote:
> On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
[...]
> > In particular, it sets the credentials for the platform hierarchy.
> > The platform hierarchy is essentially the "root" account of the
> > TPM, so it's critical that those credentials be set before the TPM
> > is exposed to user-space.  (The platform credentials aren't
> > persisted in the TPM and must be set by the platform on every
> > boot.)  If the driver registers the TPM before doing
> > initialization, there's a chance that something else could access
> > the TPM before the platform credentials get set.
> 
> Maybe. Not sure yet where to draw the line eg should TSS2 daemon to
> do it for example.
> 
> James? Philip?

I don't see any reason to set an unreachable password for the platform
hierarchy if the UEFI didn't.  If the desire is to disable the platform
hierarchy, then it should be disabled, not have a random password set. 
I'd also say this is probably the job of early boot based on policy.

James

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Tue, May 8, 2018 at 10:25 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> > On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
> [...]
> > > In particular, it sets the credentials for the platform hierarchy.
> > > The platform hierarchy is essentially the "root" account of the
> > > TPM, so it's critical that those credentials be set before the TPM
> > > is exposed to user-space.  (The platform credentials aren't
> > > persisted in the TPM and must be set by the platform on every
> > > boot.)  If the driver registers the TPM before doing
> > > initialization, there's a chance that something else could access
> > > the TPM before the platform credentials get set.
>
> I don't see any reason to set an unreachable password for the platform
> hierarchy if the UEFI didn't.  If the desire is to disable the platform
> hierarchy, then it should be disabled, not have a random password set.

"Set random password and throw away the key" was my way of disabling
the platform hierarchy.  Is there a better way of doing that?

> I'd also say this is probably the job of early boot based on policy.

Agreed.  And since this card has no "early boot", the driver/kernel
need to do it.

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[James Bottomley]

On Tue, 2018-05-08 at 10:29 -0500, David R. Bild wrote:
> On Tue, May 8, 2018 at 10:25 AM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > 
> > > On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
> > 
> > [...]
> > > > In particular, it sets the credentials for the platform
> > > > hierarchy.
> > > > The platform hierarchy is essentially the "root" account of the
> > > > TPM, so it's critical that those credentials be set before the
> > > > TPM
> > > > is exposed to user-space.  (The platform credentials aren't
> > > > persisted in the TPM and must be set by the platform on every
> > > > boot.)  If the driver registers the TPM before doing
> > > > initialization, there's a chance that something else could
> > > > access
> > > > the TPM before the platform credentials get set.
> > 
> > I don't see any reason to set an unreachable password for the
> > platform
> > hierarchy if the UEFI didn't.  If the desire is to disable the
> > platform
> > hierarchy, then it should be disabled, not have a random password
> > set.
> 
> "Set random password and throw away the key" was my way of disabling
> the platform hierarchy.  Is there a better way of doing that?

Well, yes, use TPM2_HierarchyControl to set phEnable to CLEAR.

> > I'd also say this is probably the job of early boot based on
> > policy.
> 
> Agreed.  And since this card has no "early boot", the driver/kernel
> need to do it.

Early boot means userspace. for a hot pluggable device, this would
probably be something in udev if you follow the no-daemon model and the
daemon could do it if you do follow the daemon model.

James

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Tue, May 08, 2018 at 08:36:25AM -0700, James Bottomley wrote:
> Early boot means userspace. for a hot pluggable device, this would
> probably be something in udev if you follow the no-daemon model and the
> daemon could do it if you do follow the daemon model.
> 
> James

Could this be implemented as a first priority to daemon. If it turns out
to be bad approach we can reconsider kernel. If we land it to kernel it
is harder to take steps back.

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Wed, May 9, 2018 at 8:59 PM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Tue, May 08, 2018 at 08:36:25AM -0700, James Bottomley wrote:
>> Early boot means userspace. for a hot pluggable device, this would
>> probably be something in udev if you follow the no-daemon model and the
>> daemon could do it if you do follow the daemon model.
>>
>
> Could this be implemented as a first priority to daemon. If it turns out
> to be bad approach we can reconsider kernel. If we land it to kernel it
> is harder to take steps back.

Is the daemon an implementation of the TCG resource manager spec?

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Thu, May 10, 2018 at 09:31:07AM -0500, David R. Bild wrote:
> On Wed, May 9, 2018 at 8:59 PM, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > On Tue, May 08, 2018 at 08:36:25AM -0700, James Bottomley wrote:
> >> Early boot means userspace. for a hot pluggable device, this would
> >> probably be something in udev if you follow the no-daemon model and the
> >> daemon could do it if you do follow the daemon model.
> >>
> >
> > Could this be implemented as a first priority to daemon. If it turns out
> > to be bad approach we can reconsider kernel. If we land it to kernel it
> > is harder to take steps back.
> 
> Is the daemon an implementation of the TCG resource manager spec?
> 
> Best,
> David

Philip and James would be the right people to answer this. Ping? :-)

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Ken Goldman]

On 5/10/2018 10:31 AM, David R. Bild wrote:
>>
>> Could this be implemented as a first priority to daemon. If it turns out
>> to be bad approach we can reconsider kernel. If we land it to kernel it
>> is harder to take steps back.
> 
> Is the daemon an implementation of the TCG resource manager spec?

The TCG spec does use a daemon approach, similar to tcsd for TPM 1.2.

The Linux TPM driver is currently using a different approach, a 
in-kernel (in the TPM device driver) resource manager.

The advantages I see to putting the resource manager in the device 
driver are:

1 - Kernel uses of the TPM go through the same device driver, so they 
leverage the resource manager.

2 - The TPM device driver offers a standard /dev/tpmrm0 interface,
100% compatible with /dev/tpm0.

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Tue, May 8, 2018 at 10:36 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Tue, 2018-05-08 at 10:29 -0500, David R. Bild wrote:
> > On Tue, May 8, 2018 at 10:25 AM, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > >
> > > I don't see any reason to set an unreachable password for the
> > > platform
> > > hierarchy if the UEFI didn't.  If the desire is to disable the
> > > platform
> > > hierarchy, then it should be disabled, not have a random password
> > > set.
> >
> > "Set random password and throw away the key" was my way of disabling
> > the platform hierarchy.  Is there a better way of doing that?
>
> Well, yes, use TPM2_HierarchyControl to set phEnable to CLEAR.


I'm not sure that will work for us.  Let me give a little more detail
about this card.

The TPM holds access credentials for connecting to the Xaptum network.
This approach enables secure, zero-touch provisioning for IoT devices:
 Xaptum pre-provisions the TPMs *before* they are assembled onto a
device PCB. The device is shipped directly from factory to end
customer. The first time it turns on, the TPM is used to authenticate
the Xaptum network. Using a TPM protects the credentials from being
copied or duplicated by someone in the manufacturing chain.

These cards are designed for existing devices, like IoT gateways. You
can't add a TPM to an existing PCB, but you can plug in a mini PCI-e
card.

We provision the credentials (the DAA secret key, specifically) under
the platform hierarchy. The key can be used without platform
authorization, but not removed.  If we disable the platform hierarchy
entirely, I think the credentials will no longer be available for use.

> > > I'd also say this is probably the job of early boot based on
> > > policy.
> >
> > Agreed.  And since this card has no "early boot", the driver/kernel
> > need to do it.
>
> Early boot means userspace. for a hot pluggable device, this would
> probably be something in udev if you follow the no-daemon model and the
> daemon could do it if you do follow the daemon model.

Could you expand on the udev approach?  I might not understand enough
about udev (or the coming TPM resource manager changes) to follow the
suggestion.

This seems unsafe to me.  There's a race between a malicious userspace
program and the daemon to set the platform authorization.  If the
malicious program wins, it can reset the TPM, removing the
credentials, and the device won't be able to connect to the Xaptum
network. (This is a liveness concern, not safety.  A denial-of-service
attack, essentially.)

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[James Bottomley]

On Thu, 2018-05-10 at 09:25 -0500, David R. Bild wrote:
> On Tue, May 8, 2018 at 10:36 AM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > 
> > On Tue, 2018-05-08 at 10:29 -0500, David R. Bild wrote:
> > > On Tue, May 8, 2018 at 10:25 AM, James Bottomley
> > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > 
> > > > I don't see any reason to set an unreachable password for the
> > > > platform hierarchy if the UEFI didn't.  If the desire is to
> > > > disable the platform hierarchy, then it should be disabled, not
> > > > have a random password set.
> > > 
> > > "Set random password and throw away the key" was my way of
> > > disabling the platform hierarchy.  Is there a better way of doing
> > > that?
> > 
> > Well, yes, use TPM2_HierarchyControl to set phEnable to CLEAR.
> 
> 
> I'm not sure that will work for us.  Let me give a little more detail
> about this card.
> 
> The TPM holds access credentials for connecting to the Xaptum
> network. This approach enables secure, zero-touch provisioning for
> IoT devices:  Xaptum pre-provisions the TPMs *before* they are
> assembled onto a device PCB. The device is shipped directly from
> factory to end customer. The first time it turns on, the TPM is used
> to authenticate the Xaptum network. Using a TPM protects the
> credentials from being copied or duplicated by someone in the
> manufacturing chain.

OK, so these are effectively DevId keys.  However, what makes you think
knowing the platform auth allows you to duplicate the keys?  As long as
you created them correctly (as in without duplication authority) then
even knowing the platform authorization I can't get them out of your
TPM.

> These cards are designed for existing devices, like IoT gateways. You
> can't add a TPM to an existing PCB, but you can plug in a mini PCI-e
> card.
> 
> We provision the credentials (the DAA secret key, specifically) under
> the platform hierarchy. The key can be used without platform
> authorization, but not removed.  If we disable the platform hierarchy
> entirely, I think the credentials will no longer be available for
> use.

That's certainly true if you actually need to use the platform
hierarchy.  Your initial emails on the subject did say you were
disabling it though ...

> > > > I'd also say this is probably the job of early boot based on
> > > > policy.
> > > 
> > > Agreed.  And since this card has no "early boot", the
> > > driver/kernel need to do it.
> > 
> > Early boot means userspace. for a hot pluggable device, this would
> > probably be something in udev if you follow the no-daemon model and
> > the daemon could do it if you do follow the daemon model.
> 
> Could you expand on the udev approach?  I might not understand enough
> about udev (or the coming TPM resource manager changes) to follow the
> suggestion.
> 
> This seems unsafe to me.  There's a race between a malicious
> userspace program and the daemon to set the platform
> authorization.  If the malicious program wins, it can reset the TPM,
> removing the credentials, and the device won't be able to connect to
> the Xaptum network. (This is a liveness concern, not safety.  A
> denial-of-service attack, essentially.)

OK, I'm getting confused by your threat model.  I don't think knowing
the platform auth I can obtain your keys.  However, I agree, I can
definitely remove them.  However, setting platform auth doesn't solve
this: I can execute a TPM2_Clear to regain the platform auth and if you
disable this, I can't re-own the TPM at all.

James

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Thu, May 10, 2018 at 9:47 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Thu, 2018-05-10 at 09:25 -0500, David R. Bild wrote:
>> The TPM holds access credentials for connecting to the Xaptum
>> network.
>
> OK, so these are effectively DevId keys.  However, what makes you think
> knowing the platform auth allows you to duplicate the keys?

It doesn't and we don't think that.

> As long as
> you created them correctly (as in without duplication authority) then
> even knowing the platform authorization I can't get them out of your
> TPM.

Correct.  No one can copy/duplicate/read them.

But they can delete them, which is effectively a denial of service
attack against the device.

>> We provision the credentials (the DAA secret key, specifically) under
>> the platform hierarchy. The key can be used without platform
>> authorization, but not removed.  If we disable the platform hierarchy
>> entirely, I think the credentials will no longer be available for
>> use.
>
> That's certainly true if you actually need to use the platform
> hierarchy.  Your initial emails on the subject did say you were
> disabling it though ...

Mea culpa.  Lazy wording on my part.

>> > Early boot means userspace. for a hot pluggable device, this would
>> > probably be something in udev if you follow the no-daemon model and
>> > the daemon could do it if you do follow the daemon model.
>>
>> Could you expand on the udev approach?  I might not understand enough
>> about udev (or the coming TPM resource manager changes) to follow the
>> suggestion.

>> This seems unsafe to me.  There's a race between a malicious
>> userspace program and the daemon to set the platform
>> authorization.  If the malicious program wins, it can reset the TPM,
>> removing the credentials, and the device won't be able to connect to
>> the Xaptum network. (This is a liveness concern, not safety.  A
>> denial-of-service attack, essentially.)
>
> OK, I'm getting confused by your threat model.  I don't think knowing
> the platform auth I can obtain your keys.  However, I agree, I can
> definitely remove them.

Correct. Removal (not copying) is our concern.

>  However, setting platform auth doesn't solve
> this: I can execute a TPM2_Clear to regain the platform auth and if you
> disable this

According to the spec (v1.38) TPM2_Clear

- flushes the Storage and Endorsement hierarchies, not the Platform hierarchy.

- resets the Storage, Endorsement, and Lockout auth, but not the Platform auth.

> I can't re-own the TPM at all.

You can execute TPM2_Clear  (if you have lockout auth. We don't set
lockout auth, so you will.) to regain control of the Storage and
Endorsement hierarchies.  We only control the platform hierarchy.

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Ken Goldman]

On 5/8/2018 11:36 AM, James Bottomley wrote:
> On Tue, 2018-05-08 at 10:29 -0500, David R. Bild wrote:
>> On Tue, May 8, 2018 at 10:25 AM, James Bottomley

>>>
>>> I don't see any reason to set an unreachable password for the
>>> platform
>>> hierarchy if the UEFI didn't.  If the desire is to disable the
>>> platform
>>> hierarchy, then it should be disabled, not have a random password
>>> set.
>>
>> "Set random password and throw away the key" was my way of disabling
>> the platform hierarchy.  Is there a better way of doing that?
> 
> Well, yes, use TPM2_HierarchyControl to set phEnable to CLEAR.

There is a huge difference between the two.

"Set a random password" is the recommended approach.  This just
prohibits using the platform authorization - a good idea.

phEnable CLEAR disables the hierarchy, preventing it from being used
at all.  A basic problem would be that the EK certificates could not be
read.

There are likely to be other issues, like not being able to do a field 
upgrade post-OS,

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Tue, May 08, 2018 at 10:29:41AM -0500, David R. Bild wrote:
> On Tue, May 8, 2018 at 10:25 AM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > > On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
> > [...]
> > > > In particular, it sets the credentials for the platform hierarchy.
> > > > The platform hierarchy is essentially the "root" account of the
> > > > TPM, so it's critical that those credentials be set before the TPM
> > > > is exposed to user-space.  (The platform credentials aren't
> > > > persisted in the TPM and must be set by the platform on every
> > > > boot.)  If the driver registers the TPM before doing
> > > > initialization, there's a chance that something else could access
> > > > the TPM before the platform credentials get set.
> >
> > I don't see any reason to set an unreachable password for the platform
> > hierarchy if the UEFI didn't.  If the desire is to disable the platform
> > hierarchy, then it should be disabled, not have a random password set.
> 
> "Set random password and throw away the key" was my way of disabling
> the platform hierarchy.  Is there a better way of doing that?
> 
> > I'd also say this is probably the job of early boot based on policy.
> 
> Agreed.  And since this card has no "early boot", the driver/kernel
> need to do it.
> 
> Best,
> David

Who is able to test these changes if we even consider pulling them?
I do not have such a card so it will be hard to accept also given
that it is more intrusive change than usual.

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Wed, May 9, 2018 at 8:44 PM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Tue, May 08, 2018 at 10:29:41AM -0500, David R. Bild wrote:
>> On Tue, May 8, 2018 at 10:25 AM, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> >
>> > > On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
>> > [...]
>> > > > In particular, it sets the credentials for the platform hierarchy.
>> > > > The platform hierarchy is essentially the "root" account of the
>> > > > TPM, so it's critical that those credentials be set before the TPM
>> > > > is exposed to user-space.  (The platform credentials aren't
>> > > > persisted in the TPM and must be set by the platform on every
>> > > > boot.)  If the driver registers the TPM before doing
>> > > > initialization, there's a chance that something else could access
>> > > > the TPM before the platform credentials get set.
>> >
>
> Who is able to test these changes if we even consider pulling them?

I can send you and the other maintainers cards to test with. That's
dead simple.  (With a USB-A plug, not mini PCI-e, so you can plug it
into any computer.)

They won't have the Xaptum credentials pre-provisioned, and will just
function as normal TPMs.

> I do not have such a card so it will be hard to accept also given
> that it is more intrusive change than usual.

The current approach (the driver does all the initialization) requires
no changes to the TPM driver.  Only someone who buys our card will
ever run that code, so it doesn't impact anyone else.

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Tue, May 08, 2018 at 08:25:48AM -0700, James Bottomley wrote:
> On Tue, 2018-05-08 at 13:55 +0300, Jarkko Sakkinen wrote:
> > On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
> [...]
> > > In particular, it sets the credentials for the platform hierarchy.
> > > The platform hierarchy is essentially the "root" account of the
> > > TPM, so it's critical that those credentials be set before the TPM
> > > is exposed to user-space.  (The platform credentials aren't
> > > persisted in the TPM and must be set by the platform on every
> > > boot.)  If the driver registers the TPM before doing
> > > initialization, there's a chance that something else could access
> > > the TPM before the platform credentials get set.
> > 
> > Maybe. Not sure yet where to draw the line eg should TSS2 daemon to
> > do it for example.
> > 
> > James? Philip?
> 
> I don't see any reason to set an unreachable password for the platform
> hierarchy if the UEFI didn't.  If the desire is to disable the platform
> hierarchy, then it should be disabled, not have a random password set. 
> I'd also say this is probably the job of early boot based on policy.
> 
> James

A valid point.

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Tue, May 8, 2018 at 5:55 AM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> On Fri, May 04, 2018 at 02:56:25PM -0500, David R. Bild wrote:
>> On Fri, May 4, 2018 at 2:06 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
>> >
>> > On Fri, May 04, 2018 at 08:00:22AM -0500, David R. Bild wrote:
>> > > Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
>> > > for performing initialization of the TPM.  For these modules, the host
>> > > kernel is the platform, so we perform the initialization in the driver
>> > > before registering the TPM with the kernel TPM subsystem.
>> >
>> > The tpm driver already does most of this stuff automatically, why
>> > duplicate it there and why is it coded in a way that doesn't use the
>> > existing TPM services to do it?
>>
>> I didn't want to have to duplicate all that functionality and was
>> disappointed when that became the only option (due to the two reasons
>> outlined below) for supporting existing kernels with an out-of-tree
>> module.
>>
>> Bringing the module in-tree opens the option of reworking some of the
>> TPM subsystem to support this use case.  I'm open to concrete
>> suggestions on how to do so.
>>
>> 1) The first reason is that I don't think the necessary pieces are
>> currently made available for reuse. I'd love to not repeat that code,
>> but
>>
>> - some required structs and functions are declared in private headers
>> (drivers/char/tpm/*.h instead of include/linux/tpm.h).
>> - many of the required functions are not exported.
>>
>> If the TPM maintainers are open to more of the API being "public", I
>> can look into preparing patches that export the necessary operations.
>>
>> 2) The second reason is that the initialization done by the driver is
>> work that should be done by platform, before the kernel ever sees the
>> TPM.
>
> This is too speculative to give any confirmitive promises. Do not fully
> understand the reasoning. For example: why should I care about
> out-of-tree modules?

You shouldn't care about out-of-tree modules, per se.  But the same
issues affect *in-tree* modules too.

Currently there is no way for an in-tree driver to accomplish all
three of the following

1) Disable the platform hierarchy (or set the platform auth password).
2) Use the TPM subsystem to accomplish step 1 (as opposed to
reimplementing startup, self-test, etc. which this driver currently
does).
3) Ensure that step 1 is done before the TPM is exposed to userspace
as /dev/tpmX (to prevent a race with userspace for control of the
platform hierarchy).

> I can look code changes but the text above contains
> too many words to nail anything down. I'm confused.

I see four possible resolutions:

1) Accept the code as is. No changes to the TPM subsystem. I do not
like this option.

2) Make more of the TPM driver API public (internally public) so that
the driver can reuse that code instead of reimplementing.  Not ideal,
as this could require major restructuring of the TPM code.

3) Allow the driver to register the TPM with TPM driver, but not yet
expose the TPM to userspace.  Let the driver do some additional work
(like set the platform hierarchy password) and then explicitly inform
the TPM driver that it is safe to expose the TPM to userspace.  This
would be my preferred approach.

4) The TPM driver (or daemon, perhaps. I don't under James's proposal
yet) sets the platform password.  As you say, this is an intrusive
change and only for a very specific use case right now. Perhaps not
ideal.

Do these make sense?

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Thu, May 10, 2018 at 09:41:53AM -0500, David R. Bild wrote:
> 2) Make more of the TPM driver API public (internally public) so that
> the driver can reuse that code instead of reimplementing.  Not ideal,
> as this could require major restructuring of the TPM code.

Can you open this up a bit? It depends what this means in practice.

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jason Gunthorpe]

On Thu, May 10, 2018 at 09:41:53AM -0500, David R. Bild wrote:

> 3) Allow the driver to register the TPM with TPM driver, but not yet
> expose the TPM to userspace.  Let the driver do some additional work
> (like set the platform hierarchy password) and then explicitly inform
> the TPM driver that it is safe to expose the TPM to userspace.  This
> would be my preferred approach.

We already have this. The driver can setup enough to use the TPM
framework to send commands before completing registration. We use it
in startup timeouts and other flows today.

Jason

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Mon, May 14, 2018 at 2:31 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, May 10, 2018 at 09:41:53AM -0500, David R. Bild wrote:
>
> > 3) Allow the driver to register the TPM with TPM driver, but not yet
> > expose the TPM to userspace.  Let the driver do some additional work
> > (like set the platform hierarchy password) and then explicitly inform
> > the TPM driver that it is safe to expose the TPM to userspace.  This
> > would be my preferred approach.
>
> We already have this. The driver can setup enough to use the TPM
> framework to send commands before completing registration. We use it
> in startup timeouts and other flows today.


That sounds perfect.  Can you point me to some usages in the code (or
relevant functions)?

This driver registers with the TPM subsystem using the "tpm_tis_spi"
driver like this:

"
  static struct spi_board_info tpm_board_info = {
      .modalias           = "tpm_tis_spi",
      .max_speed_hz = 43 * 1000 * 1000, // Hz
      .chip_select       = 0,
      .mode                = SPI_MODE_0
  };

  struct spi_device *tpm = spi_new_device(spi_master, &tpm_board_info);
"

I don't see how sending of commands before completing registration.
At the very least, the "tpm_tis_spi" driver probably has to be
changed?

Best,
David

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jason Gunthorpe]

On Mon, May 14, 2018 at 02:59:36PM -0500, David R. Bild wrote:
> On Mon, May 14, 2018 at 2:31 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Thu, May 10, 2018 at 09:41:53AM -0500, David R. Bild wrote:
> >
> > > 3) Allow the driver to register the TPM with TPM driver, but not yet
> > > expose the TPM to userspace.  Let the driver do some additional work
> > > (like set the platform hierarchy password) and then explicitly inform
> > > the TPM driver that it is safe to expose the TPM to userspace.  This
> > > would be my preferred approach.
> >
> > We already have this. The driver can setup enough to use the TPM
> > framework to send commands before completing registration. We use it
> > in startup timeouts and other flows today.
> 
> 
> That sounds perfect.  Can you point me to some usages in the code (or
> relevant functions)?
> 
> This driver registers with the TPM subsystem using the "tpm_tis_spi"
> driver like this:
> 
> "
>   static struct spi_board_info tpm_board_info = {
>       .modalias           = "tpm_tis_spi",
>       .max_speed_hz = 43 * 1000 * 1000, // Hz
>       .chip_select       = 0,
>       .mode                = SPI_MODE_0
>   };
> 
>   struct spi_device *tpm = spi_new_device(spi_master, &tpm_board_info);
> "
> 
> I don't see how sending of commands before completing registration.
> At the very least, the "tpm_tis_spi" driver probably has to be
> changed?

Well, tpm_tis_core_init() does it

Only the driver that calls tpm_chip_register() gets to use this
capability, so you can't use "tpm_tis_spi" and the automatic binding
if you need it.

Looks like the spi binding and tpm_tis_core_init will need some gentle
editing to allow a SPI based driver the same opportunity.

Jason

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Mon, May 14, 2018 at 3:08 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Mon, May 14, 2018 at 02:59:36PM -0500, David R. Bild wrote:
>> On Mon, May 14, 2018 at 2:31 PM, Jason Gunthorpe <jgg@ziepe.ca> wrote:
 The driver can setup enough to use the TPM
>> > framework to send commands before completing registration. We use it
>> > in startup timeouts and other flows today.
>>
>>
>> That sounds perfect.  Can you point me to some usages in the code (or
>> relevant functions)?
>>
>
> Well, tpm_tis_core_init() does it
>
> Only the driver that calls tpm_chip_register() gets to use this
> capability, so you can't use "tpm_tis_spi" and the automatic binding
> if you need it.
>
> Looks like the spi binding and tpm_tis_core_init will need some gentle
> editing to allow a SPI based driver the same opportunity.

Thanks for the pointers.  I'll work with Peter to add this capability
for SPI drivers.

Best,
David

RE: EXTERNAL: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jeremy Boone]

Hi there,

I have some feedback. A few months ago, I disclosed a series of bugs to this list that could be triggered by an interposer that sits on the serial bus between the TPM chip and the kernel.  These patches can be found here [1], and the tool/whitepaper to go with my research can be found here [2].

I noticed that your change set contains similar flaws that may be triggered by an interposer.

================
Unsafe parsing of TPM response header
================

The function xapea00x_tpm_recv is used to receive bytes from the TPM after a command has been issued. This function does not adequately validate the cmd->size field which is contained in the response header. Below, if cmd->size is less than TIS_HEADER_LEN (10 bytes), then the arithmetic operation "size-=TIS_HEADER_LEN" will underflow.

+static int xapea00x_tpm_recv(struct xapea00x_device *dev, void *buf, u32 len)
+{
...
+retval = xapea00x_tpm_read_bytes(dev, TPM_DATA_FIFO_0, buf,
+ TIS_HEADER_LEN);
+...
+size = __be32_to_cpu(cmd->size);
+if (len < size) {
+retval = -EINVAL;
+goto cancel;
+}
+size -= TIS_HEADER_LEN;

Once the underflow has occurred the value of `size` will be very large. Consequently, the following while-loop will take too many iterations, causing the write of too many bytes into the response buffer, `buf`:

+while (size > TIS_HEADER_LEN) {
+xapea00x_tpm_burst_count(dev, &burst);
+burst = min(burst, size);
+
+retval = xapea00x_tpm_read_bytes(dev, TPM_DATA_FIFO_0, buf,
+ burst);
+if (retval)
+goto cancel;
+
+size -= burst;
+buf += burst;
+}

Here a simple check is needed to ensure that `cmd->size` is not less than `TIS_HEADER_LEN`:

+size = __be32_to_cpu(cmd->size);
+if (size > len || size < TIS_HEADER_LEN) {

================
Use of sessions
================

The xapea00x_tpm_get_random function does not use a session (it uses TPM2_ST_NO_SESSIONS), and as such, the response bytes are not encrypted and are not protected by an HMAC. This allows an interposer device to eavesdrop and/or modify the traffic. James Bottomley recently proposed a patch [3] that would add sessions to TPM2_GET_RANDOM commands, and you may want to take advantage of that in your driver.


[1] https://lkml.org/lkml/2018/2/26/1176
[2] https://github.com/nccgroup/TPMGenie
[3] https://www.spinics.net/lists/linux-integrity/msg02000.html


-----Original Message-----
From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-owner@vger.kernel.org] On Behalf Of David R. Bild
Sent: Friday, May 04, 2018 9:00 AM
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Peter Huewe <peterhuewe@gmx.de>; Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: linux-usb@vger.kernel.org; linux-integrity@vger.kernel.org; David R. Bild <david.bild@xaptum.com>
Subject: EXTERNAL: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
for performing initialization of the TPM.  For these modules, the host
kernel is the platform, so we perform the initialization in the driver
before registering the TPM with the kernel TPM subsystem.

The initialization consists of issuing the TPM startup command,
running the TPM self-test, and setting the TPM platform hierarchy
authorization to a random, unsaved value so that it can never be used
after the driver has loaded.

Signed-off-by: David R. Bild <david.bild@xaptum.com>
---
 drivers/usb/misc/xapea00x/Makefile        |   3 +-
 drivers/usb/misc/xapea00x/xapea00x-core.c |  25 +
 drivers/usb/misc/xapea00x/xapea00x-tpm.c  | 952 ++++++++++++++++++++++++++++++
 3 files changed, 979 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/misc/xapea00x/xapea00x-tpm.c

diff --git a/drivers/usb/misc/xapea00x/Makefile b/drivers/usb/misc/xapea00x/Makefile
index c4bcd7524c31..aa3f8803cdf5 100644
--- a/drivers/usb/misc/xapea00x/Makefile
+++ b/drivers/usb/misc/xapea00x/Makefile
@@ -4,4 +4,5 @@
 #
 obj-$(CONFIG_USB_XAPEA00X) += xapea00x.o

-xapea00x-y += xapea00x-core.o xapea00x-bridge.o
+xapea00x-y += xapea00x-core.o xapea00x-bridge.o xapea00x-tpm.o
+
diff --git a/drivers/usb/misc/xapea00x/xapea00x-core.c b/drivers/usb/misc/xapea00x/xapea00x-core.c
index 885bcda9c01d..53e82f8b38f3 100644
--- a/drivers/usb/misc/xapea00x/xapea00x-core.c
+++ b/drivers/usb/misc/xapea00x/xapea00x-core.c
@@ -280,6 +280,31 @@ static void xapea00x_tpm_probe(struct work_struct *work)
 struct spi_device *tpm;
 int retval;

+mutex_lock(&dev->usb_mutex);
+if (!dev->interface) {
+retval = -ENODEV;
+goto out;
+}
+/*
+ * This driver is the "platform" in TPM terminology. Before
+ * passing control of the TPM to the Linux TPM subsystem, do
+ * the TPM initialization normally done by the platform code
+ * (e.g., BIOS).
+ */
+retval = xapea00x_tpm_platform_initialize(dev);
+if (retval) {
+dev_err(&dev->interface->dev,
+"unable to do TPM platform initialization: %d\n",
+retval);
+goto err;
+}
+
+/*
+ * Now register the TPM with the Linux TPM subsystem.  This
+ * may call through to xapea00x_spi_transfer_one_message(), so
+ * don't hold usb_mutex here.
+ */
+mutex_unlock(&dev->usb_mutex);
 tpm = spi_new_device(spi_master, &tpm_board_info);
 mutex_lock(&dev->usb_mutex);
 if (!dev->interface) {
diff --git a/drivers/usb/misc/xapea00x/xapea00x-tpm.c b/drivers/usb/misc/xapea00x/xapea00x-tpm.c
new file mode 100644
index 000000000000..27159043ce3c
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-tpm.c
@@ -0,0 +1,952 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ *  TPM 2.0-based hardware module for authenticating IoT devices and
+ *  gateways.
+ *
+ *  Copyright (c) 2017-2018 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+#define TPM_RETRY50
+#define TPM_TIMEOUT5    // msecs
+#define TPM_TIMEOUT_RANGE_US300  // usecs
+
+#define TIS_SHORT_TIMEOUT750  // msecs
+#define TIS_LONG_TIMEOUT2000 // msecs
+
+#define TIS_MAX_BUF1024 // byte
+#define TIS_HEADER_LEN10   // byte
+
+#define TPM2_TIMEOUT_A750  // msecs
+#define TPM2_TIMEOUT_B2000 // msecs
+#define TPM2_TIMEOUT_C200  // msecs
+#define TPM2_TIMEOUT_D30   // msecs
+
+#define TPM_ACCESS_00x0000
+#define TPM_STS_00x0018
+#define TPM_DATA_FIFO_00x0024
+
+#define TPM2_ST_NO_SESSIONS0x8001
+#define TPM2_ST_SESSIONS0x8002
+
+#define TPM2_CC_STARTUP0x0144
+#define TPM2_CC_SHUTDOWN0x0145
+#define TPM2_CC_SELF_TEST0x0143
+#define TPM2_CC_GET_RANDOM0x017B
+#define TPM2_CC_HIERARCHY_CHANGE_AUTH0x0129
+#define TPM2_CC_DICT_ATTACK_LOCK_RST0x0139
+
+#define TPM_RC_SUCCESS0x000
+#define TPM_RC_INITIALIZE0x100
+
+enum tis_access {
+TPM_ACCESS_VALID= 0x80,
+TPM_ACCESS_ACTIVE_LOCALITY= 0x20,
+TPM_ACCESS_REQUEST_PENDING= 0x04,
+TPM_ACCESS_REQUEST_USE= 0x02
+};
+
+enum tis_status {
+TPM_STS_VALID= 0x80,
+TPM_STS_COMMAND_READY= 0x40,
+TPM_STS_GO= 0x20,
+TPM_STS_DATA_AVAIL= 0x10,
+TPM_STS_DATA_EXPECT= 0x08,
+TPM_STS_SELF_TEST_DONE= 0x04,
+TPM_STS_RESPONSE_RETRY= 0x02
+};
+
+struct tpm_tis_command {
+__be16 tag;
+__be32 size;
+__be32 code;
+    u8 body[0];
+} __attribute__((__packed__));
+
+/*******************************************************************************
+ * TPM TIS functions
+ */
+
+/**
+ * xapea00x_tpm_msleep - sleep for at least the specified time.
+ * @msecs: minimum duration to sleep for in milliseconds
+ */
+static void xapea00x_tpm_msleep(int msecs)
+{
+usleep_range(msecs * 1000,
+     msecs * 1000 + TPM_TIMEOUT_RANGE_US);
+}
+
+/**
+ * xapea00x_tpm_transfer - execute an SPI transfer.
+ * @dev: pointer to the device
+ * @addr: the TPM TIS register address
+ * @in: if a write or write_read transfer, the data to write
+ * @out: if a read or write_read  transfer, the buffer to read data into
+ * @len: the number of bytes to transfer
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transfer(struct xapea00x_device *dev,
+ u32 addr, u8 *in, u8 *out, u16 len)
+{
+u8 header[4];
+int i, retval;
+
+header[0] = (in ? 0x80 : 0x00) | (len - 1);
+header[1] = 0xd4;
+header[2] = addr >> 8;
+header[3] = addr;
+
+retval = xapea00x_spi_transfer(dev, header, header, 4, 1, 0);
+if (retval)
+goto out;
+
+/* handle SPI wait states */
+if ((header[3] & 0x01) == 0x00) {
+header[0] = 0;
+
+for (i = 0; i < TPM_RETRY; i++) {
+retval = xapea00x_spi_transfer(dev, header, header, 1,
+       1, 0);
+if (retval)
+goto out;
+if ((header[0] & 0x01) == 00)
+break;
+}
+
+if (i == TPM_RETRY) {
+retval = -ETIMEDOUT;
+goto out;
+}
+}
+
+retval = xapea00x_spi_transfer(dev, out, in, len, 0, 0);
+if (retval)
+goto out;
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_read_bytes - read data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @result: buffer to in which to place the read data
+ * @len: the number of bytes to read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_read_bytes(struct xapea00x_device *dev, u32 addr,
+   void *result, u16 len)
+{
+return xapea00x_tpm_transfer(dev, addr, result, NULL, len);
+}
+
+/**
+ * xapea00x_tpm_write_bytes - write data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to write to
+ * @data: pointer to the data to write
+ * @len: the number of bytes to read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_write_bytes(struct xapea00x_device *dev, u32 addr,
+    void *data, u16 len)
+{
+return xapea00x_tpm_transfer(dev, addr, NULL, data, len);
+}
+
+/**
+ * xapea00x_tpm_read8 - read one byte of data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @result: pointer to the destination
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_read8(struct xapea00x_device *dev, u32 addr, u8 *result)
+{
+return xapea00x_tpm_read_bytes(dev, addr, result, 1);
+}
+
+/**
+ * xapea00x_tpm_write8 - write one byte of data to the TPM
+ * @dev: pointer to the device
+ * @addr: the register to write to
+ * @data:  the byte to write
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_write8(struct xapea00x_device *dev, u32 addr, u8 data)
+{
+return xapea00x_tpm_write_bytes(dev, addr, &data, 1);
+}
+
+/**
+ * xapea00x_tpm_read32 - read one integer of data from the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @result: pointer to the destination
+ *
+ * The method performs any required endianness conversion on the
+ * result.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_read32(struct xapea00x_device *dev, u32 addr,
+       u32 *result)
+{
+__le32 result_le;
+int retval;
+
+retval = xapea00x_tpm_read_bytes(dev, addr, &result_le,
+ sizeof(result_le));
+if (retval)
+goto out;
+
+*result = __le32_to_cpu(result_le);
+retval = 0;
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_write32 - write one integer of data to the TPM
+ * @dev: pointer to the device
+ * @addr: the register to read from
+ * @data: the integer to write
+ *
+ * The method performs any required endianness conversion on the
+ * data.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_write32(struct xapea00x_device *dev, u32 addr, u32 data)
+{
+__le32 data_le;
+
+data_le = __cpu_to_le32(data);
+return xapea00x_tpm_write_bytes(dev, addr, &data_le, sizeof(data_le));
+}
+
+/**
+ * xapea00x_tpm_wait_reg8 - waits for the specified flags on the
+ * register to be set.
+ * @dev: pointer to the device
+ * @addr: the register to check
+ * @flags: mask of the flags to check
+ * @timeout_msecs: maximum amount of time to wait in milliseconds
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_wait_reg8(struct xapea00x_device *dev,
+  u8 addr, u8 flags,
+  int timeout_msecs)
+{
+unsigned long stop = jiffies + msecs_to_jiffies(timeout_msecs);
+u8 reg;
+int retval;
+
+do {
+retval = xapea00x_tpm_read8(dev, addr, &reg);
+if (retval)
+goto out;
+
+if ((reg & flags) == flags) {
+retval = 0;
+goto out;
+}
+
+xapea00x_tpm_msleep(TPM_TIMEOUT);
+} while (time_before(jiffies, stop));
+
+retval = -ETIMEDOUT;
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_request_locality0 - sets the active locality to 0
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_request_locality0(struct xapea00x_device *dev)
+{
+int retval;
+
+retval = xapea00x_tpm_write8(dev, TPM_ACCESS_0, TPM_ACCESS_REQUEST_USE);
+if (retval)
+goto out;
+
+retval = xapea00x_tpm_wait_reg8(dev, TPM_ACCESS_0,
+TPM_ACCESS_ACTIVE_LOCALITY,
+TPM2_TIMEOUT_A);
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_release_locality0 - release the active locality
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0.  Otherwise a negative error code.
+ */
+static int xapea00x_tpm_release_locality0(struct xapea00x_device *dev)
+{
+return xapea00x_tpm_write8(dev, TPM_ACCESS_0,
+   TPM_ACCESS_ACTIVE_LOCALITY);
+}
+
+/**
+ * xapea00x_tpm_burst_count - fetch the number of bytes of data the
+ * TPM can currently handle in one burst.
+ * @dev: pointer to the device
+ * @counter: pointer to the destination for the count
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_burst_count(struct xapea00x_device *dev, u32 *count)
+{
+u32 reg;
+int retval;
+
+retval = xapea00x_tpm_read32(dev, TPM_STS_0, &reg);
+if (retval)
+goto out;
+
+*count = (reg >> 8) & 0xFFFF;
+retval = 0;
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_send - send the command to the TPM and execute it.
+ * @dev: pointer to the device
+ * @buf: the buffer containing the command
+ * @len: size of the buffer in bytes.
+ *
+ * N.B., the command may not fill the entire buffer. This function
+ * parses the command to determine its actual size.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_send(struct xapea00x_device *dev, void *buf, u32 len)
+{
+struct tpm_tis_command *cmd = buf;
+u32 size, burst;
+int retval;
+
+/* wait for TPM to be ready for command */
+retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_COMMAND_READY,
+TPM2_TIMEOUT_B);
+if (retval)
+goto err;
+
+/* extract size of from header */
+size = __be32_to_cpu(cmd->size);
+
+if (size > len) {
+retval = -EINVAL;
+goto err;
+}
+
+/* Write the command */
+while (size > 0) {
+xapea00x_tpm_burst_count(dev, &burst);
+burst = min(burst, size);
+
+retval = xapea00x_tpm_write_bytes(dev, TPM_DATA_FIFO_0, buf,
+  burst);
+if (retval)
+goto cancel;
+
+retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_VALID,
+TPM2_TIMEOUT_C);
+if (retval)
+goto cancel;
+
+buf += burst;
+size -= burst;
+}
+
+/* Do it */
+retval = xapea00x_tpm_write8(dev, TPM_STS_0, TPM_STS_GO);
+if (retval)
+goto cancel;
+
+return 0;
+
+cancel:
+/* Attempt to cancel */
+xapea00x_tpm_write8(dev, TPM_STS_0, TPM_STS_COMMAND_READY);
+
+err:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_recv - recv a command response from the TPM.
+ * @dev: pointer to the device
+ * @buf: the buffer in which to store the response
+ * @len: size of the buffer in bytes.
+ *
+ * N.B., the result may not fill the entire buffer. The caller must
+ * parse the response header to determine its actual size.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_recv(struct xapea00x_device *dev, void *buf, u32 len)
+{
+struct tpm_tis_command *cmd = buf;
+u32 burst;
+u32 size;
+int retval;
+
+/* wait for TPM to have data available */
+retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_DATA_AVAIL,
+TPM2_TIMEOUT_C);
+if (retval)
+goto cancel;
+
+/* read the header */
+if (len < TIS_HEADER_LEN) {
+retval = -EINVAL;
+goto cancel;
+}
+
+retval = xapea00x_tpm_read_bytes(dev, TPM_DATA_FIFO_0, buf,
+ TIS_HEADER_LEN);
+if (retval)
+goto cancel;
+
+/* extract size of body from header */
+size = __be32_to_cpu(cmd->size);
+if (len < size) {
+retval = -EINVAL;
+goto cancel;
+}
+
+size -= TIS_HEADER_LEN;
+buf   = &cmd->body;
+
+/* read the body */
+while (size > TIS_HEADER_LEN) {
+xapea00x_tpm_burst_count(dev, &burst);
+burst = min(burst, size);
+
+retval = xapea00x_tpm_read_bytes(dev, TPM_DATA_FIFO_0, buf,
+ burst);
+if (retval)
+goto cancel;
+
+size -= burst;
+buf += burst;
+}
+
+/* wait for valid */
+retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_VALID,
+TPM2_TIMEOUT_C);
+if (retval)
+goto err;
+
+return 0;
+
+cancel:
+xapea00x_tpm_write32(dev, TPM_STS_0, TPM_STS_COMMAND_READY);
+
+err:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_transmit - transmit one command to the TPM and receive
+ * the response.
+ * @dev: pointer to the device
+ * @buf: the buffer containing the command and to place the response in.
+ * @len: size of the buffer in bytes.
+ *
+ * N.B., the command and result may not fill the entire buffer. The
+ * caller must parse the response header to determine its actual size.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transmit(struct xapea00x_device *dev, void *buf,
+ u32 len)
+{
+int retval;
+
+retval = xapea00x_tpm_request_locality0(dev);
+if (retval)
+goto out;
+
+retval = xapea00x_tpm_send(dev, buf, len);
+if (retval)
+goto release;
+
+retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_DATA_AVAIL,
+TIS_LONG_TIMEOUT);
+if (retval)
+goto cancel;
+
+retval = xapea00x_tpm_recv(dev, buf, len);
+if (retval)
+goto release;
+
+retval = 0;
+goto release;
+
+cancel:
+xapea00x_tpm_write32(dev, TPM_STS_0, TPM_STS_COMMAND_READY);
+
+release:
+xapea00x_tpm_release_locality0(dev);
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_transmit_cmd - build and transmit one command to the
+ * TPM and receive the response.
+ * @dev: pointer to the device
+ * @tag: the TPM command header tag
+ * @cc: the TPM command header code
+ * @body: pointer to the command body
+ * @body_len: size in bytes of the command body
+ * @rc: pointer to the destination for the result code
+ * @result: pointer to the destination for the result body. If NULL,
+ *          the result body will be discarded.
+ * @result_len: size in bytes of the result buffer
+ * @actual_len: size in bytes of the result body. May be NULL is
+ *              result is NULL.
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transmit_cmd(struct xapea00x_device *dev,
+     u16 tag, u32 cc, void *body, u32 body_len,
+     u32 *rc, void *result, u32 result_len,
+     u32 *actual_len)
+{
+struct tpm_tis_command *cmd;
+void *buf;
+int buflen, cmdlen, retval;
+
+buflen = TIS_MAX_BUF + 4;
+cmdlen = buflen - 2; /* reserve 2 bytes for realignment */
+
+if (body_len + TIS_HEADER_LEN > cmdlen) {
+retval = -E2BIG;
+pr_notice("transmit_cmd: body_len + TIS_HEADER_LEN > cmdlen (%d)",
+  cmdlen);
+goto out;
+}
+
+buf = kzalloc(buflen, GFP_KERNEL);
+if (!buf) {
+retval = -ENOMEM;
+goto out;
+}
+cmd = buf + 2; /* ensure all fields are properly aligned */
+
+/* Build the command */
+cmd->tag  = __cpu_to_be16(tag);
+cmd->size = __cpu_to_be32(TIS_HEADER_LEN + body_len);
+cmd->code = __cpu_to_be32(cc);
+memcpy(&cmd->body, body, body_len);
+
+/* Execute the command */
+retval = xapea00x_tpm_transmit(dev, cmd, cmdlen);
+if (retval)
+goto free;
+
+/* Extract result code */
+*rc = __be32_to_cpu(cmd->code);
+
+/* Copy the response data */
+if (result) {
+*actual_len = __be32_to_cpu(cmd->size) - TIS_HEADER_LEN;
+if (*actual_len > result_len) {
+retval = -E2BIG;
+goto free;
+}
+memcpy(result, &cmd->body, *actual_len);
+}
+
+retval = 0;
+
+free:
+memset(buf, 0, buflen);
+kzfree(buf);
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_transmit_cmd_simple - build and transmit one command to the
+ * TPM and discard the respone body.
+ * @dev: pointer to the device
+ * @tag: the TPM command header tag
+ * @cc: the TPM command header code
+ * @body: pointer to the command body
+ * @len: size in bytes of the command body
+ * @rc: pointer to the destination for the result code
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_transmit_cmd_simple(struct xapea00x_device *dev,
+    u16 tag, u32 cc,
+    void *body, u32 len, u32 *rc)
+{
+return xapea00x_tpm_transmit_cmd(dev, tag, cc, body, len, rc, NULL, 0,
+ NULL);
+}
+
+/*******************************************************************************
+ * TPM commands
+ */
+
+/**
+ * xapea00x_tpm_startup - executes the TPM2_Startup command.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_startup(struct xapea00x_device *dev)
+{
+u8 body[2] = { 0x00, 0x00 };
+u32 rc;
+int retval;
+
+retval = xapea00x_tpm_transmit_cmd_simple(dev, TPM2_ST_NO_SESSIONS,
+  TPM2_CC_STARTUP, body,
+  sizeof(body), &rc);
+if (retval)
+goto out;
+
+if (rc != TPM_RC_SUCCESS && rc != TPM_RC_INITIALIZE) {
+retval = -EIO;
+goto out;
+}
+
+retval = 0;
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_self_test - executes the TPM2_SelfTest command.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_self_test(struct xapea00x_device *dev)
+{
+u8 body[1] = { 0x01 };
+u32 rc;
+int retval;
+
+retval = xapea00x_tpm_transmit_cmd_simple(dev, TPM2_ST_NO_SESSIONS,
+  TPM2_CC_SELF_TEST, body,
+  sizeof(body), &rc);
+if (retval)
+goto out;
+
+if (rc != TPM_RC_SUCCESS) {
+retval = -EIO;
+goto out;
+}
+
+retval = xapea00x_tpm_wait_reg8(dev, TPM_STS_0, TPM_STS_SELF_TEST_DONE,
+TIS_LONG_TIMEOUT);
+if (retval) {
+retval = -EIO;
+goto out;
+}
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_dict_attack_lock_reset - executes the
+ * TPM2_DictionaryAttackLockReset command.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int
+xapea00x_tpm_dict_attack_lock_reset(struct xapea00x_device *dev)
+{
+u8 body[17] = { 0x40, 0x00, 0x00, 0x0A, // TPM_RH_LOCKOUT
+0x00, 0x00, 0x00, 0x09, // authorizationSize
+0x40, 0x00, 0x00, 0x09, // TPM_RS_PW
+0x00, 0x00,// nonce size
+// nonce
+0x01,// session attributes
+0x00, 0x00// payload size
+// payload
+      };
+u32 rc;
+int retval;
+
+retval = xapea00x_tpm_transmit_cmd_simple(dev,
+  TPM2_ST_SESSIONS,
+  TPM2_CC_DICT_ATTACK_LOCK_RST,
+  body, sizeof(body), &rc);
+if (retval)
+goto out;
+
+if (rc != TPM_RC_SUCCESS) {
+retval = -EIO;
+goto out;
+}
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_get_random - executes the TPM2_GetRandom command.
+ * @dev: pointer to the device
+ * @len: number of bytes to request
+ * @bytes: pointer to the destination
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_get_random(struct xapea00x_device *dev, u16 len,
+   void *bytes)
+{
+__be16 body;
+u8 *buf;
+u32 buf_len, result_len;
+u32 rc;
+int retval;
+
+buf_len = len + 2;
+buf = kzalloc(buf_len, GFP_KERNEL);
+if (!buf) {
+retval = -ENOMEM;
+goto out;
+}
+
+while (len > 0) {
+body = __cpu_to_be16(len);
+
+retval = xapea00x_tpm_transmit_cmd(dev, TPM2_ST_NO_SESSIONS,
+   TPM2_CC_GET_RANDOM,
+   &body, sizeof(body),
+   &rc, buf, buf_len,
+   &result_len);
+
+if (retval)
+goto free;
+
+if (rc != TPM_RC_SUCCESS) {
+retval = -EIO;
+goto free;
+}
+
+result_len = __be16_to_cpu(*(__be16 *)buf);
+if (result_len > len) {
+retval = -E2BIG;
+goto free;
+}
+
+memcpy(bytes, buf + 2, result_len);
+len -= result_len;
+}
+
+retval = 0;
+
+free:
+memset(buf, 0, buf_len);
+kzfree(buf);
+
+out:
+return retval;
+}
+
+/**
+ * xapea00x_tpm_randomize_platform_auth - sets the platform
+ * authorization to a random password and then discards it.
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_tpm_randomize_platform_auth(struct xapea00x_device *dev)
+{
+u8 password[16];
+u8 body[35] = { 0x40, 0x00, 0x00, 0x0C, // TPM_RH_PLATFORM
+0x00, 0x00, 0x00, 0x09, // authorizationSize
+0x40, 0x00, 0x00, 0x09, // TPM_RS_PW
+0x00, 0x00,// nonce size
+// nonce
+0x01,// session attributes
+0x00, 0x00,// old auth payload size
+// old auth payload
+0x00, 0x10,// new auth payload size
+0x00, 0x00, 0x00, 0x00, // new auth payload
+0x00, 0x00, 0x00, 0x00, // new auth payload
+0x00, 0x00, 0x00, 0x00, // new auth payload
+0x00, 0x00, 0x00, 0x00// new auth payload
+      };
+u32 rc;
+int retval;
+
+retval = xapea00x_tpm_get_random(dev, sizeof(password), password);
+if (retval) {
+dev_err(&dev->interface->dev,
+"TPM get random failed with %d\n", retval);
+goto out;
+}
+
+memcpy(body + 19, password, sizeof(password));
+
+retval = xapea00x_tpm_transmit_cmd_simple(dev, TPM2_ST_SESSIONS,
+  TPM2_CC_HIERARCHY_CHANGE_AUTH,
+  &body, sizeof(body), &rc);
+if (retval)
+goto out;
+
+if (rc != TPM_RC_SUCCESS) {
+retval = -EIO;
+pr_notice("HierarchyChangeAuth result code: %d\n", rc);
+goto out;
+}
+
+retval = 0;
+
+out:
+memset(password, 0, sizeof(password));
+memset(body, 0, sizeof(body));
+return retval;
+}
+
+/**
+ * xapea00x_tpm_platform_initialize - performs the minimal
+ * initialization of the TPM normally performed by the platform code
+ * (e.g., BIOS). This consists of executing the TPM startup and
+ * self-test commands and setting the platform authorization password.
+ *
+ * @dev: pointer to the device
+ *
+ * Context: !in_interrupt()
+ *
+ * Result: If successful, 0. Otherwise a negative error code.
+ */
+int xapea00x_tpm_platform_initialize(struct xapea00x_device *dev)
+{
+int retval;
+
+/* wait for TPM to be ready */
+retval =  xapea00x_tpm_wait_reg8(dev, TPM_ACCESS_0, TPM_ACCESS_VALID,
+ TPM2_TIMEOUT_A);
+if (retval)
+goto out;
+
+/* issue TPM2_CC_STARTUP command */
+retval = xapea00x_tpm_startup(dev);
+if (retval) {
+dev_err(&dev->interface->dev, "TPM startup failed with %d\n",
+retval);
+goto out;
+}
+
+/* issue TPM2_SELF_TEST command */
+retval = xapea00x_tpm_self_test(dev);
+if (retval) {
+dev_err(&dev->interface->dev, "TPM self-test failed with %d\n",
+retval);
+goto out;
+}
+
+/*
+ * The TPM will enter dictionary lockout mode if turned off
+ * too many times without a proper shutdown. For the
+ * "thumb-drive"-esque demo devices, this happens whenever it
+ * is unplugged. Dictionary attacks against the demo devices
+ * (XAP-EA-00{1,2}) don't matter, so reset the lockout on every
+ * boot. Production devices (XAP-EA-003) are internal mPCI-e
+ * devices that should not be hot-plugged, so do not need to be
+ * reset.
+ */
+if (dev->pid == USB_PRODUCT_ID_XAPEA001 ||
+    dev->pid == USB_PRODUCT_ID_XAPEA002) {
+retval = xapea00x_tpm_dict_attack_lock_reset(dev);
+if (retval) {
+dev_err(&dev->interface->dev,
+"Resetting TPM lockout failed with %d\n",
+retval);
+goto out;
+}
+}
+
+/* set the platform authorization to random bytes */
+retval = xapea00x_tpm_randomize_platform_auth(dev);
+if (retval) {
+dev_err(&dev->interface->dev,
+"Setting TPM platform auth failed with %d\n",
+retval);
+goto out;
+}
+
+retval = 0;
+
+out:
+return retval;
+}
--
2.16.3

________________________________

Jeremy Boone
Principal Security Consultant
NCC Group
51 Breithaupt Street,
Suite 100, Kitchener, N2H 5G5

Telephone: +1 226 606 8318<tel:+1 226 606 8318>
Mobile: +1 226 606 8318<tel:+1 226 606 8318>
Website: www.nccgroup.trust<http://www.nccgroup.trust>
Twitter: @NCCGroupplc<https://twitter.com/NCCGroupplc>
        [https://www.nccgroup.trust/static/img/emaillogo/ncc-group-logo.png]  <http://www.nccgroup.trust/>
________________________________


This email is sent for and on behalf of NCC Group. NCC Group is the trading name of NCC Services Limited (Registered in England CRN: 2802141). The ultimate holding company is NCC Group plc (Registered in England CRN: 4627044). This email may be confidential and/or legally privileged.

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[Jarkko Sakkinen]

On Fri, May 04, 2018 at 08:00:22AM -0500, David R. Bild wrote:
> Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
> for performing initialization of the TPM.  For these modules, the host
> kernel is the platform, so we perform the initialization in the driver
> before registering the TPM with the kernel TPM subsystem.
> 
> The initialization consists of issuing the TPM startup command,
> running the TPM self-test, and setting the TPM platform hierarchy
> authorization to a random, unsaved value so that it can never be used
> after the driver has loaded.
> 
> Signed-off-by: David R. Bild <david.bild@xaptum.com>

Have you checked what the TPM driver already does?

/Jarkko

Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

[David R. Bild]

On Tue, May 8, 2018 at 5:47 AM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, May 04, 2018 at 08:00:22AM -0500, David R. Bild wrote:
> > Normally the system platform (i.e., BIOS/UEFI for x86) is responsible
> > for performing initialization of the TPM.  For these modules, the host
> > kernel is the platform, so we perform the initialization in the driver
> > before registering the TPM with the kernel TPM subsystem.
> >
> > The initialization consists of issuing the TPM startup command,
> > running the TPM self-test, and setting the TPM platform hierarchy
> > authorization to a random, unsaved value so that it can never be used
> > after the driver has loaded.
> >
> > Signed-off-by: David R. Bild <david.bild@xaptum.com>
>
> Have you checked what the TPM driver already does?


Yes.  It does the startup and self-test.

 However, this driver sets the platform hierarchy password *before*
giving the TPM to the TPM driver (thus, before the TPM driver can do
the startup and self-test).  Startup and self-test are prerequisites
for setting the platform hierarchy password, so this driver does those
itself too.

Best,
David