All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2]  Extend the vTPM proxy driver to pass locality
@ 2017-05-10 23:54 ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-10 23:54 UTC (permalink / raw)
  To: tpmdd-devel, linux-security-module, jarkko.sakkinen
  Cc: jgunthorpe, linux-kernel, Stefan Berger

The purpose of this series of patches is to enable the passing of the locality
a command is executing in to a recipient, i.e., TPM emulator. To enable this we
introduce vendor-specific TPM commands for TPM 1.2 and TPM 2 that the driver
sends to the TPM emulator.

v3->v4:
  - addressed Jarkko's comments: largely a rewrite of the patches

v2->v3:
  - addressed Jarkko's comments

v1->v2:
  - fixed return value from function in patch 3/3

Stefan Berger (2):
  tpm: Refactor tpm_transmit pulling out tpm_transfer function
  tpm: vtpm_proxy: Implement request_locality function.

 drivers/char/tpm/tpm-interface.c  | 121 +++++++++++++++++++++++---------------
 drivers/char/tpm/tpm.h            |   1 +
 drivers/char/tpm/tpm_vtpm_proxy.c |  34 +++++++++++
 include/uapi/linux/vtpm_proxy.h   |   5 ++
 4 files changed, 113 insertions(+), 48 deletions(-)

-- 
2.4.3

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

* [PATCH v4 0/2]  Extend the vTPM proxy driver to pass locality
@ 2017-05-10 23:54 ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-10 23:54 UTC (permalink / raw)
  To: linux-security-module

The purpose of this series of patches is to enable the passing of the locality
a command is executing in to a recipient, i.e., TPM emulator. To enable this we
introduce vendor-specific TPM commands for TPM 1.2 and TPM 2 that the driver
sends to the TPM emulator.

v3->v4:
  - addressed Jarkko's comments: largely a rewrite of the patches

v2->v3:
  - addressed Jarkko's comments

v1->v2:
  - fixed return value from function in patch 3/3

Stefan Berger (2):
  tpm: Refactor tpm_transmit pulling out tpm_transfer function
  tpm: vtpm_proxy: Implement request_locality function.

 drivers/char/tpm/tpm-interface.c  | 121 +++++++++++++++++++++++---------------
 drivers/char/tpm/tpm.h            |   1 +
 drivers/char/tpm/tpm_vtpm_proxy.c |  34 +++++++++++
 include/uapi/linux/vtpm_proxy.h   |   5 ++
 4 files changed, 113 insertions(+), 48 deletions(-)

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
  2017-05-10 23:54 ` Stefan Berger
@ 2017-05-10 23:54   ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-10 23:54 UTC (permalink / raw)
  To: tpmdd-devel, linux-security-module, jarkko.sakkinen
  Cc: jgunthorpe, linux-kernel, Stefan Berger

Refactor tpm_transmit and pull out code sending the command
and receiving the response and put this into tpm_transfer.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
 1 file changed, 73 insertions(+), 48 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 158c1db..263b6d1 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
 }
 
 /**
- * tmp_transmit - Internal kernel interface to transmit TPM commands.
+ * tmp_transfer - Send a TPM command to the TPM and receive response
  *
  * @chip: TPM chip to use
  * @buf: TPM command buffer
+ * @count: size of the TPM command
  * @bufsiz: length of the TPM command buffer
- * @flags: tpm transmit flags - bitmap
  *
  * Return:
- *     0 when the operation is successful.
+ *     >0 when the operation is successful; returns response length
  *     A negative number for system errors (errno).
  */
-ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
-		     u8 *buf, size_t bufsiz, unsigned int flags)
+ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)
 {
-	struct tpm_output_header *header = (void *)buf;
 	int rc;
+	struct tpm_output_header *header = (void *)buf;
+	u32 ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
 	ssize_t len = 0;
-	u32 count, ordinal;
 	unsigned long stop;
-	bool need_locality;
-
-	if (!tpm_validate_command(chip, space, buf, bufsiz))
-		return -EINVAL;
-
-	if (bufsiz > TPM_BUFSIZE)
-		bufsiz = TPM_BUFSIZE;
-
-	count = be32_to_cpu(*((__be32 *) (buf + 2)));
-	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
-	if (count == 0)
-		return -ENODATA;
-	if (count > bufsiz) {
-		dev_err(&chip->dev,
-			"invalid count value %x %zx\n", count, bufsiz);
-		return -E2BIG;
-	}
-
-	if (!(flags & TPM_TRANSMIT_UNLOCKED))
-		mutex_lock(&chip->tpm_mutex);
-
-	if (chip->dev.parent)
-		pm_runtime_get_sync(chip->dev.parent);
-
-	/* Store the decision as chip->locality will be changed. */
-	need_locality = chip->locality == -1;
-
-	if (need_locality && chip->ops->request_locality)  {
-		rc = chip->ops->request_locality(chip, 0);
-		if (rc < 0)
-			goto out_no_locality;
-		chip->locality = rc;
-	}
-
-	rc = tpm2_prepare_space(chip, space, ordinal, buf);
-	if (rc)
-		goto out;
 
 	rc = chip->ops->send(chip, (u8 *) buf, count);
 	if (rc < 0) {
 		dev_err(&chip->dev,
-			"tpm_transmit: tpm_send: error %d\n", rc);
+			"tpm_transfer: tpm_send: error %d\n", rc);
 		goto out;
 	}
 
@@ -467,18 +429,81 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	if (len < 0) {
 		rc = len;
 		dev_err(&chip->dev,
-			"tpm_transmit: tpm_recv: error %d\n", rc);
+			"tpm_transfer: tpm_recv: error %d\n", rc);
 		goto out;
 	} else if (len < TPM_HEADER_SIZE) {
 		rc = -EFAULT;
 		goto out;
 	}
 
-	if (len != be32_to_cpu(header->length)) {
+	if (len != be32_to_cpu(header->length))
 		rc = -EFAULT;
-		goto out;
+
+out:
+	return rc ? rc : len;
+}
+EXPORT_SYMBOL_GPL(tpm_transfer);
+
+/**
+ * tmp_transmit - Internal kernel interface to transmit TPM commands.
+ *
+ * @chip: TPM chip to use
+ * @buf: TPM command buffer
+ * @bufsiz: length of the TPM command buffer
+ * @flags: tpm transmit flags - bitmap
+ *
+ * Return:
+ *     0 when the operation is successful.
+ *     A negative number for system errors (errno).
+ */
+ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
+		     u8 *buf, size_t bufsiz, unsigned int flags)
+{
+	int rc;
+	ssize_t len = 0;
+	u32 count, ordinal;
+	bool need_locality;
+
+	if (!tpm_validate_command(chip, space, buf, bufsiz))
+		return -EINVAL;
+
+	if (bufsiz > TPM_BUFSIZE)
+		bufsiz = TPM_BUFSIZE;
+
+	count = be32_to_cpu(*((__be32 *) (buf + 2)));
+	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
+	if (count == 0)
+		return -ENODATA;
+	if (count > bufsiz) {
+		dev_err(&chip->dev,
+			"invalid count value %x %zx\n", count, bufsiz);
+		return -E2BIG;
+	}
+
+	if (!(flags & TPM_TRANSMIT_UNLOCKED))
+		mutex_lock(&chip->tpm_mutex);
+
+	if (chip->dev.parent)
+		pm_runtime_get_sync(chip->dev.parent);
+
+	/* Store the decision as chip->locality will be changed. */
+	need_locality = chip->locality == -1;
+
+	if (need_locality && chip->ops->request_locality)  {
+		rc = chip->ops->request_locality(chip, 0);
+		if (rc < 0)
+			goto out_no_locality;
+		chip->locality = rc;
 	}
 
+	rc = tpm2_prepare_space(chip, space, ordinal, buf);
+	if (rc)
+		goto out;
+
+	len = tpm_transfer(chip, buf, count, bufsiz);
+	if (len < 0)
+		goto out;
+
 	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
 
 out:
-- 
2.4.3

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

* [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
@ 2017-05-10 23:54   ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-10 23:54 UTC (permalink / raw)
  To: linux-security-module

Refactor tpm_transmit and pull out code sending the command
and receiving the response and put this into tpm_transfer.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
 1 file changed, 73 insertions(+), 48 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 158c1db..263b6d1 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
 }
 
 /**
- * tmp_transmit - Internal kernel interface to transmit TPM commands.
+ * tmp_transfer - Send a TPM command to the TPM and receive response
  *
  * @chip: TPM chip to use
  * @buf: TPM command buffer
+ * @count: size of the TPM command
  * @bufsiz: length of the TPM command buffer
- * @flags: tpm transmit flags - bitmap
  *
  * Return:
- *     0 when the operation is successful.
+ *     >0 when the operation is successful; returns response length
  *     A negative number for system errors (errno).
  */
-ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
-		     u8 *buf, size_t bufsiz, unsigned int flags)
+ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)
 {
-	struct tpm_output_header *header = (void *)buf;
 	int rc;
+	struct tpm_output_header *header = (void *)buf;
+	u32 ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
 	ssize_t len = 0;
-	u32 count, ordinal;
 	unsigned long stop;
-	bool need_locality;
-
-	if (!tpm_validate_command(chip, space, buf, bufsiz))
-		return -EINVAL;
-
-	if (bufsiz > TPM_BUFSIZE)
-		bufsiz = TPM_BUFSIZE;
-
-	count = be32_to_cpu(*((__be32 *) (buf + 2)));
-	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
-	if (count == 0)
-		return -ENODATA;
-	if (count > bufsiz) {
-		dev_err(&chip->dev,
-			"invalid count value %x %zx\n", count, bufsiz);
-		return -E2BIG;
-	}
-
-	if (!(flags & TPM_TRANSMIT_UNLOCKED))
-		mutex_lock(&chip->tpm_mutex);
-
-	if (chip->dev.parent)
-		pm_runtime_get_sync(chip->dev.parent);
-
-	/* Store the decision as chip->locality will be changed. */
-	need_locality = chip->locality == -1;
-
-	if (need_locality && chip->ops->request_locality)  {
-		rc = chip->ops->request_locality(chip, 0);
-		if (rc < 0)
-			goto out_no_locality;
-		chip->locality = rc;
-	}
-
-	rc = tpm2_prepare_space(chip, space, ordinal, buf);
-	if (rc)
-		goto out;
 
 	rc = chip->ops->send(chip, (u8 *) buf, count);
 	if (rc < 0) {
 		dev_err(&chip->dev,
-			"tpm_transmit: tpm_send: error %d\n", rc);
+			"tpm_transfer: tpm_send: error %d\n", rc);
 		goto out;
 	}
 
@@ -467,18 +429,81 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	if (len < 0) {
 		rc = len;
 		dev_err(&chip->dev,
-			"tpm_transmit: tpm_recv: error %d\n", rc);
+			"tpm_transfer: tpm_recv: error %d\n", rc);
 		goto out;
 	} else if (len < TPM_HEADER_SIZE) {
 		rc = -EFAULT;
 		goto out;
 	}
 
-	if (len != be32_to_cpu(header->length)) {
+	if (len != be32_to_cpu(header->length))
 		rc = -EFAULT;
-		goto out;
+
+out:
+	return rc ? rc : len;
+}
+EXPORT_SYMBOL_GPL(tpm_transfer);
+
+/**
+ * tmp_transmit - Internal kernel interface to transmit TPM commands.
+ *
+ * @chip: TPM chip to use
+ * @buf: TPM command buffer
+ * @bufsiz: length of the TPM command buffer
+ * @flags: tpm transmit flags - bitmap
+ *
+ * Return:
+ *     0 when the operation is successful.
+ *     A negative number for system errors (errno).
+ */
+ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
+		     u8 *buf, size_t bufsiz, unsigned int flags)
+{
+	int rc;
+	ssize_t len = 0;
+	u32 count, ordinal;
+	bool need_locality;
+
+	if (!tpm_validate_command(chip, space, buf, bufsiz))
+		return -EINVAL;
+
+	if (bufsiz > TPM_BUFSIZE)
+		bufsiz = TPM_BUFSIZE;
+
+	count = be32_to_cpu(*((__be32 *) (buf + 2)));
+	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
+	if (count == 0)
+		return -ENODATA;
+	if (count > bufsiz) {
+		dev_err(&chip->dev,
+			"invalid count value %x %zx\n", count, bufsiz);
+		return -E2BIG;
+	}
+
+	if (!(flags & TPM_TRANSMIT_UNLOCKED))
+		mutex_lock(&chip->tpm_mutex);
+
+	if (chip->dev.parent)
+		pm_runtime_get_sync(chip->dev.parent);
+
+	/* Store the decision as chip->locality will be changed. */
+	need_locality = chip->locality == -1;
+
+	if (need_locality && chip->ops->request_locality)  {
+		rc = chip->ops->request_locality(chip, 0);
+		if (rc < 0)
+			goto out_no_locality;
+		chip->locality = rc;
 	}
 
+	rc = tpm2_prepare_space(chip, space, ordinal, buf);
+	if (rc)
+		goto out;
+
+	len = tpm_transfer(chip, buf, count, bufsiz);
+	if (len < 0)
+		goto out;
+
 	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
 
 out:
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info@ http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-10 23:54 ` Stefan Berger
  (?)
@ 2017-05-10 23:54   ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-10 23:54 UTC (permalink / raw)
  To: tpmdd-devel, linux-security-module, jarkko.sakkinen
  Cc: jgunthorpe, linux-kernel, Stefan Berger

Implement the request_locality function. To set the locality on the
backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
a command to the backend to set the locality for the next commands.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm.h            |  1 +
 drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/vtpm_proxy.h   |  5 +++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8de..10f0467 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -527,6 +527,7 @@ enum tpm_transmit_flags {
 	TPM_TRANSMIT_UNLOCKED	= BIT(0),
 };
 
+ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
 ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 		     u8 *buf, size_t bufsiz, unsigned int flags);
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 751059d..374c4ff 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
 	return ret;
 }
 
+static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
+{
+	struct tpm_buf buf;
+	int rc;
+	const struct tpm_output_header *header;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
+				  TPM2_CC_SET_LOCALITY);
+	else
+		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
+				  TPM_ORD_SET_LOCALITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u8(&buf, locality);
+
+	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
+	if (rc < 0) {
+		locality = rc;
+		goto out;
+	}
+
+	header = (const struct tpm_output_header *)buf.data;
+	rc = be32_to_cpu(header->return_code);
+	if (rc)
+		locality = -1;
+
+out:
+	tpm_buf_destroy(&buf);
+
+	return locality;
+}
+
 static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.flags = TPM_OPS_AUTO_STARTUP,
 	.recv = vtpm_proxy_tpm_op_recv,
@@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_canceled = vtpm_proxy_tpm_req_canceled,
+	.request_locality = vtpm_proxy_request_locality,
 };
 
 /*
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
index a69e991..6b91c2c 100644
--- a/include/uapi/linux/vtpm_proxy.h
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
 
 #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
 
+/* vendor specific commands to set locality */
+#define TPM2_CC_SET_LOCALITY	0x20001000
+#define TPM_ORD_SET_LOCALITY	0x20001000
+
+
 #endif /* _UAPI_LINUX_VTPM_PROXY_H */
-- 
2.4.3

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-10 23:54   ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-10 23:54 UTC (permalink / raw)
  To: linux-security-module

Implement the request_locality function. To set the locality on the
backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
a command to the backend to set the locality for the next commands.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm.h            |  1 +
 drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/vtpm_proxy.h   |  5 +++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8de..10f0467 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -527,6 +527,7 @@ enum tpm_transmit_flags {
 	TPM_TRANSMIT_UNLOCKED	= BIT(0),
 };
 
+ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
 ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 		     u8 *buf, size_t bufsiz, unsigned int flags);
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 751059d..374c4ff 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
 	return ret;
 }
 
+static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
+{
+	struct tpm_buf buf;
+	int rc;
+	const struct tpm_output_header *header;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
+				  TPM2_CC_SET_LOCALITY);
+	else
+		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
+				  TPM_ORD_SET_LOCALITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u8(&buf, locality);
+
+	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
+	if (rc < 0) {
+		locality = rc;
+		goto out;
+	}
+
+	header = (const struct tpm_output_header *)buf.data;
+	rc = be32_to_cpu(header->return_code);
+	if (rc)
+		locality = -1;
+
+out:
+	tpm_buf_destroy(&buf);
+
+	return locality;
+}
+
 static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.flags = TPM_OPS_AUTO_STARTUP,
 	.recv = vtpm_proxy_tpm_op_recv,
@@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_canceled = vtpm_proxy_tpm_req_canceled,
+	.request_locality = vtpm_proxy_request_locality,
 };
 
 /*
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
index a69e991..6b91c2c 100644
--- a/include/uapi/linux/vtpm_proxy.h
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
 
 #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
 
+/* vendor specific commands to set locality */
+#define TPM2_CC_SET_LOCALITY	0x20001000
+#define TPM_ORD_SET_LOCALITY	0x20001000
+
+
 #endif /* _UAPI_LINUX_VTPM_PROXY_H */
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-10 23:54   ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-10 23:54 UTC (permalink / raw)
  To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

Implement the request_locality function. To set the locality on the
backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
a command to the backend to set the locality for the next commands.

Signed-off-by: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm.h            |  1 +
 drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/vtpm_proxy.h   |  5 +++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4b4c8de..10f0467 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -527,6 +527,7 @@ enum tpm_transmit_flags {
 	TPM_TRANSMIT_UNLOCKED	= BIT(0),
 };
 
+ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
 ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 		     u8 *buf, size_t bufsiz, unsigned int flags);
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 751059d..374c4ff 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
 	return ret;
 }
 
+static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
+{
+	struct tpm_buf buf;
+	int rc;
+	const struct tpm_output_header *header;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
+				  TPM2_CC_SET_LOCALITY);
+	else
+		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
+				  TPM_ORD_SET_LOCALITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u8(&buf, locality);
+
+	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
+	if (rc < 0) {
+		locality = rc;
+		goto out;
+	}
+
+	header = (const struct tpm_output_header *)buf.data;
+	rc = be32_to_cpu(header->return_code);
+	if (rc)
+		locality = -1;
+
+out:
+	tpm_buf_destroy(&buf);
+
+	return locality;
+}
+
 static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.flags = TPM_OPS_AUTO_STARTUP,
 	.recv = vtpm_proxy_tpm_op_recv,
@@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_canceled = vtpm_proxy_tpm_req_canceled,
+	.request_locality = vtpm_proxy_request_locality,
 };
 
 /*
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
index a69e991..6b91c2c 100644
--- a/include/uapi/linux/vtpm_proxy.h
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
 
 #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
 
+/* vendor specific commands to set locality */
+#define TPM2_CC_SET_LOCALITY	0x20001000
+#define TPM_ORD_SET_LOCALITY	0x20001000
+
+
 #endif /* _UAPI_LINUX_VTPM_PROXY_H */
-- 
2.4.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

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

* Re: [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
  2017-05-10 23:54   ` Stefan Berger
@ 2017-05-12 13:30     ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-12 13:30 UTC (permalink / raw)
  To: tpmdd-devel, linux-security-module, jarkko.sakkinen
  Cc: jgunthorpe, linux-kernel

On 05/10/2017 07:54 PM, Stefan Berger wrote:
> Refactor tpm_transmit and pull out code sending the command
> and receiving the response and put this into tpm_transfer.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>   drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
>   1 file changed, 73 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 158c1db..263b6d1 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
>   }
>
>   /**
> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + * tmp_transfer - Send a TPM command to the TPM and receive response
>    *
>    * @chip: TPM chip to use
>    * @buf: TPM command buffer
> + * @count: size of the TPM command
>    * @bufsiz: length of the TPM command buffer
> - * @flags: tpm transmit flags - bitmap
>    *
>    * Return:
> - *     0 when the operation is successful.
> + *     >0 when the operation is successful; returns response length
>    *     A negative number for system errors (errno).
>    */
> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> -		     u8 *buf, size_t bufsiz, unsigned int flags)
> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)
>   {
> -	struct tpm_output_header *header = (void *)buf;
>   	int rc;
> +	struct tpm_output_header *header = (void *)buf;
> +	u32 ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
>   	ssize_t len = 0;
> -	u32 count, ordinal;
>   	unsigned long stop;
> -	bool need_locality;
> -
> -	if (!tpm_validate_command(chip, space, buf, bufsiz))
> -		return -EINVAL;
> -
> -	if (bufsiz > TPM_BUFSIZE)
> -		bufsiz = TPM_BUFSIZE;
> -
> -	count = be32_to_cpu(*((__be32 *) (buf + 2)));
> -	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> -	if (count == 0)
> -		return -ENODATA;
> -	if (count > bufsiz) {
> -		dev_err(&chip->dev,
> -			"invalid count value %x %zx\n", count, bufsiz);
> -		return -E2BIG;
> -	}
> -
> -	if (!(flags & TPM_TRANSMIT_UNLOCKED))
> -		mutex_lock(&chip->tpm_mutex);
> -
> -	if (chip->dev.parent)
> -		pm_runtime_get_sync(chip->dev.parent);
> -
> -	/* Store the decision as chip->locality will be changed. */
> -	need_locality = chip->locality == -1;
> -
> -	if (need_locality && chip->ops->request_locality)  {
> -		rc = chip->ops->request_locality(chip, 0);
> -		if (rc < 0)
> -			goto out_no_locality;
> -		chip->locality = rc;
> -	}
> -
> -	rc = tpm2_prepare_space(chip, space, ordinal, buf);
> -	if (rc)
> -		goto out;
>
>   	rc = chip->ops->send(chip, (u8 *) buf, count);
>   	if (rc < 0) {
>   		dev_err(&chip->dev,
> -			"tpm_transmit: tpm_send: error %d\n", rc);
> +			"tpm_transfer: tpm_send: error %d\n", rc);
>   		goto out;
>   	}
>
> @@ -467,18 +429,81 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
missing here: rc = 0;

>   	if (len < 0) {
>   		rc = len;
>   		dev_err(&chip->dev,
> -			"tpm_transmit: tpm_recv: error %d\n", rc);
> +			"tpm_transfer: tpm_recv: error %d\n", rc);
>   		goto out;
>   	} else if (len < TPM_HEADER_SIZE) {
>   		rc = -EFAULT;
>   		goto out;
>   	}
>
> -	if (len != be32_to_cpu(header->length)) {
> +	if (len != be32_to_cpu(header->length))
>   		rc = -EFAULT;
> -		goto out;
> +
> +out:
> +	return rc ? rc : len;
> +}
> +EXPORT_SYMBOL_GPL(tpm_transfer);
> +
> +/**
> + * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + *
> + * @chip: TPM chip to use
> + * @buf: TPM command buffer
> + * @bufsiz: length of the TPM command buffer
> + * @flags: tpm transmit flags - bitmap
> + *
> + * Return:
> + *     0 when the operation is successful.
> + *     A negative number for system errors (errno).
> + */
> +ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> +		     u8 *buf, size_t bufsiz, unsigned int flags)
> +{
> +	int rc;
> +	ssize_t len = 0;
> +	u32 count, ordinal;
> +	bool need_locality;
> +
> +	if (!tpm_validate_command(chip, space, buf, bufsiz))
> +		return -EINVAL;
> +
> +	if (bufsiz > TPM_BUFSIZE)
> +		bufsiz = TPM_BUFSIZE;
> +
> +	count = be32_to_cpu(*((__be32 *) (buf + 2)));
> +	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +	if (count == 0)
> +		return -ENODATA;
> +	if (count > bufsiz) {
> +		dev_err(&chip->dev,
> +			"invalid count value %x %zx\n", count, bufsiz);
> +		return -E2BIG;
> +	}
> +
> +	if (!(flags & TPM_TRANSMIT_UNLOCKED))
> +		mutex_lock(&chip->tpm_mutex);
> +
> +	if (chip->dev.parent)
> +		pm_runtime_get_sync(chip->dev.parent);
> +
> +	/* Store the decision as chip->locality will be changed. */
> +	need_locality = chip->locality == -1;
> +
> +	if (need_locality && chip->ops->request_locality)  {
> +		rc = chip->ops->request_locality(chip, 0);
> +		if (rc < 0)
> +			goto out_no_locality;
> +		chip->locality = rc;
>   	}
>
> +	rc = tpm2_prepare_space(chip, space, ordinal, buf);
> +	if (rc)
> +		goto out;
> +
> +	len = tpm_transfer(chip, buf, count, bufsiz);
> +	if (len < 0)
> +		goto out;
> +
>   	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
>
>   out:

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

* [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
@ 2017-05-12 13:30     ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-12 13:30 UTC (permalink / raw)
  To: linux-security-module

On 05/10/2017 07:54 PM, Stefan Berger wrote:
> Refactor tpm_transmit and pull out code sending the command
> and receiving the response and put this into tpm_transfer.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>   drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
>   1 file changed, 73 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 158c1db..263b6d1 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
>   }
>
>   /**
> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + * tmp_transfer - Send a TPM command to the TPM and receive response
>    *
>    * @chip: TPM chip to use
>    * @buf: TPM command buffer
> + * @count: size of the TPM command
>    * @bufsiz: length of the TPM command buffer
> - * @flags: tpm transmit flags - bitmap
>    *
>    * Return:
> - *     0 when the operation is successful.
> + *     >0 when the operation is successful; returns response length
>    *     A negative number for system errors (errno).
>    */
> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> -		     u8 *buf, size_t bufsiz, unsigned int flags)
> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)
>   {
> -	struct tpm_output_header *header = (void *)buf;
>   	int rc;
> +	struct tpm_output_header *header = (void *)buf;
> +	u32 ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
>   	ssize_t len = 0;
> -	u32 count, ordinal;
>   	unsigned long stop;
> -	bool need_locality;
> -
> -	if (!tpm_validate_command(chip, space, buf, bufsiz))
> -		return -EINVAL;
> -
> -	if (bufsiz > TPM_BUFSIZE)
> -		bufsiz = TPM_BUFSIZE;
> -
> -	count = be32_to_cpu(*((__be32 *) (buf + 2)));
> -	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> -	if (count == 0)
> -		return -ENODATA;
> -	if (count > bufsiz) {
> -		dev_err(&chip->dev,
> -			"invalid count value %x %zx\n", count, bufsiz);
> -		return -E2BIG;
> -	}
> -
> -	if (!(flags & TPM_TRANSMIT_UNLOCKED))
> -		mutex_lock(&chip->tpm_mutex);
> -
> -	if (chip->dev.parent)
> -		pm_runtime_get_sync(chip->dev.parent);
> -
> -	/* Store the decision as chip->locality will be changed. */
> -	need_locality = chip->locality == -1;
> -
> -	if (need_locality && chip->ops->request_locality)  {
> -		rc = chip->ops->request_locality(chip, 0);
> -		if (rc < 0)
> -			goto out_no_locality;
> -		chip->locality = rc;
> -	}
> -
> -	rc = tpm2_prepare_space(chip, space, ordinal, buf);
> -	if (rc)
> -		goto out;
>
>   	rc = chip->ops->send(chip, (u8 *) buf, count);
>   	if (rc < 0) {
>   		dev_err(&chip->dev,
> -			"tpm_transmit: tpm_send: error %d\n", rc);
> +			"tpm_transfer: tpm_send: error %d\n", rc);
>   		goto out;
>   	}
>
> @@ -467,18 +429,81 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
missing here: rc = 0;

>   	if (len < 0) {
>   		rc = len;
>   		dev_err(&chip->dev,
> -			"tpm_transmit: tpm_recv: error %d\n", rc);
> +			"tpm_transfer: tpm_recv: error %d\n", rc);
>   		goto out;
>   	} else if (len < TPM_HEADER_SIZE) {
>   		rc = -EFAULT;
>   		goto out;
>   	}
>
> -	if (len != be32_to_cpu(header->length)) {
> +	if (len != be32_to_cpu(header->length))
>   		rc = -EFAULT;
> -		goto out;
> +
> +out:
> +	return rc ? rc : len;
> +}
> +EXPORT_SYMBOL_GPL(tpm_transfer);
> +
> +/**
> + * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + *
> + * @chip: TPM chip to use
> + * @buf: TPM command buffer
> + * @bufsiz: length of the TPM command buffer
> + * @flags: tpm transmit flags - bitmap
> + *
> + * Return:
> + *     0 when the operation is successful.
> + *     A negative number for system errors (errno).
> + */
> +ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> +		     u8 *buf, size_t bufsiz, unsigned int flags)
> +{
> +	int rc;
> +	ssize_t len = 0;
> +	u32 count, ordinal;
> +	bool need_locality;
> +
> +	if (!tpm_validate_command(chip, space, buf, bufsiz))
> +		return -EINVAL;
> +
> +	if (bufsiz > TPM_BUFSIZE)
> +		bufsiz = TPM_BUFSIZE;
> +
> +	count = be32_to_cpu(*((__be32 *) (buf + 2)));
> +	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +	if (count == 0)
> +		return -ENODATA;
> +	if (count > bufsiz) {
> +		dev_err(&chip->dev,
> +			"invalid count value %x %zx\n", count, bufsiz);
> +		return -E2BIG;
> +	}
> +
> +	if (!(flags & TPM_TRANSMIT_UNLOCKED))
> +		mutex_lock(&chip->tpm_mutex);
> +
> +	if (chip->dev.parent)
> +		pm_runtime_get_sync(chip->dev.parent);
> +
> +	/* Store the decision as chip->locality will be changed. */
> +	need_locality = chip->locality == -1;
> +
> +	if (need_locality && chip->ops->request_locality)  {
> +		rc = chip->ops->request_locality(chip, 0);
> +		if (rc < 0)
> +			goto out_no_locality;
> +		chip->locality = rc;
>   	}
>
> +	rc = tpm2_prepare_space(chip, space, ordinal, buf);
> +	if (rc)
> +		goto out;
> +
> +	len = tpm_transfer(chip, buf, count, bufsiz);
> +	if (len < 0)
> +		goto out;
> +
>   	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
>
>   out:


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
  2017-05-10 23:54   ` Stefan Berger
@ 2017-05-15 12:40     ` Jarkko Sakkinen
  -1 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-15 12:40 UTC (permalink / raw)
  To: Stefan Berger
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On Wed, May 10, 2017 at 07:54:21PM -0400, Stefan Berger wrote:
> Refactor tpm_transmit and pull out code sending the command
> and receiving the response and put this into tpm_transfer.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
>  1 file changed, 73 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 158c1db..263b6d1 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
>  }
>  
>  /**
> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + * tmp_transfer - Send a TPM command to the TPM and receive response
>   *
>   * @chip: TPM chip to use
>   * @buf: TPM command buffer
> + * @count: size of the TPM command
>   * @bufsiz: length of the TPM command buffer
> - * @flags: tpm transmit flags - bitmap
>   *
>   * Return:
> - *     0 when the operation is successful.
> + *     >0 when the operation is successful; returns response length
>   *     A negative number for system errors (errno).
>   */
> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> -		     u8 *buf, size_t bufsiz, unsigned int flags)
> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)

Add instead a flag TPM_TRANSMIT_RAW (this name is just a suggestion)
that skips "prepare" and "commit" parts. That would save us from a
new export.

Better way to make it less messy would be to add static functions
tpm_prepare_command and tpm_commit_command that would be always
called and would return immediately if flags contain TPM_TRANSMIT_RAW.

/Jarkko

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

* [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
@ 2017-05-15 12:40     ` Jarkko Sakkinen
  0 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-15 12:40 UTC (permalink / raw)
  To: linux-security-module

On Wed, May 10, 2017 at 07:54:21PM -0400, Stefan Berger wrote:
> Refactor tpm_transmit and pull out code sending the command
> and receiving the response and put this into tpm_transfer.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
>  1 file changed, 73 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 158c1db..263b6d1 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
>  }
>  
>  /**
> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + * tmp_transfer - Send a TPM command to the TPM and receive response
>   *
>   * @chip: TPM chip to use
>   * @buf: TPM command buffer
> + * @count: size of the TPM command
>   * @bufsiz: length of the TPM command buffer
> - * @flags: tpm transmit flags - bitmap
>   *
>   * Return:
> - *     0 when the operation is successful.
> + *     >0 when the operation is successful; returns response length
>   *     A negative number for system errors (errno).
>   */
> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> -		     u8 *buf, size_t bufsiz, unsigned int flags)
> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)

Add instead a flag TPM_TRANSMIT_RAW (this name is just a suggestion)
that skips "prepare" and "commit" parts. That would save us from a
new export.

Better way to make it less messy would be to add static functions
tpm_prepare_command and tpm_commit_command that would be always
called and would return immediately if flags contain TPM_TRANSMIT_RAW.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-10 23:54   ` Stefan Berger
@ 2017-05-15 12:41     ` Jarkko Sakkinen
  -1 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-15 12:41 UTC (permalink / raw)
  To: Stefan Berger
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
> Implement the request_locality function. To set the locality on the
> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> a command to the backend to set the locality for the next commands.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm.h            |  1 +
>  drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vtpm_proxy.h   |  5 +++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4b4c8de..10f0467 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
>  	TPM_TRANSMIT_UNLOCKED	= BIT(0),
>  };
>  
> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
>  ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>  		     u8 *buf, size_t bufsiz, unsigned int flags);
>  ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 751059d..374c4ff 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
>  	return ret;
>  }
>  
> +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> +{
> +	struct tpm_buf buf;
> +	int rc;
> +	const struct tpm_output_header *header;
> +
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> +				  TPM2_CC_SET_LOCALITY);
> +	else
> +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
> +				  TPM_ORD_SET_LOCALITY);
> +	if (rc)
> +		return rc;
> +	tpm_buf_append_u8(&buf, locality);
> +
> +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
> +	if (rc < 0) {
> +		locality = rc;
> +		goto out;
> +	}
> +
> +	header = (const struct tpm_output_header *)buf.data;
> +	rc = be32_to_cpu(header->return_code);
> +	if (rc)
> +		locality = -1;
> +
> +out:
> +	tpm_buf_destroy(&buf);
> +
> +	return locality;
> +}
> +
>  static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>  	.flags = TPM_OPS_AUTO_STARTUP,
>  	.recv = vtpm_proxy_tpm_op_recv,
> @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>  	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
>  	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
>  	.req_canceled = vtpm_proxy_tpm_req_canceled,
> +	.request_locality = vtpm_proxy_request_locality,
>  };
>  
>  /*
> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> index a69e991..6b91c2c 100644
> --- a/include/uapi/linux/vtpm_proxy.h
> +++ b/include/uapi/linux/vtpm_proxy.h
> @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
>  
>  #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
>  
> +/* vendor specific commands to set locality */
> +#define TPM2_CC_SET_LOCALITY	0x20001000
> +#define TPM_ORD_SET_LOCALITY	0x20001000
> +
> +
>  #endif /* _UAPI_LINUX_VTPM_PROXY_H */
> -- 
> 2.4.3

This is a question.

Did you have some kind of big idea with 0x20? I was just thinking
that one way to do this would to allocate starting from 0xFFFFFFFF.

/Jarkko

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-15 12:41     ` Jarkko Sakkinen
  0 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-15 12:41 UTC (permalink / raw)
  To: linux-security-module

On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
> Implement the request_locality function. To set the locality on the
> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> a command to the backend to set the locality for the next commands.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm.h            |  1 +
>  drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vtpm_proxy.h   |  5 +++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4b4c8de..10f0467 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
>  	TPM_TRANSMIT_UNLOCKED	= BIT(0),
>  };
>  
> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
>  ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>  		     u8 *buf, size_t bufsiz, unsigned int flags);
>  ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 751059d..374c4ff 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
>  	return ret;
>  }
>  
> +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> +{
> +	struct tpm_buf buf;
> +	int rc;
> +	const struct tpm_output_header *header;
> +
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> +				  TPM2_CC_SET_LOCALITY);
> +	else
> +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
> +				  TPM_ORD_SET_LOCALITY);
> +	if (rc)
> +		return rc;
> +	tpm_buf_append_u8(&buf, locality);
> +
> +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
> +	if (rc < 0) {
> +		locality = rc;
> +		goto out;
> +	}
> +
> +	header = (const struct tpm_output_header *)buf.data;
> +	rc = be32_to_cpu(header->return_code);
> +	if (rc)
> +		locality = -1;
> +
> +out:
> +	tpm_buf_destroy(&buf);
> +
> +	return locality;
> +}
> +
>  static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>  	.flags = TPM_OPS_AUTO_STARTUP,
>  	.recv = vtpm_proxy_tpm_op_recv,
> @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>  	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
>  	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
>  	.req_canceled = vtpm_proxy_tpm_req_canceled,
> +	.request_locality = vtpm_proxy_request_locality,
>  };
>  
>  /*
> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> index a69e991..6b91c2c 100644
> --- a/include/uapi/linux/vtpm_proxy.h
> +++ b/include/uapi/linux/vtpm_proxy.h
> @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
>  
>  #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
>  
> +/* vendor specific commands to set locality */
> +#define TPM2_CC_SET_LOCALITY	0x20001000
> +#define TPM_ORD_SET_LOCALITY	0x20001000
> +
> +
>  #endif /* _UAPI_LINUX_VTPM_PROXY_H */
> -- 
> 2.4.3

This is a question.

Did you have some kind of big idea with 0x20? I was just thinking
that one way to do this would to allocate starting from 0xFFFFFFFF.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-15 12:41     ` Jarkko Sakkinen
@ 2017-05-15 15:56       ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-15 15:56 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
> On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
>> Implement the request_locality function. To set the locality on the
>> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
>> a command to the backend to set the locality for the next commands.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   drivers/char/tpm/tpm.h            |  1 +
>>   drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/vtpm_proxy.h   |  5 +++++
>>   3 files changed, 40 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 4b4c8de..10f0467 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
>>   	TPM_TRANSMIT_UNLOCKED	= BIT(0),
>>   };
>>   
>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
>>   ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>>   		     u8 *buf, size_t bufsiz, unsigned int flags);
>>   ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
>> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>> index 751059d..374c4ff 100644
>> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
>> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>> @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
>>   	return ret;
>>   }
>>   
>> +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
>> +{
>> +	struct tpm_buf buf;
>> +	int rc;
>> +	const struct tpm_output_header *header;
>> +
>> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
>> +				  TPM2_CC_SET_LOCALITY);
>> +	else
>> +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
>> +				  TPM_ORD_SET_LOCALITY);
>> +	if (rc)
>> +		return rc;
>> +	tpm_buf_append_u8(&buf, locality);
>> +
>> +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
>> +	if (rc < 0) {
>> +		locality = rc;
>> +		goto out;
>> +	}
>> +
>> +	header = (const struct tpm_output_header *)buf.data;
>> +	rc = be32_to_cpu(header->return_code);
>> +	if (rc)
>> +		locality = -1;
>> +
>> +out:
>> +	tpm_buf_destroy(&buf);
>> +
>> +	return locality;
>> +}
>> +
>>   static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>   	.flags = TPM_OPS_AUTO_STARTUP,
>>   	.recv = vtpm_proxy_tpm_op_recv,
>> @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>   	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>   	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>   	.req_canceled = vtpm_proxy_tpm_req_canceled,
>> +	.request_locality = vtpm_proxy_request_locality,
>>   };
>>   
>>   /*
>> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
>> index a69e991..6b91c2c 100644
>> --- a/include/uapi/linux/vtpm_proxy.h
>> +++ b/include/uapi/linux/vtpm_proxy.h
>> @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
>>   
>>   #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
>>   
>> +/* vendor specific commands to set locality */
>> +#define TPM2_CC_SET_LOCALITY	0x20001000
>> +#define TPM_ORD_SET_LOCALITY	0x20001000
>> +
>> +
>>   #endif /* _UAPI_LINUX_VTPM_PROXY_H */
>> -- 
>> 2.4.3
> This is a question.
>
> Did you have some kind of big idea with 0x20? I was just thinking
> that one way to do this would to allocate starting from 0xFFFFFFFF.

Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with 
Vendor specific commands and they are identified by bit 29.

Check section 17 'Ordinals': 
https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf

Check section 8.9 TPMA_CC: 
https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf

If anything was the 'big idea' then it was the offset '0x1000' :-)

    Stefan

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-15 15:56       ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-15 15:56 UTC (permalink / raw)
  To: linux-security-module

On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
> On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
>> Implement the request_locality function. To set the locality on the
>> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
>> a command to the backend to set the locality for the next commands.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   drivers/char/tpm/tpm.h            |  1 +
>>   drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/vtpm_proxy.h   |  5 +++++
>>   3 files changed, 40 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 4b4c8de..10f0467 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
>>   	TPM_TRANSMIT_UNLOCKED	= BIT(0),
>>   };
>>   
>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
>>   ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>>   		     u8 *buf, size_t bufsiz, unsigned int flags);
>>   ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
>> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>> index 751059d..374c4ff 100644
>> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
>> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>> @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
>>   	return ret;
>>   }
>>   
>> +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
>> +{
>> +	struct tpm_buf buf;
>> +	int rc;
>> +	const struct tpm_output_header *header;
>> +
>> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
>> +				  TPM2_CC_SET_LOCALITY);
>> +	else
>> +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
>> +				  TPM_ORD_SET_LOCALITY);
>> +	if (rc)
>> +		return rc;
>> +	tpm_buf_append_u8(&buf, locality);
>> +
>> +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
>> +	if (rc < 0) {
>> +		locality = rc;
>> +		goto out;
>> +	}
>> +
>> +	header = (const struct tpm_output_header *)buf.data;
>> +	rc = be32_to_cpu(header->return_code);
>> +	if (rc)
>> +		locality = -1;
>> +
>> +out:
>> +	tpm_buf_destroy(&buf);
>> +
>> +	return locality;
>> +}
>> +
>>   static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>   	.flags = TPM_OPS_AUTO_STARTUP,
>>   	.recv = vtpm_proxy_tpm_op_recv,
>> @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>   	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>   	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>   	.req_canceled = vtpm_proxy_tpm_req_canceled,
>> +	.request_locality = vtpm_proxy_request_locality,
>>   };
>>   
>>   /*
>> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
>> index a69e991..6b91c2c 100644
>> --- a/include/uapi/linux/vtpm_proxy.h
>> +++ b/include/uapi/linux/vtpm_proxy.h
>> @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
>>   
>>   #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
>>   
>> +/* vendor specific commands to set locality */
>> +#define TPM2_CC_SET_LOCALITY	0x20001000
>> +#define TPM_ORD_SET_LOCALITY	0x20001000
>> +
>> +
>>   #endif /* _UAPI_LINUX_VTPM_PROXY_H */
>> -- 
>> 2.4.3
> This is a question.
>
> Did you have some kind of big idea with 0x20? I was just thinking
> that one way to do this would to allocate starting from 0xFFFFFFFF.

Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with 
Vendor specific commands and they are identified by bit 29.

Check section 17 'Ordinals': 
https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf

Check section 8.9 TPMA_CC: 
https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf

If anything was the 'big idea' then it was the offset '0x1000' :-)

    Stefan


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
  2017-05-15 12:40     ` Jarkko Sakkinen
@ 2017-05-15 16:04       ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-15 16:04 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On 05/15/2017 08:40 AM, Jarkko Sakkinen wrote:
> On Wed, May 10, 2017 at 07:54:21PM -0400, Stefan Berger wrote:
>> Refactor tpm_transmit and pull out code sending the command
>> and receiving the response and put this into tpm_transfer.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
>>   1 file changed, 73 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index 158c1db..263b6d1 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
>>   }
>>   
>>   /**
>> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
>> + * tmp_transfer - Send a TPM command to the TPM and receive response
>>    *
>>    * @chip: TPM chip to use
>>    * @buf: TPM command buffer
>> + * @count: size of the TPM command
>>    * @bufsiz: length of the TPM command buffer
>> - * @flags: tpm transmit flags - bitmap
>>    *
>>    * Return:
>> - *     0 when the operation is successful.
>> + *     >0 when the operation is successful; returns response length
>>    *     A negative number for system errors (errno).
>>    */
>> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>> -		     u8 *buf, size_t bufsiz, unsigned int flags)
>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)
> Add instead a flag TPM_TRANSMIT_RAW (this name is just a suggestion)
> that skips "prepare" and "commit" parts. That would save us from a
> new export.
>
> Better way to make it less messy would be to add static functions
> tpm_prepare_command and tpm_commit_command that would be always
> called and would return immediately if flags contain TPM_TRANSMIT_RAW.

I'll do that then, modifying the code to skip tpm_validate_command as well.

    Stefan

>
> /Jarkko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

* [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
@ 2017-05-15 16:04       ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-15 16:04 UTC (permalink / raw)
  To: linux-security-module

On 05/15/2017 08:40 AM, Jarkko Sakkinen wrote:
> On Wed, May 10, 2017 at 07:54:21PM -0400, Stefan Berger wrote:
>> Refactor tpm_transmit and pull out code sending the command
>> and receiving the response and put this into tpm_transfer.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++----------------
>>   1 file changed, 73 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index 158c1db..263b6d1 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct tpm_chip *chip,
>>   }
>>   
>>   /**
>> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
>> + * tmp_transfer - Send a TPM command to the TPM and receive response
>>    *
>>    * @chip: TPM chip to use
>>    * @buf: TPM command buffer
>> + * @count: size of the TPM command
>>    * @bufsiz: length of the TPM command buffer
>> - * @flags: tpm transmit flags - bitmap
>>    *
>>    * Return:
>> - *     0 when the operation is successful.
>> + *     >0 when the operation is successful; returns response length
>>    *     A negative number for system errors (errno).
>>    */
>> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>> -		     u8 *buf, size_t bufsiz, unsigned int flags)
>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz)
> Add instead a flag TPM_TRANSMIT_RAW (this name is just a suggestion)
> that skips "prepare" and "commit" parts. That would save us from a
> new export.
>
> Better way to make it less messy would be to add static functions
> tpm_prepare_command and tpm_commit_command that would be always
> called and would return immediately if flags contain TPM_TRANSMIT_RAW.

I'll do that then, modifying the code to skip tpm_validate_command as well.

    Stefan

>
> /Jarkko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
  2017-05-15 16:04       ` Stefan Berger
@ 2017-05-15 16:06         ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-15 16:06 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On 05/15/2017 12:04 PM, Stefan Berger wrote:
> On 05/15/2017 08:40 AM, Jarkko Sakkinen wrote:
>> On Wed, May 10, 2017 at 07:54:21PM -0400, Stefan Berger wrote:
>>> Refactor tpm_transmit and pull out code sending the command
>>> and receiving the response and put this into tpm_transfer.
>>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>> ---
>>>   drivers/char/tpm/tpm-interface.c | 121 
>>> +++++++++++++++++++++++----------------
>>>   1 file changed, 73 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/char/tpm/tpm-interface.c 
>>> b/drivers/char/tpm/tpm-interface.c
>>> index 158c1db..263b6d1 100644
>>> --- a/drivers/char/tpm/tpm-interface.c
>>> +++ b/drivers/char/tpm/tpm-interface.c
>>> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct 
>>> tpm_chip *chip,
>>>   }
>>>     /**
>>> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
>>> + * tmp_transfer - Send a TPM command to the TPM and receive response
>>>    *
>>>    * @chip: TPM chip to use
>>>    * @buf: TPM command buffer
>>> + * @count: size of the TPM command
>>>    * @bufsiz: length of the TPM command buffer
>>> - * @flags: tpm transmit flags - bitmap
>>>    *
>>>    * Return:
>>> - *     0 when the operation is successful.
>>> + *     >0 when the operation is successful; returns response length
>>>    *     A negative number for system errors (errno).
>>>    */
>>> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>>> -             u8 *buf, size_t bufsiz, unsigned int flags)
>>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, 
>>> size_t bufsiz)
>> Add instead a flag TPM_TRANSMIT_RAW (this name is just a suggestion)
>> that skips "prepare" and "commit" parts. That would save us from a
>> new export.
>>
>> Better way to make it less messy would be to add static functions
>> tpm_prepare_command and tpm_commit_command that would be always
>> called and would return immediately if flags contain TPM_TRANSMIT_RAW.
>
> I'll do that then, modifying the code to skip tpm_validate_command as 
> well.
>

But we don't want to recurse (in the 2nd patch) into 
chip->ops->request_locality(). Any suggestions ?

    Stefan

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

* [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function
@ 2017-05-15 16:06         ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-15 16:06 UTC (permalink / raw)
  To: linux-security-module

On 05/15/2017 12:04 PM, Stefan Berger wrote:
> On 05/15/2017 08:40 AM, Jarkko Sakkinen wrote:
>> On Wed, May 10, 2017 at 07:54:21PM -0400, Stefan Berger wrote:
>>> Refactor tpm_transmit and pull out code sending the command
>>> and receiving the response and put this into tpm_transfer.
>>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>> ---
>>>   drivers/char/tpm/tpm-interface.c | 121 
>>> +++++++++++++++++++++++----------------
>>>   1 file changed, 73 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/char/tpm/tpm-interface.c 
>>> b/drivers/char/tpm/tpm-interface.c
>>> index 158c1db..263b6d1 100644
>>> --- a/drivers/char/tpm/tpm-interface.c
>>> +++ b/drivers/char/tpm/tpm-interface.c
>>> @@ -370,67 +370,29 @@ static bool tpm_validate_command(struct 
>>> tpm_chip *chip,
>>>   }
>>>     /**
>>> - * tmp_transmit - Internal kernel interface to transmit TPM commands.
>>> + * tmp_transfer - Send a TPM command to the TPM and receive response
>>>    *
>>>    * @chip: TPM chip to use
>>>    * @buf: TPM command buffer
>>> + * @count: size of the TPM command
>>>    * @bufsiz: length of the TPM command buffer
>>> - * @flags: tpm transmit flags - bitmap
>>>    *
>>>    * Return:
>>> - *     0 when the operation is successful.
>>> + *     >0 when the operation is successful; returns response length
>>>    *     A negative number for system errors (errno).
>>>    */
>>> -ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>>> -             u8 *buf, size_t bufsiz, unsigned int flags)
>>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, 
>>> size_t bufsiz)
>> Add instead a flag TPM_TRANSMIT_RAW (this name is just a suggestion)
>> that skips "prepare" and "commit" parts. That would save us from a
>> new export.
>>
>> Better way to make it less messy would be to add static functions
>> tpm_prepare_command and tpm_commit_command that would be always
>> called and would return immediately if flags contain TPM_TRANSMIT_RAW.
>
> I'll do that then, modifying the code to skip tpm_validate_command as 
> well.
>

But we don't want to recurse (in the 2nd patch) into 
chip->ops->request_locality(). Any suggestions ?

    Stefan

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [tpmdd-devel] [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-10 23:54   ` Stefan Berger
@ 2017-05-16 19:03     ` Ken Goldman
  -1 siblings, 0 replies; 29+ messages in thread
From: Ken Goldman @ 2017-05-16 19:03 UTC (permalink / raw)
  To: tpmdd-devel, linux-security-module; +Cc: linux-kernel

On 5/10/2017 7:54 PM, Stefan Berger wrote:
> Implement the request_locality function. To set the locality on the
> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> a command to the backend to set the locality for the next commands.

When this says "for the next commands", does that mean that the locality
is fixed for all commands until a new request_locality is sent to change it?

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

* [tpmdd-devel] [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-16 19:03     ` Ken Goldman
  0 siblings, 0 replies; 29+ messages in thread
From: Ken Goldman @ 2017-05-16 19:03 UTC (permalink / raw)
  To: linux-security-module

On 5/10/2017 7:54 PM, Stefan Berger wrote:
> Implement the request_locality function. To set the locality on the
> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> a command to the backend to set the locality for the next commands.

When this says "for the next commands", does that mean that the locality
is fixed for all commands until a new request_locality is sent to change it?

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [tpmdd-devel] [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-16 19:03     ` Ken Goldman
@ 2017-05-16 19:32       ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-16 19:32 UTC (permalink / raw)
  To: Ken Goldman, tpmdd-devel, linux-security-module; +Cc: linux-kernel

On 05/16/2017 03:03 PM, Ken Goldman wrote:
> On 5/10/2017 7:54 PM, Stefan Berger wrote:
>> Implement the request_locality function. To set the locality on the
>> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
>> a command to the backend to set the locality for the next commands.
>
> When this says "for the next commands", does that mean that the locality
> is fixed for all commands until a new request_locality is sent to 
> change it?

Yes.

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

* [tpmdd-devel] [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-16 19:32       ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-16 19:32 UTC (permalink / raw)
  To: linux-security-module

On 05/16/2017 03:03 PM, Ken Goldman wrote:
> On 5/10/2017 7:54 PM, Stefan Berger wrote:
>> Implement the request_locality function. To set the locality on the
>> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
>> a command to the backend to set the locality for the next commands.
>
> When this says "for the next commands", does that mean that the locality
> is fixed for all commands until a new request_locality is sent to 
> change it?

Yes.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-15 15:56       ` Stefan Berger
@ 2017-05-20 12:47         ` Jarkko Sakkinen
  -1 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-20 12:47 UTC (permalink / raw)
  To: Stefan Berger
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On Mon, May 15, 2017 at 11:56:51AM -0400, Stefan Berger wrote:
> On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
> > On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
> > > Implement the request_locality function. To set the locality on the
> > > backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> > > a command to the backend to set the locality for the next commands.
> > > 
> > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > > ---
> > >   drivers/char/tpm/tpm.h            |  1 +
> > >   drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
> > >   include/uapi/linux/vtpm_proxy.h   |  5 +++++
> > >   3 files changed, 40 insertions(+)
> > > 
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 4b4c8de..10f0467 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
> > >   	TPM_TRANSMIT_UNLOCKED	= BIT(0),
> > >   };
> > > +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
> > >   ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> > >   		     u8 *buf, size_t bufsiz, unsigned int flags);
> > >   ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> > > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > index 751059d..374c4ff 100644
> > > --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> > > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
> > >   	return ret;
> > >   }
> > > +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> > > +{
> > > +	struct tpm_buf buf;
> > > +	int rc;
> > > +	const struct tpm_output_header *header;
> > > +
> > > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > > +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> > > +				  TPM2_CC_SET_LOCALITY);
> > > +	else
> > > +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
> > > +				  TPM_ORD_SET_LOCALITY);
> > > +	if (rc)
> > > +		return rc;
> > > +	tpm_buf_append_u8(&buf, locality);
> > > +
> > > +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
> > > +	if (rc < 0) {
> > > +		locality = rc;
> > > +		goto out;
> > > +	}
> > > +
> > > +	header = (const struct tpm_output_header *)buf.data;
> > > +	rc = be32_to_cpu(header->return_code);
> > > +	if (rc)
> > > +		locality = -1;
> > > +
> > > +out:
> > > +	tpm_buf_destroy(&buf);
> > > +
> > > +	return locality;
> > > +}
> > > +
> > >   static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > >   	.flags = TPM_OPS_AUTO_STARTUP,
> > >   	.recv = vtpm_proxy_tpm_op_recv,
> > > @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > >   	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > >   	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > >   	.req_canceled = vtpm_proxy_tpm_req_canceled,
> > > +	.request_locality = vtpm_proxy_request_locality,
> > >   };
> > >   /*
> > > diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> > > index a69e991..6b91c2c 100644
> > > --- a/include/uapi/linux/vtpm_proxy.h
> > > +++ b/include/uapi/linux/vtpm_proxy.h
> > > @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
> > >   #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
> > > +/* vendor specific commands to set locality */
> > > +#define TPM2_CC_SET_LOCALITY	0x20001000
> > > +#define TPM_ORD_SET_LOCALITY	0x20001000
> > > +
> > > +
> > >   #endif /* _UAPI_LINUX_VTPM_PROXY_H */
> > > -- 
> > > 2.4.3
> > This is a question.
> > 
> > Did you have some kind of big idea with 0x20? I was just thinking
> > that one way to do this would to allocate starting from 0xFFFFFFFF.
> 
> Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with Vendor
> specific commands and they are identified by bit 29.
> 
> Check section 17 'Ordinals': https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf
> 
> Check section 8.9 TPMA_CC: https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf
> 
> If anything was the 'big idea' then it was the offset '0x1000' :-)
> 
>    Stefan

Maybe we should use have concept of driver specific commands? You could
in theory use vtpm to proxy a real chip and this would cause a potential
regression as vendor specific code might collide.

/Jarkko

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-20 12:47         ` Jarkko Sakkinen
  0 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-20 12:47 UTC (permalink / raw)
  To: linux-security-module

On Mon, May 15, 2017 at 11:56:51AM -0400, Stefan Berger wrote:
> On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
> > On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
> > > Implement the request_locality function. To set the locality on the
> > > backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> > > a command to the backend to set the locality for the next commands.
> > > 
> > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > > ---
> > >   drivers/char/tpm/tpm.h            |  1 +
> > >   drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
> > >   include/uapi/linux/vtpm_proxy.h   |  5 +++++
> > >   3 files changed, 40 insertions(+)
> > > 
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 4b4c8de..10f0467 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
> > >   	TPM_TRANSMIT_UNLOCKED	= BIT(0),
> > >   };
> > > +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
> > >   ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> > >   		     u8 *buf, size_t bufsiz, unsigned int flags);
> > >   ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> > > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > index 751059d..374c4ff 100644
> > > --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> > > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
> > >   	return ret;
> > >   }
> > > +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> > > +{
> > > +	struct tpm_buf buf;
> > > +	int rc;
> > > +	const struct tpm_output_header *header;
> > > +
> > > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > > +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> > > +				  TPM2_CC_SET_LOCALITY);
> > > +	else
> > > +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
> > > +				  TPM_ORD_SET_LOCALITY);
> > > +	if (rc)
> > > +		return rc;
> > > +	tpm_buf_append_u8(&buf, locality);
> > > +
> > > +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
> > > +	if (rc < 0) {
> > > +		locality = rc;
> > > +		goto out;
> > > +	}
> > > +
> > > +	header = (const struct tpm_output_header *)buf.data;
> > > +	rc = be32_to_cpu(header->return_code);
> > > +	if (rc)
> > > +		locality = -1;
> > > +
> > > +out:
> > > +	tpm_buf_destroy(&buf);
> > > +
> > > +	return locality;
> > > +}
> > > +
> > >   static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > >   	.flags = TPM_OPS_AUTO_STARTUP,
> > >   	.recv = vtpm_proxy_tpm_op_recv,
> > > @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > >   	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > >   	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > >   	.req_canceled = vtpm_proxy_tpm_req_canceled,
> > > +	.request_locality = vtpm_proxy_request_locality,
> > >   };
> > >   /*
> > > diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> > > index a69e991..6b91c2c 100644
> > > --- a/include/uapi/linux/vtpm_proxy.h
> > > +++ b/include/uapi/linux/vtpm_proxy.h
> > > @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
> > >   #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
> > > +/* vendor specific commands to set locality */
> > > +#define TPM2_CC_SET_LOCALITY	0x20001000
> > > +#define TPM_ORD_SET_LOCALITY	0x20001000
> > > +
> > > +
> > >   #endif /* _UAPI_LINUX_VTPM_PROXY_H */
> > > -- 
> > > 2.4.3
> > This is a question.
> > 
> > Did you have some kind of big idea with 0x20? I was just thinking
> > that one way to do this would to allocate starting from 0xFFFFFFFF.
> 
> Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with Vendor
> specific commands and they are identified by bit 29.
> 
> Check section 17 'Ordinals': https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf
> 
> Check section 8.9 TPMA_CC: https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf
> 
> If anything was the 'big idea' then it was the offset '0x1000' :-)
> 
>    Stefan

Maybe we should use have concept of driver specific commands? You could
in theory use vtpm to proxy a real chip and this would cause a potential
regression as vendor specific code might collide.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-20 12:47         ` Jarkko Sakkinen
@ 2017-05-21 23:52           ` Stefan Berger
  -1 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-21 23:52 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On 05/20/2017 08:47 AM, Jarkko Sakkinen wrote:
> On Mon, May 15, 2017 at 11:56:51AM -0400, Stefan Berger wrote:
>> On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
>>> On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
>>>> Implement the request_locality function. To set the locality on the
>>>> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
>>>> a command to the backend to set the locality for the next commands.
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>>> ---
>>>>    drivers/char/tpm/tpm.h            |  1 +
>>>>    drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
>>>>    include/uapi/linux/vtpm_proxy.h   |  5 +++++
>>>>    3 files changed, 40 insertions(+)
>>>>
>>>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>>>> index 4b4c8de..10f0467 100644
>>>> --- a/drivers/char/tpm/tpm.h
>>>> +++ b/drivers/char/tpm/tpm.h
>>>> @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
>>>>    	TPM_TRANSMIT_UNLOCKED	= BIT(0),
>>>>    };
>>>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
>>>>    ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>>>>    		     u8 *buf, size_t bufsiz, unsigned int flags);
>>>>    ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
>>>> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>>>> index 751059d..374c4ff 100644
>>>> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
>>>> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>>>> @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
>>>>    	return ret;
>>>>    }
>>>> +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
>>>> +{
>>>> +	struct tpm_buf buf;
>>>> +	int rc;
>>>> +	const struct tpm_output_header *header;
>>>> +
>>>> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>>>> +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
>>>> +				  TPM2_CC_SET_LOCALITY);
>>>> +	else
>>>> +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
>>>> +				  TPM_ORD_SET_LOCALITY);
>>>> +	if (rc)
>>>> +		return rc;
>>>> +	tpm_buf_append_u8(&buf, locality);
>>>> +
>>>> +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
>>>> +	if (rc < 0) {
>>>> +		locality = rc;
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	header = (const struct tpm_output_header *)buf.data;
>>>> +	rc = be32_to_cpu(header->return_code);
>>>> +	if (rc)
>>>> +		locality = -1;
>>>> +
>>>> +out:
>>>> +	tpm_buf_destroy(&buf);
>>>> +
>>>> +	return locality;
>>>> +}
>>>> +
>>>>    static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>>>    	.flags = TPM_OPS_AUTO_STARTUP,
>>>>    	.recv = vtpm_proxy_tpm_op_recv,
>>>> @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>>>    	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>>>    	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>>>    	.req_canceled = vtpm_proxy_tpm_req_canceled,
>>>> +	.request_locality = vtpm_proxy_request_locality,
>>>>    };
>>>>    /*
>>>> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
>>>> index a69e991..6b91c2c 100644
>>>> --- a/include/uapi/linux/vtpm_proxy.h
>>>> +++ b/include/uapi/linux/vtpm_proxy.h
>>>> @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
>>>>    #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
>>>> +/* vendor specific commands to set locality */
>>>> +#define TPM2_CC_SET_LOCALITY	0x20001000
>>>> +#define TPM_ORD_SET_LOCALITY	0x20001000
>>>> +
>>>> +
>>>>    #endif /* _UAPI_LINUX_VTPM_PROXY_H */
>>>> -- 
>>>> 2.4.3
>>> This is a question.
>>>
>>> Did you have some kind of big idea with 0x20? I was just thinking
>>> that one way to do this would to allocate starting from 0xFFFFFFFF.
>> Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with Vendor
>> specific commands and they are identified by bit 29.
>>
>> Check section 17 'Ordinals': https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf
>>
>> Check section 8.9 TPMA_CC: https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf
>>
>> If anything was the 'big idea' then it was the offset '0x1000' :-)
>>
>>     Stefan
> Maybe we should use have concept of driver specific commands? You could
> in theory use vtpm to proxy a real chip and this would cause a potential
> regression as vendor specific code might collide.

As long as the recipient knows where the command is coming from, it can 
interpret it and convert it to whatever the target understands. In this 
case it would only need to know that these commands are coming from the 
Linux vTPM proxy device driver with the meaning of setting the locality.

I think the danger of collision is low and anything like driver specific 
commands seems like it would have to become a standard within TCG where 
they would allocate a range for those types of commands. I'd rather not 
go down that road.

    Stefan

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-21 23:52           ` Stefan Berger
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Berger @ 2017-05-21 23:52 UTC (permalink / raw)
  To: linux-security-module

On 05/20/2017 08:47 AM, Jarkko Sakkinen wrote:
> On Mon, May 15, 2017 at 11:56:51AM -0400, Stefan Berger wrote:
>> On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
>>> On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
>>>> Implement the request_locality function. To set the locality on the
>>>> backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
>>>> a command to the backend to set the locality for the next commands.
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>>> ---
>>>>    drivers/char/tpm/tpm.h            |  1 +
>>>>    drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
>>>>    include/uapi/linux/vtpm_proxy.h   |  5 +++++
>>>>    3 files changed, 40 insertions(+)
>>>>
>>>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>>>> index 4b4c8de..10f0467 100644
>>>> --- a/drivers/char/tpm/tpm.h
>>>> +++ b/drivers/char/tpm/tpm.h
>>>> @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
>>>>    	TPM_TRANSMIT_UNLOCKED	= BIT(0),
>>>>    };
>>>> +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
>>>>    ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>>>>    		     u8 *buf, size_t bufsiz, unsigned int flags);
>>>>    ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
>>>> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>>>> index 751059d..374c4ff 100644
>>>> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
>>>> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>>>> @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
>>>>    	return ret;
>>>>    }
>>>> +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
>>>> +{
>>>> +	struct tpm_buf buf;
>>>> +	int rc;
>>>> +	const struct tpm_output_header *header;
>>>> +
>>>> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>>>> +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
>>>> +				  TPM2_CC_SET_LOCALITY);
>>>> +	else
>>>> +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
>>>> +				  TPM_ORD_SET_LOCALITY);
>>>> +	if (rc)
>>>> +		return rc;
>>>> +	tpm_buf_append_u8(&buf, locality);
>>>> +
>>>> +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
>>>> +	if (rc < 0) {
>>>> +		locality = rc;
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	header = (const struct tpm_output_header *)buf.data;
>>>> +	rc = be32_to_cpu(header->return_code);
>>>> +	if (rc)
>>>> +		locality = -1;
>>>> +
>>>> +out:
>>>> +	tpm_buf_destroy(&buf);
>>>> +
>>>> +	return locality;
>>>> +}
>>>> +
>>>>    static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>>>    	.flags = TPM_OPS_AUTO_STARTUP,
>>>>    	.recv = vtpm_proxy_tpm_op_recv,
>>>> @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
>>>>    	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>>>    	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
>>>>    	.req_canceled = vtpm_proxy_tpm_req_canceled,
>>>> +	.request_locality = vtpm_proxy_request_locality,
>>>>    };
>>>>    /*
>>>> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
>>>> index a69e991..6b91c2c 100644
>>>> --- a/include/uapi/linux/vtpm_proxy.h
>>>> +++ b/include/uapi/linux/vtpm_proxy.h
>>>> @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
>>>>    #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
>>>> +/* vendor specific commands to set locality */
>>>> +#define TPM2_CC_SET_LOCALITY	0x20001000
>>>> +#define TPM_ORD_SET_LOCALITY	0x20001000
>>>> +
>>>> +
>>>>    #endif /* _UAPI_LINUX_VTPM_PROXY_H */
>>>> -- 
>>>> 2.4.3
>>> This is a question.
>>>
>>> Did you have some kind of big idea with 0x20? I was just thinking
>>> that one way to do this would to allocate starting from 0xFFFFFFFF.
>> Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with Vendor
>> specific commands and they are identified by bit 29.
>>
>> Check section 17 'Ordinals': https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf
>>
>> Check section 8.9 TPMA_CC: https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf
>>
>> If anything was the 'big idea' then it was the offset '0x1000' :-)
>>
>>     Stefan
> Maybe we should use have concept of driver specific commands? You could
> in theory use vtpm to proxy a real chip and this would cause a potential
> regression as vendor specific code might collide.

As long as the recipient knows where the command is coming from, it can 
interpret it and convert it to whatever the target understands. In this 
case it would only need to know that these commands are coming from the 
Linux vTPM proxy device driver with the meaning of setting the locality.

I think the danger of collision is low and anything like driver specific 
commands seems like it would have to become a standard within TCG where 
they would allocate a range for those types of commands. I'd rather not 
go down that road.

    Stefan

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
  2017-05-21 23:52           ` Stefan Berger
@ 2017-05-24 19:25             ` Jarkko Sakkinen
  -1 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-24 19:25 UTC (permalink / raw)
  To: Stefan Berger
  Cc: tpmdd-devel, linux-security-module, jgunthorpe, linux-kernel

On Sun, May 21, 2017 at 07:52:32PM -0400, Stefan Berger wrote:
> On 05/20/2017 08:47 AM, Jarkko Sakkinen wrote:
> > On Mon, May 15, 2017 at 11:56:51AM -0400, Stefan Berger wrote:
> > > On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
> > > > On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
> > > > > Implement the request_locality function. To set the locality on the
> > > > > backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> > > > > a command to the backend to set the locality for the next commands.
> > > > > 
> > > > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > > > > ---
> > > > >    drivers/char/tpm/tpm.h            |  1 +
> > > > >    drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
> > > > >    include/uapi/linux/vtpm_proxy.h   |  5 +++++
> > > > >    3 files changed, 40 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > > > index 4b4c8de..10f0467 100644
> > > > > --- a/drivers/char/tpm/tpm.h
> > > > > +++ b/drivers/char/tpm/tpm.h
> > > > > @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
> > > > >    	TPM_TRANSMIT_UNLOCKED	= BIT(0),
> > > > >    };
> > > > > +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
> > > > >    ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> > > > >    		     u8 *buf, size_t bufsiz, unsigned int flags);
> > > > >    ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> > > > > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > > index 751059d..374c4ff 100644
> > > > > --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > > @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
> > > > >    	return ret;
> > > > >    }
> > > > > +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> > > > > +{
> > > > > +	struct tpm_buf buf;
> > > > > +	int rc;
> > > > > +	const struct tpm_output_header *header;
> > > > > +
> > > > > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > > > > +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> > > > > +				  TPM2_CC_SET_LOCALITY);
> > > > > +	else
> > > > > +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
> > > > > +				  TPM_ORD_SET_LOCALITY);
> > > > > +	if (rc)
> > > > > +		return rc;
> > > > > +	tpm_buf_append_u8(&buf, locality);
> > > > > +
> > > > > +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
> > > > > +	if (rc < 0) {
> > > > > +		locality = rc;
> > > > > +		goto out;
> > > > > +	}
> > > > > +
> > > > > +	header = (const struct tpm_output_header *)buf.data;
> > > > > +	rc = be32_to_cpu(header->return_code);
> > > > > +	if (rc)
> > > > > +		locality = -1;
> > > > > +
> > > > > +out:
> > > > > +	tpm_buf_destroy(&buf);
> > > > > +
> > > > > +	return locality;
> > > > > +}
> > > > > +
> > > > >    static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > > > >    	.flags = TPM_OPS_AUTO_STARTUP,
> > > > >    	.recv = vtpm_proxy_tpm_op_recv,
> > > > > @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > > > >    	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > > > >    	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > > > >    	.req_canceled = vtpm_proxy_tpm_req_canceled,
> > > > > +	.request_locality = vtpm_proxy_request_locality,
> > > > >    };
> > > > >    /*
> > > > > diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> > > > > index a69e991..6b91c2c 100644
> > > > > --- a/include/uapi/linux/vtpm_proxy.h
> > > > > +++ b/include/uapi/linux/vtpm_proxy.h
> > > > > @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
> > > > >    #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
> > > > > +/* vendor specific commands to set locality */
> > > > > +#define TPM2_CC_SET_LOCALITY	0x20001000
> > > > > +#define TPM_ORD_SET_LOCALITY	0x20001000
> > > > > +
> > > > > +
> > > > >    #endif /* _UAPI_LINUX_VTPM_PROXY_H */
> > > > > -- 
> > > > > 2.4.3
> > > > This is a question.
> > > > 
> > > > Did you have some kind of big idea with 0x20? I was just thinking
> > > > that one way to do this would to allocate starting from 0xFFFFFFFF.
> > > Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with Vendor
> > > specific commands and they are identified by bit 29.
> > > 
> > > Check section 17 'Ordinals': https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf
> > > 
> > > Check section 8.9 TPMA_CC: https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf
> > > 
> > > If anything was the 'big idea' then it was the offset '0x1000' :-)
> > > 
> > >     Stefan
> > Maybe we should use have concept of driver specific commands? You could
> > in theory use vtpm to proxy a real chip and this would cause a potential
> > regression as vendor specific code might collide.
> 
> As long as the recipient knows where the command is coming from, it can
> interpret it and convert it to whatever the target understands. In this case
> it would only need to know that these commands are coming from the Linux
> vTPM proxy device driver with the meaning of setting the locality.
> 
> I think the danger of collision is low and anything like driver specific
> commands seems like it would have to become a standard within TCG where they
> would allocate a range for those types of commands. I'd rather not go down
> that road.
> 
>    Stefan

I think this makes sense.

Reviewed-by: Jarkko Sakkinen <jarko.sakkinen@linux.intel.com>

/Jarkko

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

* [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.
@ 2017-05-24 19:25             ` Jarkko Sakkinen
  0 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2017-05-24 19:25 UTC (permalink / raw)
  To: linux-security-module

On Sun, May 21, 2017 at 07:52:32PM -0400, Stefan Berger wrote:
> On 05/20/2017 08:47 AM, Jarkko Sakkinen wrote:
> > On Mon, May 15, 2017 at 11:56:51AM -0400, Stefan Berger wrote:
> > > On 05/15/2017 08:41 AM, Jarkko Sakkinen wrote:
> > > > On Wed, May 10, 2017 at 07:54:22PM -0400, Stefan Berger wrote:
> > > > > Implement the request_locality function. To set the locality on the
> > > > > backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
> > > > > a command to the backend to set the locality for the next commands.
> > > > > 
> > > > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > > > > ---
> > > > >    drivers/char/tpm/tpm.h            |  1 +
> > > > >    drivers/char/tpm/tpm_vtpm_proxy.c | 34 ++++++++++++++++++++++++++++++++++
> > > > >    include/uapi/linux/vtpm_proxy.h   |  5 +++++
> > > > >    3 files changed, 40 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > > > index 4b4c8de..10f0467 100644
> > > > > --- a/drivers/char/tpm/tpm.h
> > > > > +++ b/drivers/char/tpm/tpm.h
> > > > > @@ -527,6 +527,7 @@ enum tpm_transmit_flags {
> > > > >    	TPM_TRANSMIT_UNLOCKED	= BIT(0),
> > > > >    };
> > > > > +ssize_t tpm_transfer(struct tpm_chip *chip, u8 *buf, u32 count, size_t bufsiz);
> > > > >    ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> > > > >    		     u8 *buf, size_t bufsiz, unsigned int flags);
> > > > >    ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> > > > > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > > index 751059d..374c4ff 100644
> > > > > --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > > @@ -371,6 +371,39 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
> > > > >    	return ret;
> > > > >    }
> > > > > +static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
> > > > > +{
> > > > > +	struct tpm_buf buf;
> > > > > +	int rc;
> > > > > +	const struct tpm_output_header *header;
> > > > > +
> > > > > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > > > > +		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
> > > > > +				  TPM2_CC_SET_LOCALITY);
> > > > > +	else
> > > > > +		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
> > > > > +				  TPM_ORD_SET_LOCALITY);
> > > > > +	if (rc)
> > > > > +		return rc;
> > > > > +	tpm_buf_append_u8(&buf, locality);
> > > > > +
> > > > > +	rc = tpm_transfer(chip, buf.data, tpm_buf_length(&buf), PAGE_SIZE);
> > > > > +	if (rc < 0) {
> > > > > +		locality = rc;
> > > > > +		goto out;
> > > > > +	}
> > > > > +
> > > > > +	header = (const struct tpm_output_header *)buf.data;
> > > > > +	rc = be32_to_cpu(header->return_code);
> > > > > +	if (rc)
> > > > > +		locality = -1;
> > > > > +
> > > > > +out:
> > > > > +	tpm_buf_destroy(&buf);
> > > > > +
> > > > > +	return locality;
> > > > > +}
> > > > > +
> > > > >    static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > > > >    	.flags = TPM_OPS_AUTO_STARTUP,
> > > > >    	.recv = vtpm_proxy_tpm_op_recv,
> > > > > @@ -380,6 +413,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> > > > >    	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > > > >    	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
> > > > >    	.req_canceled = vtpm_proxy_tpm_req_canceled,
> > > > > +	.request_locality = vtpm_proxy_request_locality,
> > > > >    };
> > > > >    /*
> > > > > diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> > > > > index a69e991..6b91c2c 100644
> > > > > --- a/include/uapi/linux/vtpm_proxy.h
> > > > > +++ b/include/uapi/linux/vtpm_proxy.h
> > > > > @@ -46,4 +46,9 @@ struct vtpm_proxy_new_dev {
> > > > >    #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
> > > > > +/* vendor specific commands to set locality */
> > > > > +#define TPM2_CC_SET_LOCALITY	0x20001000
> > > > > +#define TPM_ORD_SET_LOCALITY	0x20001000
> > > > > +
> > > > > +
> > > > >    #endif /* _UAPI_LINUX_VTPM_PROXY_H */
> > > > > -- 
> > > > > 2.4.3
> > > > This is a question.
> > > > 
> > > > Did you have some kind of big idea with 0x20? I was just thinking
> > > > that one way to do this would to allocate starting from 0xFFFFFFFF.
> > > Not quite. Per TPM 1.2 and TPM 2 specs we should operate here with Vendor
> > > specific commands and they are identified by bit 29.
> > > 
> > > Check section 17 'Ordinals': https://trustedcomputinggroup.org/wp-content/uploads/TPM-Main-Part-2-TPM-Structures_v1.2_rev116_01032011.pdf
> > > 
> > > Check section 8.9 TPMA_CC: https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf
> > > 
> > > If anything was the 'big idea' then it was the offset '0x1000' :-)
> > > 
> > >     Stefan
> > Maybe we should use have concept of driver specific commands? You could
> > in theory use vtpm to proxy a real chip and this would cause a potential
> > regression as vendor specific code might collide.
> 
> As long as the recipient knows where the command is coming from, it can
> interpret it and convert it to whatever the target understands. In this case
> it would only need to know that these commands are coming from the Linux
> vTPM proxy device driver with the meaning of setting the locality.
> 
> I think the danger of collision is low and anything like driver specific
> commands seems like it would have to become a standard within TCG where they
> would allocate a range for those types of commands. I'd rather not go down
> that road.
> 
>    Stefan

I think this makes sense.

Reviewed-by: Jarkko Sakkinen <jarko.sakkinen@linux.intel.com>

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2017-05-24 19:25 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-10 23:54 [PATCH v4 0/2] Extend the vTPM proxy driver to pass locality Stefan Berger
2017-05-10 23:54 ` Stefan Berger
2017-05-10 23:54 ` [PATCH v4 1/2] tpm: Refactor tpm_transmit pulling out tpm_transfer function Stefan Berger
2017-05-10 23:54   ` Stefan Berger
2017-05-12 13:30   ` Stefan Berger
2017-05-12 13:30     ` Stefan Berger
2017-05-15 12:40   ` Jarkko Sakkinen
2017-05-15 12:40     ` Jarkko Sakkinen
2017-05-15 16:04     ` Stefan Berger
2017-05-15 16:04       ` Stefan Berger
2017-05-15 16:06       ` Stefan Berger
2017-05-15 16:06         ` Stefan Berger
2017-05-10 23:54 ` [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function Stefan Berger
2017-05-10 23:54   ` Stefan Berger
2017-05-10 23:54   ` Stefan Berger
2017-05-15 12:41   ` Jarkko Sakkinen
2017-05-15 12:41     ` Jarkko Sakkinen
2017-05-15 15:56     ` Stefan Berger
2017-05-15 15:56       ` Stefan Berger
2017-05-20 12:47       ` Jarkko Sakkinen
2017-05-20 12:47         ` Jarkko Sakkinen
2017-05-21 23:52         ` Stefan Berger
2017-05-21 23:52           ` Stefan Berger
2017-05-24 19:25           ` Jarkko Sakkinen
2017-05-24 19:25             ` Jarkko Sakkinen
2017-05-16 19:03   ` [tpmdd-devel] " Ken Goldman
2017-05-16 19:03     ` Ken Goldman
2017-05-16 19:32     ` Stefan Berger
2017-05-16 19:32       ` Stefan Berger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.