[PATCH v4 0/4] Migrate all TPM 2.0 commands to use struct tpm_buf

[PATCH v4 0/4] Migrate all TPM 2.0 commands to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, this patch set migrates all TPM 2.0 commands to use it. Eventually,
tpm_transmit_cmd() can take simply struct tpm_buf as its argument and this
interface can be exported to be used by the kernel keyring and potentially
other subsystems.

The ultimate goal of this work is to make constructing TPM commands inside
the kernel simple and robust.

v4:
* Fixed invalid return value from tpm2_probe(). TPM return codes are
  ignored on purpose as we only care about the tag.
* Removed James' fix for the self test as it was separately applied.

v3:
* Fixed error handling in tpm2_get_random()

v2:
* Fixed author information in the commit that fixes the self-test issue,
  removed '\n' from the log message and added the missing tested-by.
  (James: sincere apologies about this)
* Removed the redundant "out of memory" log message from tpm2_shutdown().
* tpm_buf_destroy() was called before using the response data in
  tpm2_probe().
* Added missing tpm_buf_destroy() to tpm2_get_random().

Jarkko Sakkinen (4):
  tpm: migrate tpm2_shutdown() to use struct tpm_buf
  tpm: migrate tpm2_probe() to use struct tpm_buf
  tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf
  tpm: migrate tpm2_get_random() to use struct tpm_buf

 drivers/char/tpm/tpm.h      |  19 ++--
 drivers/char/tpm/tpm2-cmd.c | 242 +++++++++++++++++---------------------------
 2 files changed, 105 insertions(+), 156 deletions(-)

-- 
2.15.1

[PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
prints an error message.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 44 ++++++++++++--------------------------------
 1 file changed, 12 insertions(+), 32 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 96c77c8e7f40..7665661d9230 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,10 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_startup_in {
-	__be16	startup_type;
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -55,7 +51,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_startup_in		startup_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -412,11 +407,8 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 	int rc;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
-	if (rc) {
-		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
-			 handle);
+	if (rc)
 		return;
-	}
 
 	tpm_buf_append_u32(&buf, handle);
 
@@ -762,40 +754,28 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
 
-#define TPM2_SHUTDOWN_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_startup_in))
-
-static const struct tpm_input_header tpm2_shutdown_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
-};
-
 /**
  * tpm2_shutdown() - send shutdown command to the TPM chip
  *
+ * In places where shutdown command is sent there's no much we can do except
+ * print the error code on a system failure.
+ *
  * @chip:		TPM chip to use.
  * @shutdown_type:	shutdown type. The value is either
  *			TPM_SU_CLEAR or TPM_SU_STATE.
  */
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_shutdown_header;
-	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
-			      "stopping the TPM");
-
-	/* In places where shutdown command is sent there's no much we can do
-	 * except print the error code on a system failure.
-	 */
-	if (rc < 0 && rc != -EPIPE)
-		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
-			 rc);
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+	if (rc)
+		return;
+	tpm_buf_append_u16(&buf, shutdown_type);
+	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+			 "stopping the TPM");
+	tpm_buf_destroy(&buf);
 }
 
 /*
-- 
2.15.1

Re: [PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Nayna Jain]

On 03/26/2018 05:44 PM, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing TPM
> commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> prints an error message.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Reviewed-by: Nayna Jain<nayna@linux.vnet.ibm.com>
Tested-by: Nayna Jain<nayna@linux.vnet.ibm.com>

> ---
>   drivers/char/tpm/tpm2-cmd.c | 44 ++++++++++++--------------------------------
>   1 file changed, 12 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 96c77c8e7f40..7665661d9230 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,10 +27,6 @@ enum tpm2_session_attributes {
>   	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>   };
>
> -struct tpm2_startup_in {
> -	__be16	startup_type;
> -} __packed;
> -
>   struct tpm2_get_tpm_pt_in {
>   	__be32	cap_id;
>   	__be32	property_id;
> @@ -55,7 +51,6 @@ struct tpm2_get_random_out {
>   } __packed;
>
>   union tpm2_cmd_params {
> -	struct	tpm2_startup_in		startup_in;
>   	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
>   	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
>   	struct	tpm2_get_random_in	getrandom_in;
> @@ -412,11 +407,8 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>   	int rc;
>
>   	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
> -	if (rc) {
> -		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
> -			 handle);
> +	if (rc)
>   		return;
> -	}
>
>   	tpm_buf_append_u32(&buf, handle);
>
> @@ -762,40 +754,28 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>   }
>   EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
>
> -#define TPM2_SHUTDOWN_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_startup_in))
> -
> -static const struct tpm_input_header tpm2_shutdown_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
> -};
> -
>   /**
>    * tpm2_shutdown() - send shutdown command to the TPM chip
>    *
> + * In places where shutdown command is sent there's no much we can do except
> + * print the error code on a system failure.
> + *
>    * @chip:		TPM chip to use.
>    * @shutdown_type:	shutdown type. The value is either
>    *			TPM_SU_CLEAR or TPM_SU_STATE.
>    */
>   void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
>   {
> -	struct tpm2_cmd cmd;
> +	struct tpm_buf buf;
>   	int rc;
>
> -	cmd.header.in = tpm2_shutdown_header;
> -	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
> -
> -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> -			      "stopping the TPM");
> -
> -	/* In places where shutdown command is sent there's no much we can do
> -	 * except print the error code on a system failure.
> -	 */
> -	if (rc < 0 && rc != -EPIPE)
> -		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
> -			 rc);
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
> +	if (rc)
> +		return;
> +	tpm_buf_append_u16(&buf, shutdown_type);
> +	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> +			 "stopping the TPM");
> +	tpm_buf_destroy(&buf);
>   }
>
>   /*

Re: [PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jerry Snitselaar]

On Mon Mar 26 18, Jarkko Sakkinen wrote:
>In order to make struct tpm_buf the first class object for constructing TPM
>commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
>entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
>prints an error message.
>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

>---
> drivers/char/tpm/tpm2-cmd.c | 44 ++++++++++++--------------------------------
> 1 file changed, 12 insertions(+), 32 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>index 96c77c8e7f40..7665661d9230 100644
>--- a/drivers/char/tpm/tpm2-cmd.c
>+++ b/drivers/char/tpm/tpm2-cmd.c
>@@ -27,10 +27,6 @@ enum tpm2_session_attributes {
> 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> };
>
>-struct tpm2_startup_in {
>-	__be16	startup_type;
>-} __packed;
>-
> struct tpm2_get_tpm_pt_in {
> 	__be32	cap_id;
> 	__be32	property_id;
>@@ -55,7 +51,6 @@ struct tpm2_get_random_out {
> } __packed;
>
> union tpm2_cmd_params {
>-	struct	tpm2_startup_in		startup_in;
> 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
> 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
> 	struct	tpm2_get_random_in	getrandom_in;
>@@ -412,11 +407,8 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
> 	int rc;
>
> 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
>-	if (rc) {
>-		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
>-			 handle);
>+	if (rc)
> 		return;
>-	}
>
> 	tpm_buf_append_u32(&buf, handle);
>
>@@ -762,40 +754,28 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
> }
> EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
>
>-#define TPM2_SHUTDOWN_IN_SIZE \
>-	(sizeof(struct tpm_input_header) + \
>-	 sizeof(struct tpm2_startup_in))
>-
>-static const struct tpm_input_header tpm2_shutdown_header = {
>-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
>-	.length = cpu_to_be32(TPM2_SHUTDOWN_IN_SIZE),
>-	.ordinal = cpu_to_be32(TPM2_CC_SHUTDOWN)
>-};
>-
> /**
>  * tpm2_shutdown() - send shutdown command to the TPM chip
>  *
>+ * In places where shutdown command is sent there's no much we can do except
>+ * print the error code on a system failure.
>+ *
>  * @chip:		TPM chip to use.
>  * @shutdown_type:	shutdown type. The value is either
>  *			TPM_SU_CLEAR or TPM_SU_STATE.
>  */
> void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
> {
>-	struct tpm2_cmd cmd;
>+	struct tpm_buf buf;
> 	int rc;
>
>-	cmd.header.in = tpm2_shutdown_header;
>-	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
>-
>-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
>-			      "stopping the TPM");
>-
>-	/* In places where shutdown command is sent there's no much we can do
>-	 * except print the error code on a system failure.
>-	 */
>-	if (rc < 0 && rc != -EPIPE)
>-		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
>-			 rc);
>+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
>+	if (rc)
>+		return;
>+	tpm_buf_append_u16(&buf, shutdown_type);
>+	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
>+			 "stopping the TPM");
>+	tpm_buf_destroy(&buf);
> }
>
> /*
>-- 
>2.15.1
>

Re: [PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

On Fri, May 18, 2018 at 03:30:32PM -0700, Jerry Snitselaar wrote:
> On Mon Mar 26 18, Jarkko Sakkinen wrote:
> > In order to make struct tpm_buf the first class object for constructing TPM
> > commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> > entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> > prints an error message.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

Thanky you Jerry. I'll do one more round for this only to update kdoc.
It does not involve code changes so are you OK if you I keep your
reviewed-by despite this update?

/Jarkko

Re: [PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jerry Snitselaar]

On Wed May 23 18, Jarkko Sakkinen wrote:
>On Fri, May 18, 2018 at 03:30:32PM -0700, Jerry Snitselaar wrote:
>> On Mon Mar 26 18, Jarkko Sakkinen wrote:
>> > In order to make struct tpm_buf the first class object for constructing TPM
>> > commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
>> > entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
>> > prints an error message.
>> >
>> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>
>> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
>
>Thanky you Jerry. I'll do one more round for this only to update kdoc.
>It does not involve code changes so are you OK if you I keep your
>reviewed-by despite this update?
>
>/Jarkko

Sure

Re: [PATCH v4 1/4] tpm: migrate tpm2_shutdown() to use struct tpm_buf

[Jarkko Sakkinen]

On Sun, May 27, 2018 at 02:14:52AM -0700, Jerry Snitselaar wrote:
> On Wed May 23 18, Jarkko Sakkinen wrote:
> > On Fri, May 18, 2018 at 03:30:32PM -0700, Jerry Snitselaar wrote:
> > > On Mon Mar 26 18, Jarkko Sakkinen wrote:
> > > > In order to make struct tpm_buf the first class object for constructing TPM
> > > > commands, migrate tpm2_shutdown() to use it. In addition, removed the klog
> > > > entry when tpm_transmit_cmd() fails because tpm_tansmit_cmd() already
> > > > prints an error message.
> > > >
> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > 
> > > Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > 
> > Thanky you Jerry. I'll do one more round for this only to update kdoc.
> > It does not involve code changes so are you OK if you I keep your
> > reviewed-by despite this update?
> > 
> > /Jarkko
> 
> Sure

I pushed this commit to my master branch. The only changes are:

1. Update to kdoc.
2. Removed change to tpm2_flush_context_cmd() (should not be part
   of the commit).

 Did not make sense to cycle it through ML anymore.

/Jarkko

[PATCH v4 2/4] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_probe() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Acked-by: Jay Freyensee <why2jjj.linux@gmail.com>
---
 drivers/char/tpm/tpm2-cmd.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7665661d9230..7bffd0fd1dca 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -844,30 +844,35 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
 
 /**
  * tpm2_probe() - probe TPM 2.0
- * @chip: TPM chip to use
+ * @chip: a TPM chip to probe
  *
- * Return: < 0 error and 0 on success.
+ * Return: 0 on success,
+ *         -errno otherwise
  *
- * Send idempotent TPM 2.0 command and see whether TPM 2.0 chip replied based on
- * the reply tag.
+ * Send an idempotent TPM 2.0 command and see whether there is TPM2 chip in the
+ * other end based on the response tag. The flag TPM_CHIP_FLAG_TPM2 is set if
+ * this is the case.
  */
 int tpm2_probe(struct tpm_chip *chip)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_output_header *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
-	if (rc <  0)
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
 		return rc;
-
-	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
-		chip->flags |= TPM_CHIP_FLAG_TPM2;
-
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	/* We ignore TPM return codes on purpose. */
+	if (rc >=  0) {
+		out = (struct tpm_output_header *)buf.data;
+		if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
+			chip->flags |= TPM_CHIP_FLAG_TPM2;
+	}
+	tpm_buf_destroy(&buf);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(tpm2_probe);
-- 
2.15.1

Re: [PATCH v4 2/4] tpm: migrate tpm2_probe() to use struct tpm_buf

[Nayna Jain]

On 03/26/2018 05:44 PM, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing TPM
> commands, migrate tpm2_probe() to use it.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Acked-by: Jay Freyensee <why2jjj.linux@gmail.com>

Reviewed-by: Nayna Jain<nayna@linux.vnet.ibm.com>
Tested-by: Nayna Jain<nayna@linux.vnet.ibm.com>


Thanks & Regards,
     - Nayna

> ---
>   drivers/char/tpm/tpm2-cmd.c | 37 +++++++++++++++++++++----------------
>   1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 7665661d9230..7bffd0fd1dca 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -844,30 +844,35 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
>
>   /**
>    * tpm2_probe() - probe TPM 2.0
> - * @chip: TPM chip to use
> + * @chip: a TPM chip to probe
>    *
> - * Return: < 0 error and 0 on success.
> + * Return: 0 on success,
> + *         -errno otherwise
>    *
> - * Send idempotent TPM 2.0 command and see whether TPM 2.0 chip replied based on
> - * the reply tag.
> + * Send an idempotent TPM 2.0 command and see whether there is TPM2 chip in the
> + * other end based on the response tag. The flag TPM_CHIP_FLAG_TPM2 is set if
> + * this is the case.
>    */
>   int tpm2_probe(struct tpm_chip *chip)
>   {
> -	struct tpm2_cmd cmd;
> +	struct tpm_output_header *out;
> +	struct tpm_buf buf;
>   	int rc;
>
> -	cmd.header.in = tpm2_get_tpm_pt_header;
> -	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
> -	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
> -	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
> -
> -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
> -	if (rc <  0)
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
> +	if (rc)
>   		return rc;
> -
> -	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
> -		chip->flags |= TPM_CHIP_FLAG_TPM2;
> -
> +	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
> +	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
> +	tpm_buf_append_u32(&buf, 1);
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	/* We ignore TPM return codes on purpose. */
> +	if (rc >=  0) {
> +		out = (struct tpm_output_header *)buf.data;
> +		if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
> +			chip->flags |= TPM_CHIP_FLAG_TPM2;
> +	}
> +	tpm_buf_destroy(&buf);
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(tpm2_probe);

Re: [PATCH v4 2/4] tpm: migrate tpm2_probe() to use struct tpm_buf

[Jerry Snitselaar]

On Mon Mar 26 18, Jarkko Sakkinen wrote:
>In order to make struct tpm_buf the first class object for constructing TPM
>commands, migrate tpm2_probe() to use it.
>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>Acked-by: Jay Freyensee <why2jjj.linux@gmail.com>

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

>---
> drivers/char/tpm/tpm2-cmd.c | 37 +++++++++++++++++++++----------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>index 7665661d9230..7bffd0fd1dca 100644
>--- a/drivers/char/tpm/tpm2-cmd.c
>+++ b/drivers/char/tpm/tpm2-cmd.c
>@@ -844,30 +844,35 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
>
> /**
>  * tpm2_probe() - probe TPM 2.0
>- * @chip: TPM chip to use
>+ * @chip: a TPM chip to probe
>  *
>- * Return: < 0 error and 0 on success.
>+ * Return: 0 on success,
>+ *         -errno otherwise
>  *
>- * Send idempotent TPM 2.0 command and see whether TPM 2.0 chip replied based on
>- * the reply tag.
>+ * Send an idempotent TPM 2.0 command and see whether there is TPM2 chip in the
>+ * other end based on the response tag. The flag TPM_CHIP_FLAG_TPM2 is set if
>+ * this is the case.
>  */
> int tpm2_probe(struct tpm_chip *chip)
> {
>-	struct tpm2_cmd cmd;
>+	struct tpm_output_header *out;
>+	struct tpm_buf buf;
> 	int rc;
>
>-	cmd.header.in = tpm2_get_tpm_pt_header;
>-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
>-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
>-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
>-
>-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
>-	if (rc <  0)
>+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>+	if (rc)
> 		return rc;
>-
>-	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
>-		chip->flags |= TPM_CHIP_FLAG_TPM2;
>-
>+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
>+	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
>+	tpm_buf_append_u32(&buf, 1);
>+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
>+	/* We ignore TPM return codes on purpose. */
>+	if (rc >=  0) {
>+		out = (struct tpm_output_header *)buf.data;
>+		if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
>+			chip->flags |= TPM_CHIP_FLAG_TPM2;
>+	}
>+	tpm_buf_destroy(&buf);
> 	return 0;
> }
> EXPORT_SYMBOL_GPL(tpm2_probe);
>-- 
>2.15.1
>

[PATCH v4 3/4] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing TPM
commands, migrate tpm2_get_tpm_pt() to use it.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-cmd.c | 63 +++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7bffd0fd1dca..b3b52f9eb65f 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,20 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_tpm_pt_in {
-	__be32	cap_id;
-	__be32	property_id;
-	__be32	property_cnt;
-} __packed;
-
-struct tpm2_get_tpm_pt_out {
-	u8	more_data;
-	__be32	subcap_id;
-	__be32	property_cnt;
-	__be32	property_id;
-	__be32	value;
-} __packed;
-
 struct tpm2_get_random_in {
 	__be16	size;
 } __packed;
@@ -51,8 +37,6 @@ struct tpm2_get_random_out {
 } __packed;
 
 union tpm2_cmd_params {
-	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
-	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
 	struct	tpm2_get_random_out	getrandom_out;
 };
@@ -379,19 +363,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 	return total ? total : -EIO;
 }
 
-#define TPM2_GET_TPM_PT_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_tpm_pt_in))
-
-#define TPM2_GET_TPM_PT_OUT_BODY_SIZE \
-	 sizeof(struct tpm2_get_tpm_pt_out)
-
-static const struct tpm_input_header tpm2_get_tpm_pt_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
-};
-
 /**
  * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
  * @chip: TPM chip to use
@@ -725,6 +696,14 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	return rc;
 }
 
+struct tpm2_get_cap_out {
+	u8 more_data;
+	__be32 subcap_id;
+	__be32 property_cnt;
+	__be32 property_id;
+	__be32 value;
+} __packed;
+
 /**
  * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
  * @chip:		TPM chip to use.
@@ -737,19 +716,23 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 			const char *desc)
 {
-	struct tpm2_cmd cmd;
+	struct tpm2_get_cap_out *out;
+	struct tpm_buf buf;
 	int rc;
 
-	cmd.header.in = tpm2_get_tpm_pt_header;
-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
-			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
-	if (!rc)
-		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
-
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
+	tpm_buf_append_u32(&buf, property_id);
+	tpm_buf_append_u32(&buf, 1);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	if (!rc) {
+		out = (struct tpm2_get_cap_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		*value = be32_to_cpu(out->value);
+	}
+	tpm_buf_destroy(&buf);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
-- 
2.15.1

Re: [PATCH v4 3/4] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf

[Nayna Jain]

On 03/26/2018 05:44 PM, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing TPM
> commands, migrate tpm2_get_tpm_pt() to use it.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Nayna Jain<nayna@linux.vnet.ibm.com>
Tested-by: Nayna Jain<nayna@linux.vnet.ibm.com>


Thanks & Regards,
    - Nayna

> ---
>   drivers/char/tpm/tpm2-cmd.c | 63 +++++++++++++++++----------------------------
>   1 file changed, 23 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 7bffd0fd1dca..b3b52f9eb65f 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,20 +27,6 @@ enum tpm2_session_attributes {
>   	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>   };
>
> -struct tpm2_get_tpm_pt_in {
> -	__be32	cap_id;
> -	__be32	property_id;
> -	__be32	property_cnt;
> -} __packed;
> -
> -struct tpm2_get_tpm_pt_out {
> -	u8	more_data;
> -	__be32	subcap_id;
> -	__be32	property_cnt;
> -	__be32	property_id;
> -	__be32	value;
> -} __packed;
> -
>   struct tpm2_get_random_in {
>   	__be16	size;
>   } __packed;
> @@ -51,8 +37,6 @@ struct tpm2_get_random_out {
>   } __packed;
>
>   union tpm2_cmd_params {
> -	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
> -	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
>   	struct	tpm2_get_random_in	getrandom_in;
>   	struct	tpm2_get_random_out	getrandom_out;
>   };
> @@ -379,19 +363,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
>   	return total ? total : -EIO;
>   }
>
> -#define TPM2_GET_TPM_PT_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_get_tpm_pt_in))
> -
> -#define TPM2_GET_TPM_PT_OUT_BODY_SIZE \
> -	 sizeof(struct tpm2_get_tpm_pt_out)
> -
> -static const struct tpm_input_header tpm2_get_tpm_pt_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
> -};
> -
>   /**
>    * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
>    * @chip: TPM chip to use
> @@ -725,6 +696,14 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
>   	return rc;
>   }
>
> +struct tpm2_get_cap_out {
> +	u8 more_data;
> +	__be32 subcap_id;
> +	__be32 property_cnt;
> +	__be32 property_id;
> +	__be32 value;
> +} __packed;
> +
>   /**
>    * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
>    * @chip:		TPM chip to use.
> @@ -737,19 +716,23 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
>   ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>   			const char *desc)
>   {
> -	struct tpm2_cmd cmd;
> +	struct tpm2_get_cap_out *out;
> +	struct tpm_buf buf;
>   	int rc;
>
> -	cmd.header.in = tpm2_get_tpm_pt_header;
> -	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
> -	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
> -	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
> -
> -	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> -			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
> -	if (!rc)
> -		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
> -
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
> +	if (rc)
> +		return rc;
> +	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
> +	tpm_buf_append_u32(&buf, property_id);
> +	tpm_buf_append_u32(&buf, 1);
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	if (!rc) {
> +		out = (struct tpm2_get_cap_out *)
> +			&buf.data[TPM_HEADER_SIZE];
> +		*value = be32_to_cpu(out->value);
> +	}
> +	tpm_buf_destroy(&buf);
>   	return rc;
>   }
>   EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);

Re: [PATCH v4 3/4] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf

[Jarkko Sakkinen]

On Fri, May 18, 2018 at 12:01:46PM +0530, Nayna Jain wrote:
> 
> 
> On 03/26/2018 05:44 PM, Jarkko Sakkinen wrote:
> > In order to make struct tpm_buf the first class object for constructing TPM
> > commands, migrate tpm2_get_tpm_pt() to use it.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Reviewed-by: Nayna Jain<nayna@linux.vnet.ibm.com>
> Tested-by: Nayna Jain<nayna@linux.vnet.ibm.com>

For all the rest, thanks. I'll still do one more revision of these
patches to update the kdoc comments (but they do not involve any
code changes).

/Jarkko

Re: [PATCH v4 3/4] tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf

[Jerry Snitselaar]

On Mon Mar 26 18, Jarkko Sakkinen wrote:
>In order to make struct tpm_buf the first class object for constructing TPM
>commands, migrate tpm2_get_tpm_pt() to use it.
>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

>---
> drivers/char/tpm/tpm2-cmd.c | 63 +++++++++++++++++----------------------------
> 1 file changed, 23 insertions(+), 40 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>index 7bffd0fd1dca..b3b52f9eb65f 100644
>--- a/drivers/char/tpm/tpm2-cmd.c
>+++ b/drivers/char/tpm/tpm2-cmd.c
>@@ -27,20 +27,6 @@ enum tpm2_session_attributes {
> 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> };
>
>-struct tpm2_get_tpm_pt_in {
>-	__be32	cap_id;
>-	__be32	property_id;
>-	__be32	property_cnt;
>-} __packed;
>-
>-struct tpm2_get_tpm_pt_out {
>-	u8	more_data;
>-	__be32	subcap_id;
>-	__be32	property_cnt;
>-	__be32	property_id;
>-	__be32	value;
>-} __packed;
>-
> struct tpm2_get_random_in {
> 	__be16	size;
> } __packed;
>@@ -51,8 +37,6 @@ struct tpm2_get_random_out {
> } __packed;
>
> union tpm2_cmd_params {
>-	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
>-	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
> 	struct	tpm2_get_random_in	getrandom_in;
> 	struct	tpm2_get_random_out	getrandom_out;
> };
>@@ -379,19 +363,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> 	return total ? total : -EIO;
> }
>
>-#define TPM2_GET_TPM_PT_IN_SIZE \
>-	(sizeof(struct tpm_input_header) + \
>-	 sizeof(struct tpm2_get_tpm_pt_in))
>-
>-#define TPM2_GET_TPM_PT_OUT_BODY_SIZE \
>-	 sizeof(struct tpm2_get_tpm_pt_out)
>-
>-static const struct tpm_input_header tpm2_get_tpm_pt_header = {
>-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
>-	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
>-	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
>-};
>-
> /**
>  * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
>  * @chip: TPM chip to use
>@@ -725,6 +696,14 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
> 	return rc;
> }
>
>+struct tpm2_get_cap_out {
>+	u8 more_data;
>+	__be32 subcap_id;
>+	__be32 property_cnt;
>+	__be32 property_id;
>+	__be32 value;
>+} __packed;
>+
> /**
>  * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
>  * @chip:		TPM chip to use.
>@@ -737,19 +716,23 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
> ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
> 			const char *desc)
> {
>-	struct tpm2_cmd cmd;
>+	struct tpm2_get_cap_out *out;
>+	struct tpm_buf buf;
> 	int rc;
>
>-	cmd.header.in = tpm2_get_tpm_pt_header;
>-	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
>-	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
>-	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
>-
>-	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
>-			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
>-	if (!rc)
>-		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
>-
>+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>+	if (rc)
>+		return rc;
>+	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
>+	tpm_buf_append_u32(&buf, property_id);
>+	tpm_buf_append_u32(&buf, 1);
>+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
>+	if (!rc) {
>+		out = (struct tpm2_get_cap_out *)
>+			&buf.data[TPM_HEADER_SIZE];
>+		*value = be32_to_cpu(out->value);
>+	}
>+	tpm_buf_destroy(&buf);
> 	return rc;
> }
> EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
>-- 
>2.15.1
>

[PATCH v4 4/4] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

In order to make struct tpm_buf the first class object for constructing
TPM commands, migrate tpm2_get_random() to use it. In addition, removed
remaining references to struct tpm2_cmd. All of them use it to acquire
the length of the response, which can be achieved by using
tpm_buf_length().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h      | 19 ++++-----
 drivers/char/tpm/tpm2-cmd.c | 98 ++++++++++++++++++---------------------------
 2 files changed, 49 insertions(+), 68 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 7f2d0f489e9c..aa849a1b2641 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -421,23 +421,24 @@ struct tpm_buf {
 	u8 *data;
 };
 
-static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
 {
 	struct tpm_input_header *head;
+	head = (struct tpm_input_header *)buf->data;
+	head->tag = cpu_to_be16(tag);
+	head->length = cpu_to_be32(sizeof(*head));
+	head->ordinal = cpu_to_be32(ordinal);
+}
 
+static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
 	buf->data_page = alloc_page(GFP_HIGHUSER);
 	if (!buf->data_page)
 		return -ENOMEM;
 
 	buf->flags = 0;
 	buf->data = kmap(buf->data_page);
-
-	head = (struct tpm_input_header *) buf->data;
-
-	head->tag = cpu_to_be16(tag);
-	head->length = cpu_to_be32(sizeof(*head));
-	head->ordinal = cpu_to_be32(ordinal);
-
+	tpm_buf_reset(buf, tag, ordinal);
 	return 0;
 }
 
@@ -566,7 +567,7 @@ static inline u32 tpm2_rc_value(u32 rc)
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm2_digest *digests);
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			    unsigned int flags);
 int tpm2_seal_trusted(struct tpm_chip *chip,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index b3b52f9eb65f..d5c222f98515 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -27,25 +27,6 @@ enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
 };
 
-struct tpm2_get_random_in {
-	__be16	size;
-} __packed;
-
-struct tpm2_get_random_out {
-	__be16	size;
-	u8	buffer[TPM_MAX_RNG_DATA];
-} __packed;
-
-union tpm2_cmd_params {
-	struct	tpm2_get_random_in	getrandom_in;
-	struct	tpm2_get_random_out	getrandom_out;
-};
-
-struct tpm2_cmd {
-	tpm_cmd_header		header;
-	union tpm2_cmd_params	params;
-} __packed;
-
 struct tpm2_hash {
 	unsigned int crypto_id;
 	unsigned int tpm_id;
@@ -300,67 +281,70 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 }
 
 
-#define TPM2_GETRANDOM_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_random_in))
-
-static const struct tpm_input_header tpm2_getrandom_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
-};
+struct tpm2_get_random_out {
+	__be16 size;
+	u8 buffer[TPM_MAX_RNG_DATA];
+} __packed;
 
 /**
  * tpm2_get_random() - get random bytes from the TPM RNG
  *
  * @chip: TPM chip to use
- * @out: destination buffer for the random bytes
+ * @dest: destination buffer for the random bytes
  * @max: the max number of bytes to write to @out
  *
  * Return:
- *    Size of the output buffer, or -EIO on error.
+ * size of the output buffer when the operation is successful.
+ * A negative number for system errors (errno).
  */
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 {
-	struct tpm2_cmd cmd;
-	u32 recd, rlength;
-	u32 num_bytes;
+	struct tpm2_get_random_out *out;
+	struct tpm_buf buf;
+	u32 recd;
+	u32 num_bytes = max;
 	int err;
 	int total = 0;
 	int retries = 5;
-	u8 *dest = out;
-
-	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
+	u8 *dest_ptr = dest;
 
-	if (!out || !num_bytes ||
-	    max > sizeof(cmd.params.getrandom_out.buffer))
+	if (!num_bytes || max > TPM_MAX_RNG_DATA)
 		return -EINVAL;
 
-	do {
-		cmd.header.in = tpm2_getrandom_header;
-		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
+	err = tpm_buf_init(&buf, 0, 0);
+	if (err)
+		return err;
 
-		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
+	do {
+		tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
+		tpm_buf_append_u16(&buf, num_bytes);
+		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
 				       0, "attempting get random");
 		if (err)
-			break;
+			goto out;
 
-		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
-			     num_bytes);
-		rlength = be32_to_cpu(cmd.header.out.length);
-		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
-			      recd)
-			return -EFAULT;
-		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
+		out = (struct tpm2_get_random_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
+		if (tpm_buf_length(&buf) <
+		    offsetof(struct tpm2_get_random_out, buffer) + recd) {
+			err = -EFAULT;
+			goto out;
+		}
+		memcpy(dest_ptr, out->buffer, recd);
 
-		dest += recd;
+		dest_ptr += recd;
 		total += recd;
 		num_bytes -= recd;
 	} while (retries-- && total < max);
 
+	tpm_buf_destroy(&buf);
 	return total ? total : -EIO;
+out:
+	tpm_buf_destroy(&buf);
+	return err;
 }
 
 /**
@@ -434,7 +418,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
-	u32 hash, rlength;
+	u32 hash;
 	int i;
 	int rc;
 
@@ -509,8 +493,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		rc = -E2BIG;
 		goto out;
 	}
-	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
-	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
+	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
 		rc = -EFAULT;
 		goto out;
 	}
@@ -620,7 +603,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	u16 data_len;
 	u8 *data;
 	int rc;
-	u32 rlength;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
 	if (rc)
@@ -648,9 +630,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			goto out;
 		}
 
-		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
-					->header.out.length);
-		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
+		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
 			rc = -EFAULT;
 			goto out;
 		}
-- 
2.15.1

Re: [PATCH v4 4/4] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Nayna Jain]

On 03/26/2018 05:44 PM, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing
> TPM commands, migrate tpm2_get_random() to use it. In addition, removed
> remaining references to struct tpm2_cmd. All of them use it to acquire
> the length of the response, which can be achieved by using
> tpm_buf_length().
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm.h      | 19 ++++-----
>   drivers/char/tpm/tpm2-cmd.c | 98 ++++++++++++++++++---------------------------
>   2 files changed, 49 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 7f2d0f489e9c..aa849a1b2641 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -421,23 +421,24 @@ struct tpm_buf {
>   	u8 *data;
>   };
>
> -static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> +static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
>   {
>   	struct tpm_input_header *head;
> +	head = (struct tpm_input_header *)buf->data;
> +	head->tag = cpu_to_be16(tag);
> +	head->length = cpu_to_be32(sizeof(*head));
> +	head->ordinal = cpu_to_be32(ordinal);
> +}
>
> +static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> +{
>   	buf->data_page = alloc_page(GFP_HIGHUSER);
>   	if (!buf->data_page)
>   		return -ENOMEM;
>
>   	buf->flags = 0;
>   	buf->data = kmap(buf->data_page);
> -
> -	head = (struct tpm_input_header *) buf->data;
> -
> -	head->tag = cpu_to_be16(tag);
> -	head->length = cpu_to_be32(sizeof(*head));
> -	head->ordinal = cpu_to_be32(ordinal);
> -
> +	tpm_buf_reset(buf, tag, ordinal);
>   	return 0;
>   }
>
> @@ -566,7 +567,7 @@ static inline u32 tpm2_rc_value(u32 rc)
>   int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>   int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>   		    struct tpm2_digest *digests);
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>   void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>   			    unsigned int flags);
>   int tpm2_seal_trusted(struct tpm_chip *chip,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index b3b52f9eb65f..d5c222f98515 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -27,25 +27,6 @@ enum tpm2_session_attributes {
>   	TPM2_SA_CONTINUE_SESSION	= BIT(0),
>   };
>
> -struct tpm2_get_random_in {
> -	__be16	size;
> -} __packed;
> -
> -struct tpm2_get_random_out {
> -	__be16	size;
> -	u8	buffer[TPM_MAX_RNG_DATA];
> -} __packed;
> -
> -union tpm2_cmd_params {
> -	struct	tpm2_get_random_in	getrandom_in;
> -	struct	tpm2_get_random_out	getrandom_out;
> -};
> -
> -struct tpm2_cmd {
> -	tpm_cmd_header		header;
> -	union tpm2_cmd_params	params;
> -} __packed;
> -
>   struct tpm2_hash {
>   	unsigned int crypto_id;
>   	unsigned int tpm_id;
> @@ -300,67 +281,70 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>   }
>
>
> -#define TPM2_GETRANDOM_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_get_random_in))
> -
> -static const struct tpm_input_header tpm2_getrandom_header = {
> -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> -	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
> -};
> +struct tpm2_get_random_out {
> +	__be16 size;
> +	u8 buffer[TPM_MAX_RNG_DATA];
> +} __packed;
>
>   /**
>    * tpm2_get_random() - get random bytes from the TPM RNG
>    *
>    * @chip: TPM chip to use
> - * @out: destination buffer for the random bytes
> + * @dest: destination buffer for the random bytes
>    * @max: the max number of bytes to write to @out
>    *
>    * Return:
> - *    Size of the output buffer, or -EIO on error.
> + * size of the output buffer when the operation is successful.
> + * A negative number for system errors (errno).
>    */
> -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>   {
> -	struct tpm2_cmd cmd;
> -	u32 recd, rlength;
> -	u32 num_bytes;
> +	struct tpm2_get_random_out *out;
> +	struct tpm_buf buf;
> +	u32 recd;
> +	u32 num_bytes = max;
>   	int err;
>   	int total = 0;
>   	int retries = 5;
> -	u8 *dest = out;
> -
> -	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
> +	u8 *dest_ptr = dest;
>
> -	if (!out || !num_bytes ||
> -	    max > sizeof(cmd.params.getrandom_out.buffer))
> +	if (!num_bytes || max > TPM_MAX_RNG_DATA)
>   		return -EINVAL;
>
> -	do {
> -		cmd.header.in = tpm2_getrandom_header;
> -		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
> +	err = tpm_buf_init(&buf, 0, 0);
> +	if (err)
> +		return err;
>
> -		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> +	do {
> +		tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
> +		tpm_buf_append_u16(&buf, num_bytes);
> +		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
>   				       offsetof(struct tpm2_get_random_out,
>   						buffer),
>   				       0, "attempting get random");
>   		if (err)
> -			break;
> +			goto out;
>
> -		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
> -			     num_bytes);
> -		rlength = be32_to_cpu(cmd.header.out.length);
> -		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
> -			      recd)
> -			return -EFAULT;
> -		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
> +		out = (struct tpm2_get_random_out *)
> +			&buf.data[TPM_HEADER_SIZE];
> +		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
> +		if (tpm_buf_length(&buf) <
> +		    offsetof(struct tpm2_get_random_out, buffer) + recd) {
> +			err = -EFAULT;
> +			goto out;
> +		}
> +		memcpy(dest_ptr, out->buffer, recd);
>
> -		dest += recd;
> +		dest_ptr += recd;
>   		total += recd;
>   		num_bytes -= recd;
>   	} while (retries-- && total < max);
>
> +	tpm_buf_destroy(&buf);
>   	return total ? total : -EIO;
> +out:
> +	tpm_buf_destroy(&buf);
> +	return err;
>   }

How about having it as :

     if (!total)
         err = -EIO;
out:
     tpm_buf_destroy(&buf);
     return total?:err;

>   /**
> @@ -434,7 +418,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>   {
>   	unsigned int blob_len;
>   	struct tpm_buf buf;
> -	u32 hash, rlength;
> +	u32 hash;
>   	int i;
>   	int rc;
>
> @@ -509,8 +493,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>   		rc = -E2BIG;
>   		goto out;
>   	}
> -	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
> -	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
> +	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
>   		rc = -EFAULT;
>   		goto out;
>   	}
> @@ -620,7 +603,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>   	u16 data_len;
>   	u8 *data;
>   	int rc;
> -	u32 rlength;
>
>   	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
>   	if (rc)
> @@ -648,9 +630,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>   			goto out;
>   		}
>
> -		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
> -					->header.out.length);
> -		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
> +		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
>   			rc = -EFAULT;
>   			goto out;
>   		}
Probably, all the changes related to the use of tpm_buf_length() could 
be a separate patch in itself ?

Otherwise,

Tested-by: Nayna Jain<nayna@linux.vnet.ibm.com>

Thanks & Regards,
    - Nayna

Re: [PATCH v4 4/4] tpm: migrate tpm2_get_random() to use struct tpm_buf

[Jarkko Sakkinen]

On Fri, May 18, 2018 at 11:39:16AM +0530, Nayna Jain wrote:
> 
> 
> On 03/26/2018 05:44 PM, Jarkko Sakkinen wrote:
> > In order to make struct tpm_buf the first class object for constructing
> > TPM commands, migrate tpm2_get_random() to use it. In addition, removed
> > remaining references to struct tpm2_cmd. All of them use it to acquire
> > the length of the response, which can be achieved by using
> > tpm_buf_length().
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >   drivers/char/tpm/tpm.h      | 19 ++++-----
> >   drivers/char/tpm/tpm2-cmd.c | 98 ++++++++++++++++++---------------------------
> >   2 files changed, 49 insertions(+), 68 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 7f2d0f489e9c..aa849a1b2641 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -421,23 +421,24 @@ struct tpm_buf {
> >   	u8 *data;
> >   };
> > 
> > -static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> > +static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
> >   {
> >   	struct tpm_input_header *head;
> > +	head = (struct tpm_input_header *)buf->data;
> > +	head->tag = cpu_to_be16(tag);
> > +	head->length = cpu_to_be32(sizeof(*head));
> > +	head->ordinal = cpu_to_be32(ordinal);
> > +}
> > 
> > +static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> > +{
> >   	buf->data_page = alloc_page(GFP_HIGHUSER);
> >   	if (!buf->data_page)
> >   		return -ENOMEM;
> > 
> >   	buf->flags = 0;
> >   	buf->data = kmap(buf->data_page);
> > -
> > -	head = (struct tpm_input_header *) buf->data;
> > -
> > -	head->tag = cpu_to_be16(tag);
> > -	head->length = cpu_to_be32(sizeof(*head));
> > -	head->ordinal = cpu_to_be32(ordinal);
> > -
> > +	tpm_buf_reset(buf, tag, ordinal);
> >   	return 0;
> >   }
> > 
> > @@ -566,7 +567,7 @@ static inline u32 tpm2_rc_value(u32 rc)
> >   int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> >   int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> >   		    struct tpm2_digest *digests);
> > -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> > +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
> >   void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
> >   			    unsigned int flags);
> >   int tpm2_seal_trusted(struct tpm_chip *chip,
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index b3b52f9eb65f..d5c222f98515 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -27,25 +27,6 @@ enum tpm2_session_attributes {
> >   	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> >   };
> > 
> > -struct tpm2_get_random_in {
> > -	__be16	size;
> > -} __packed;
> > -
> > -struct tpm2_get_random_out {
> > -	__be16	size;
> > -	u8	buffer[TPM_MAX_RNG_DATA];
> > -} __packed;
> > -
> > -union tpm2_cmd_params {
> > -	struct	tpm2_get_random_in	getrandom_in;
> > -	struct	tpm2_get_random_out	getrandom_out;
> > -};
> > -
> > -struct tpm2_cmd {
> > -	tpm_cmd_header		header;
> > -	union tpm2_cmd_params	params;
> > -} __packed;
> > -
> >   struct tpm2_hash {
> >   	unsigned int crypto_id;
> >   	unsigned int tpm_id;
> > @@ -300,67 +281,70 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> >   }
> > 
> > 
> > -#define TPM2_GETRANDOM_IN_SIZE \
> > -	(sizeof(struct tpm_input_header) + \
> > -	 sizeof(struct tpm2_get_random_in))
> > -
> > -static const struct tpm_input_header tpm2_getrandom_header = {
> > -	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > -	.length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
> > -	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
> > -};
> > +struct tpm2_get_random_out {
> > +	__be16 size;
> > +	u8 buffer[TPM_MAX_RNG_DATA];
> > +} __packed;
> > 
> >   /**
> >    * tpm2_get_random() - get random bytes from the TPM RNG
> >    *
> >    * @chip: TPM chip to use
> > - * @out: destination buffer for the random bytes
> > + * @dest: destination buffer for the random bytes
> >    * @max: the max number of bytes to write to @out
> >    *
> >    * Return:
> > - *    Size of the output buffer, or -EIO on error.
> > + * size of the output buffer when the operation is successful.
> > + * A negative number for system errors (errno).
> >    */
> > -int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> > +int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> >   {
> > -	struct tpm2_cmd cmd;
> > -	u32 recd, rlength;
> > -	u32 num_bytes;
> > +	struct tpm2_get_random_out *out;
> > +	struct tpm_buf buf;
> > +	u32 recd;
> > +	u32 num_bytes = max;
> >   	int err;
> >   	int total = 0;
> >   	int retries = 5;
> > -	u8 *dest = out;
> > -
> > -	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
> > +	u8 *dest_ptr = dest;
> > 
> > -	if (!out || !num_bytes ||
> > -	    max > sizeof(cmd.params.getrandom_out.buffer))
> > +	if (!num_bytes || max > TPM_MAX_RNG_DATA)
> >   		return -EINVAL;
> > 
> > -	do {
> > -		cmd.header.in = tpm2_getrandom_header;
> > -		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
> > +	err = tpm_buf_init(&buf, 0, 0);
> > +	if (err)
> > +		return err;
> > 
> > -		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> > +	do {
> > +		tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
> > +		tpm_buf_append_u16(&buf, num_bytes);
> > +		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> >   				       offsetof(struct tpm2_get_random_out,
> >   						buffer),
> >   				       0, "attempting get random");
> >   		if (err)
> > -			break;
> > +			goto out;
> > 
> > -		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
> > -			     num_bytes);
> > -		rlength = be32_to_cpu(cmd.header.out.length);
> > -		if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
> > -			      recd)
> > -			return -EFAULT;
> > -		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
> > +		out = (struct tpm2_get_random_out *)
> > +			&buf.data[TPM_HEADER_SIZE];
> > +		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
> > +		if (tpm_buf_length(&buf) <
> > +		    offsetof(struct tpm2_get_random_out, buffer) + recd) {
> > +			err = -EFAULT;
> > +			goto out;
> > +		}
> > +		memcpy(dest_ptr, out->buffer, recd);
> > 
> > -		dest += recd;
> > +		dest_ptr += recd;
> >   		total += recd;
> >   		num_bytes -= recd;
> >   	} while (retries-- && total < max);
> > 
> > +	tpm_buf_destroy(&buf);
> >   	return total ? total : -EIO;
> > +out:
> > +	tpm_buf_destroy(&buf);
> > +	return err;
> >   }
> 
> How about having it as :
> 
>     if (!total)
>         err = -EIO;
> out:
>     tpm_buf_destroy(&buf);
>     return total?:err;

I think the former is actually more readable in this case albeit there
is one 

> 
> >   /**
> > @@ -434,7 +418,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> >   {
> >   	unsigned int blob_len;
> >   	struct tpm_buf buf;
> > -	u32 hash, rlength;
> > +	u32 hash;
> >   	int i;
> >   	int rc;
> > 
> > @@ -509,8 +493,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> >   		rc = -E2BIG;
> >   		goto out;
> >   	}
> > -	rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
> > -	if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
> > +	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
> >   		rc = -EFAULT;
> >   		goto out;
> >   	}
> > @@ -620,7 +603,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> >   	u16 data_len;
> >   	u8 *data;
> >   	int rc;
> > -	u32 rlength;
> > 
> >   	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> >   	if (rc)
> > @@ -648,9 +630,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> >   			goto out;
> >   		}
> > 
> > -		rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
> > -					->header.out.length);
> > -		if (rlength < TPM_HEADER_SIZE + 6 + data_len) {
> > +		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
> >   			rc = -EFAULT;
> >   			goto out;
> >   		}
> Probably, all the changes related to the use of tpm_buf_length() could be a
> separate patch in itself ?

I think so but have to recheck with time.

I'll drop these now  from master (really not something that needs to be
rushed) and also update the comments according to standard that I
described to you in my own review comments (since they are not in that
standard).

> Otherwise,
> 
> Tested-by: Nayna Jain<nayna@linux.vnet.ibm.com>

Thank you.

/Jarkko

Re: [PATCH v4 0/4] Migrate all TPM 2.0 commands to use struct tpm_buf

[Jarkko Sakkinen]

On Mon, Mar 26, 2018 at 03:14:02PM +0300, Jarkko Sakkinen wrote:
> In order to make struct tpm_buf the first class object for constructing TPM
> commands, this patch set migrates all TPM 2.0 commands to use it. Eventually,
> tpm_transmit_cmd() can take simply struct tpm_buf as its argument and this
> interface can be exported to be used by the kernel keyring and potentially
> other subsystems.
> 
> The ultimate goal of this work is to make constructing TPM commands inside
> the kernel simple and robust.

I pushed these commits to the master branch. Please report if you have
any issues. If the master branch continues working for you, as you test
it maybe for other reasons, I'm happy to get tested-by's for them. At
worst they have regressions. I seriously don't think that the code
changes have any major structural issues.

I would guess that Tomas' similar changes for TPM 1.x will follow at
some point. I'm looking forward to change the existing tpm_send()
as one that takes tpm_buf in. That will allow to remove a lot of
cruft code from keyring.

I take no rush to merge these to 'next' but I think it is fine to
have these in the bleeding edge.

/Jarkko