diff mbox

[tpmdd-devel,v2] tpm: introduce struct tpm_buf

Message ID 1433349555-30868-1-git-send-email-jarkko.sakkinen@linux.intel.com
State Superseded
Headers show

Commit Message

Jarkko Sakkinen June 3, 2015, 4:39 p.m. UTC 
This patch introduces struct tpm_buf that provides a string buffer for
constructing TPM commands. This allows to construct variable sized TPM
commands. This feature is needed for TPM 2.0 commands in order to allow
policy authentication and algorithmic agility.

The commands in the tpm2-cmd.c have been updated to use struct tpm_buf.
Lots of awkward length calculations could be dropped because the buffer
knows its length.

The code is is along the lines of the string buffer code in
security/trusted/trusted.h.

Changes since v1:

- Fixed potential alignment issues in tpm_buf_tag().

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

Comments

Peter Hüwe June 8, 2015, 8:39 p.m. UTC | #1 
Hi,


Am Mittwoch, 3. Juni 2015, 18:39:14 schrieb Jarkko Sakkinen:
> This patch introduces struct tpm_buf that provides a string buffer for
> constructing TPM commands. This allows to construct variable sized TPM
> commands. This feature is needed for TPM 2.0 commands in order to allow
> policy authentication and algorithmic agility.
> 
> The commands in the tpm2-cmd.c have been updated to use struct tpm_buf.
> Lots of awkward length calculations could be dropped because the buffer
> knows its length.
> 
> The code is is along the lines of the string buffer code in
> security/trusted/trusted.h.
> 
> Changes since v1:
> 
> - Fixed potential alignment issues in tpm_buf_tag().
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

> +static inline void tpm_buf_append(struct tpm_buf *buf,
> +				  const unsigned char *data,
> +				  unsigned int len)
> +{
> +	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> +
> +	BUG_ON((len + tpm_buf_length(buf)) > TPM_BUF_SIZE);
> +
> +	memcpy(&buf->data[tpm_buf_length(buf)], data, len);
> +	head->length = cpu_to_be32(tpm_buf_length(buf) + len);
> +}
> +
> +static inline void tpm_buf_store(struct tpm_buf *buf,
> +				 unsigned int pos,
> +				 const unsigned char *data,
> +				 unsigned int len)
> +{
> +	BUG_ON((pos + len) > TPM_BUF_SIZE);
> +
> +	memcpy(&buf->data[pos], data, len);
> +}

Don't you have to update the ->length here?


Thanks,
Peter


------------------------------------------------------------------------------
Jarkko Sakkinen June 9, 2015, 9:19 a.m. UTC | #2 
On Mon, Jun 08, 2015 at 10:39:32PM +0200, Peter Hüwe wrote:
> Hi,
> 
> 
> Am Mittwoch, 3. Juni 2015, 18:39:14 schrieb Jarkko Sakkinen:
> > This patch introduces struct tpm_buf that provides a string buffer for
> > constructing TPM commands. This allows to construct variable sized TPM
> > commands. This feature is needed for TPM 2.0 commands in order to allow
> > policy authentication and algorithmic agility.
> > 
> > The commands in the tpm2-cmd.c have been updated to use struct tpm_buf.
> > Lots of awkward length calculations could be dropped because the buffer
> > knows its length.
> > 
> > The code is is along the lines of the string buffer code in
> > security/trusted/trusted.h.
> > 
> > Changes since v1:
> > 
> > - Fixed potential alignment issues in tpm_buf_tag().
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> > +static inline void tpm_buf_append(struct tpm_buf *buf,
> > +				  const unsigned char *data,
> > +				  unsigned int len)
> > +{
> > +	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> > +
> > +	BUG_ON((len + tpm_buf_length(buf)) > TPM_BUF_SIZE);
> > +
> > +	memcpy(&buf->data[tpm_buf_length(buf)], data, len);
> > +	head->length = cpu_to_be32(tpm_buf_length(buf) + len);
> > +}
> > +
> > +static inline void tpm_buf_store(struct tpm_buf *buf,
> > +				 unsigned int pos,
> > +				 const unsigned char *data,
> > +				 unsigned int len)
> > +{
> > +	BUG_ON((pos + len) > TPM_BUF_SIZE);
> > +
> > +	memcpy(&buf->data[pos], data, len);
> > +}
> 
> Don't you have to update the ->length here?

No. Store is for placing value in position, not appending to the end.

> Thanks,
> Peter

/Jarkko

------------------------------------------------------------------------------
Peter Hüwe June 9, 2015, 10:32 a.m. UTC | #3 
Hi

>> > +static inline void tpm_buf_store(struct tpm_buf *buf,
>> > +				 unsigned int pos,
>> > +				 const unsigned char *data,
>> > +				 unsigned int len)
>> > +{
>> > +	BUG_ON((pos + len) > TPM_BUF_SIZE);
>> > +
>> > +	memcpy(&buf->data[pos], data, len);
>> > +}
>> 
>> Don't you have to update the ->length here?
>
>No. Store is for placing value in position, not appending to the end.
>
Then either add a length check (whether ->length is big enough) and/or call the function "update"

Peter


------------------------------------------------------------------------------
Jarkko Sakkinen June 9, 2015, 11:39 a.m. UTC | #4 
On Tue, Jun 09, 2015 at 12:32:57PM +0200, Peter Huewe wrote:
> 
> 
> Hi
> 
> >> > +static inline void tpm_buf_store(struct tpm_buf *buf,
> >> > +				 unsigned int pos,
> >> > +				 const unsigned char *data,
> >> > +				 unsigned int len)
> >> > +{
> >> > +	BUG_ON((pos + len) > TPM_BUF_SIZE);
> >> > +
> >> > +	memcpy(&buf->data[pos], data, len);
> >> > +}
> >> 
> >> Don't you have to update the ->length here?
> >
> >No. Store is for placing value in position, not appending to the end.
> >
> Then either add a length check (whether ->length is big enough) and/or
> call the function "update"

There is a length check in the beginning (first line of the function
body).

> Peter

/Jarkko

------------------------------------------------------------------------------
Peter Hüwe June 9, 2015, 11:58 a.m. UTC | #5 
Am 9. Juni 2015 13:39:13 MESZ, schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
>On Tue, Jun 09, 2015 at 12:32:57PM +0200, Peter Huewe wrote:
>> 
>> 
>> Hi
>> 
>> >> > +static inline void tpm_buf_store(struct tpm_buf *buf,
>> >> > +				 unsigned int pos,
>> >> > +				 const unsigned char *data,
>> >> > +				 unsigned int len)
>> >> > +{
>> >> > +	BUG_ON((pos + len) > TPM_BUF_SIZE);
>> >> > +
>> >> > +	memcpy(&buf->data[pos], data, len);
>> >> > +}
>> >> 
>> >> Don't you have to update the ->length here?
>> >
>> >No. Store is for placing value in position, not appending to the
>end.
>> >
>> Then either add a length check (whether ->length is big enough)
>and/or
>> call the function "update"
>
>There is a length check in the beginning (first line of the function
>body).
>
Nope.
The check in the first line checks whether the write is <= the max buffer size, 
but not <= head->length.

Since head->length is not updated (as per design) it is possible to write data without effect using this function. 
This is not what I expect from an API.


Example I create a buffer using tpm_buf_append with 12 bytes, so head->length == 12
Then I use tpm_buf_store at pos 10 and len 4 --> in the buffer are 14 bytes, but tpm_buf_length will only report 12 bytes.

Which is not what I would expect and your current check dies not prevent this.

Peter
Jarkko Sakkinen June 9, 2015, 12:05 p.m. UTC | #6 
On Tue, Jun 09, 2015 at 01:58:59PM +0200, Peter Huewe wrote:
> 
> 
> Am 9. Juni 2015 13:39:13 MESZ, schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
> >On Tue, Jun 09, 2015 at 12:32:57PM +0200, Peter Huewe wrote:
> >> 
> >> 
> >> Hi
> >> 
> >> >> > +static inline void tpm_buf_store(struct tpm_buf *buf,
> >> >> > +				 unsigned int pos,
> >> >> > +				 const unsigned char *data,
> >> >> > +				 unsigned int len)
> >> >> > +{
> >> >> > +	BUG_ON((pos + len) > TPM_BUF_SIZE);
> >> >> > +
> >> >> > +	memcpy(&buf->data[pos], data, len);
> >> >> > +}
> >> >> 
> >> >> Don't you have to update the ->length here?
> >> >
> >> >No. Store is for placing value in position, not appending to the
> >end.
> >> >
> >> Then either add a length check (whether ->length is big enough)
> >and/or
> >> call the function "update"
> >
> >There is a length check in the beginning (first line of the function
> >body).
> >
> Nope.
> The check in the first line checks whether the write is <= the max
> buffer size, but not <= head->length.
> 
> Since head->length is not updated (as per design) it is possible to
> write data without effect using this function.  This is not what I
> expect from an API.
> 
> 
> Example I create a buffer using tpm_buf_append with 12 bytes, so
> head->length == 12 Then I use tpm_buf_store at pos 10 and len 4 --> in
> the buffer are 14 bytes, but tpm_buf_length will only report 12 bytes.
> 
> Which is not what I would expect and your current check dies not prevent this.

Aah. Right. A valid point. I'll add this check to the invariant.

/Jarkko

------------------------------------------------------------------------------
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f8319a0..205e7c2 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -382,6 +382,93 @@  struct tpm_cmd_t {
 	tpm_cmd_params	params;
 } __packed;
 
+/* A string buffer type for constructing TPM commands. This is based on the
+ * code in security/keys/trusted.h.
+ */
+
+#define TPM_BUF_SIZE 512
+
+struct tpm_buf {
+	u8 data[TPM_BUF_SIZE];
+} __attribute__((aligned(8)));
+
+static inline void tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	struct tpm_input_header *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 u32 tpm_buf_length(struct tpm_buf *buf)
+{
+	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+
+	return be32_to_cpu(head->length);
+}
+
+static inline u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+
+	return be16_to_cpu(head->tag);
+}
+
+static inline u8 *tpm_buf_out(struct tpm_buf *buf)
+{
+	return &buf->data[TPM_HEADER_SIZE];
+}
+
+static inline void tpm_buf_append(struct tpm_buf *buf,
+				  const unsigned char *data,
+				  unsigned int len)
+{
+	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+
+	BUG_ON((len + tpm_buf_length(buf)) > TPM_BUF_SIZE);
+
+	memcpy(&buf->data[tpm_buf_length(buf)], data, len);
+	head->length = cpu_to_be32(tpm_buf_length(buf) + len);
+}
+
+static inline void tpm_buf_store(struct tpm_buf *buf,
+				 unsigned int pos,
+				 const unsigned char *data,
+				 unsigned int len)
+{
+	BUG_ON((pos + len) > TPM_BUF_SIZE);
+
+	memcpy(&buf->data[pos], data, len);
+}
+
+static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+	tpm_buf_append(buf, &value, 1);
+}
+
+static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
+{
+	__be16 value2 = cpu_to_be16(value);
+
+	tpm_buf_append(buf, (u8 *) &value2, 2);
+}
+
+static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
+{
+	__be32 value2 = cpu_to_be32(value);
+
+	tpm_buf_append(buf, (u8 *) &value2, 4);
+}
+
+static inline void tpm_buf_store_u32(struct tpm_buf *buf, unsigned int pos,
+				     const u32 value)
+{
+	__be32 value2 = cpu_to_be32(value);
+
+	tpm_buf_store(buf, pos, (u8 *) &value2, 4);
+}
+
 extern struct class *tpm_class;
 extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 011909a..a274cef 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -17,14 +17,6 @@ 
 
 #include "tpm.h"
 
-struct tpm2_startup_in {
-	__be16	startup_type;
-} __packed;
-
-struct tpm2_self_test_in {
-	u8	full_test;
-} __packed;
-
 struct tpm2_pcr_read_in {
 	__be32	pcr_selects_cnt;
 	__be16	hash_alg;
@@ -43,17 +35,7 @@  struct tpm2_pcr_read_out {
 	u8	digest[TPM_DIGEST_SIZE];
 } __packed;
 
-struct tpm2_null_auth_area {
-	__be32			handle;
-	__be16			nonce_size;
-	u8			attributes;
-	__be16			auth_size;
-} __packed;
-
 struct tpm2_pcr_extend_in {
-	__be32				pcr_idx;
-	__be32				auth_area_size;
-	struct tpm2_null_auth_area	auth_area;
 	__be32				digest_cnt;
 	__be16				hash_alg;
 	u8				digest[TPM_DIGEST_SIZE];
@@ -73,32 +55,11 @@  struct tpm2_get_tpm_pt_out {
 	__be32	value;
 } __packed;
 
-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_startup_in		startup_in;
-	struct	tpm2_self_test_in	selftest_in;
-	struct	tpm2_pcr_read_in	pcrread_in;
-	struct	tpm2_pcr_read_out	pcrread_out;
-	struct	tpm2_pcr_extend_in	pcrextend_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;
-	struct	tpm2_get_random_out	getrandom_out;
-};
-
-struct tpm2_cmd {
-	tpm_cmd_header		header;
-	union tpm2_cmd_params	params;
-} __packed;
-
 /*
  * Array with one entry per ordinal defining the maximum amount
  * of time the chip could take to return the result. The values
@@ -221,15 +182,28 @@  static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
 	TPM_UNDEFINED		/* 18f */
 };
 
-#define TPM2_PCR_READ_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_pcr_read_in))
+static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
+				 const u8 *nonce, u16 nonce_len,
+				 u8 session_attributes,
+				 const u8 *hmac, u16 hmac_len)
+{
+	u32 buf_len = tpm_buf_length(buf);
+
+	tpm_buf_append_u32(buf, 0);
+	tpm_buf_append_u32(buf, session_handle);
+	tpm_buf_append_u16(buf, nonce_len);
 
-static const struct tpm_input_header tpm2_pcrread_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
-};
+	if (nonce)
+		tpm_buf_append(buf, nonce, nonce_len);
+
+	tpm_buf_append_u8(buf, session_attributes);
+	tpm_buf_append_u16(buf, hmac_len);
+
+	if (hmac)
+		tpm_buf_append(buf, hmac, hmac_len);
+
+	tpm_buf_store_u32(buf, buf_len, tpm_buf_length(buf) - buf_len);
+}
 
 /**
  * tpm2_pcr_read() - read a PCR value
@@ -243,42 +217,36 @@  static const struct tpm_input_header tpm2_pcrread_header = {
  */
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 {
+	struct tpm_buf buf;
+	struct tpm2_pcr_read_in in;
+	struct tpm2_pcr_read_out *out;
 	int rc;
-	struct tpm2_cmd cmd;
-	u8 *buf;
 
 	if (pcr_idx >= TPM2_PLATFORM_PCR)
 		return -EINVAL;
 
-	cmd.header.in = tpm2_pcrread_header;
-	cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
-	cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
-	cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
+	tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
 
-	memset(cmd.params.pcrread_in.pcr_select, 0,
-	       sizeof(cmd.params.pcrread_in.pcr_select));
-	cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
+	in.pcr_selects_cnt = cpu_to_be32(1);
+	in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+	in.pcr_select_size = TPM2_PCR_SELECT_MIN;
+	in.pcr_select[0] = 0x00;
+	in.pcr_select[1] = 0x00;
+	in.pcr_select[2] = 0x00;
+	in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+	tpm_buf_append(&buf, (u8 *) &in, sizeof(in));
+
+	rc = tpm_transmit_cmd(chip, buf.data, TPM_BUF_SIZE,
 			      "attempting to read a pcr value");
-	if (rc == 0) {
-		buf = cmd.params.pcrread_out.digest;
-		memcpy(res_buf, buf, TPM_DIGEST_SIZE);
-	}
+	if (rc)
+		return rc;
 
-	return rc;
+	out = (struct tpm2_pcr_read_out *) &buf.data[TPM_HEADER_SIZE];
+	memcpy(res_buf, out->digest, TPM_DIGEST_SIZE);
+	return 0;
 }
 
-#define TPM2_GET_PCREXTEND_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_pcr_extend_in))
-
-static const struct tpm_input_header tpm2_pcrextend_header = {
-	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
-	.length = cpu_to_be32(TPM2_GET_PCREXTEND_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
-};
-
 /**
  * tpm2_pcr_extend() - extend a PCR value
  * @chip:	TPM chip to use.
@@ -291,76 +259,63 @@  static const struct tpm_input_header tpm2_pcrextend_header = {
  */
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
 {
-	struct tpm2_cmd cmd;
-	int rc;
-
-	cmd.header.in = tpm2_pcrextend_header;
-	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
-	cmd.params.pcrextend_in.auth_area_size =
-		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
-	cmd.params.pcrextend_in.auth_area.handle =
-		cpu_to_be32(TPM2_RS_PW);
-	cmd.params.pcrextend_in.auth_area.nonce_size = 0;
-	cmd.params.pcrextend_in.auth_area.attributes = 0;
-	cmd.params.pcrextend_in.auth_area.auth_size = 0;
-	cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1);
-	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
-	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
-
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
-			      "attempting extend a PCR value");
+	struct tpm_buf buf;
+	struct tpm2_pcr_extend_in in;
 
-	return rc;
-}
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+	tpm_buf_append_u32(&buf, pcr_idx);
+	tpm2_buf_append_auth(&buf, TPM2_RS_PW, NULL /* nonce */, 0,
+			     0 /* session_attributes */, NULL /* hmac */, 0);
 
-#define TPM2_GETRANDOM_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_get_random_in))
+	in.digest_cnt = cpu_to_be32(1);
+	in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+	memcpy(in.digest, hash, TPM_DIGEST_SIZE);
+	tpm_buf_append(&buf, (u8 *) &in, sizeof(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)
-};
+	return tpm_transmit_cmd(chip, buf.data, TPM_BUF_SIZE,
+				"attempting extend a PCR value");
+}
 
 /**
  * tpm2_get_random() - get random bytes from the TPM RNG
  * @chip: TPM chip to use
- * @out: destination buffer for the random bytes
+ * @res_buf: destination buffer for the random bytes
  * @max: the max number of bytes to write to @out
  *
  * 0 is returned when the operation is successful. If a negative number is
  * returned it remarks a POSIX error code. If a positive number is returned
  * it remarks a TPM error.
  */
-int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+int tpm2_get_random(struct tpm_chip *chip, u8 *res_buf, size_t max)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
+	struct tpm2_get_random_out *out;
 	u32 recd;
 	u32 num_bytes;
 	int err;
 	int total = 0;
 	int retries = 5;
-	u8 *dest = out;
+	u8 *dest = res_buf;
 
-	num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
+	num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
 
-	if (!out || !num_bytes ||
-	    max > sizeof(cmd.params.getrandom_out.buffer))
+	if (!res_buf || !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_transmit_cmd(chip, &cmd, sizeof(cmd),
+		tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+		tpm_buf_append_u16(&buf, num_bytes);
+
+		err = tpm_transmit_cmd(chip, buf.data, TPM_BUF_SIZE,
 				       "attempting get random");
 		if (err)
 			break;
 
-		recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
-			     num_bytes);
-		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
+		out = (struct tpm2_get_random_out *) tpm_buf_out(&buf);
+
+		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
+		memcpy(dest, out->buffer, recd);
 
 		dest += recd;
 		total += recd;
@@ -370,16 +325,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))
-
-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_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
  * @chip:		TPM chip to use.
@@ -391,33 +336,31 @@  static const struct tpm_input_header tpm2_get_tpm_pt_header = {
  * returned it remarks a POSIX error code. If a positive number is returned
  * it remarks a TPM error.
  */
-ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
+ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
 			const char *desc)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
+	struct tpm2_get_tpm_pt_in in;
+	struct tpm2_get_tpm_pt_out *out;
 	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);
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_CAPABILITY);
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), desc);
-	if (!rc)
-		*value = cmd.params.get_tpm_pt_out.value;
+	in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
+	in.property_id = cpu_to_be32(property_id);
+	in.property_cnt = cpu_to_be32(1);
 
-	return rc;
-}
+	tpm_buf_append(&buf, (u8 *) &in, sizeof(in));
 
-#define TPM2_STARTUP_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_startup_in))
+	rc = tpm_transmit_cmd(chip, buf.data, TPM_BUF_SIZE, desc);
+	if (!rc) {
+		out = (struct tpm2_get_tpm_pt_out *)
+			&buf.data[TPM_HEADER_SIZE];
+		*value = be32_to_cpu(out->value);
+	}
 
-static const struct tpm_input_header tpm2_startup_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_STARTUP_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_STARTUP)
-};
+	return rc;
+}
 
 /**
  * tpm2_startup() - send startup command to the TPM chip
@@ -431,26 +374,18 @@  static const struct tpm_input_header tpm2_startup_header = {
  */
 int tpm2_startup(struct tpm_chip *chip, u16 startup_type)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
+	int rc;
 
-	cmd.header.in = tpm2_startup_header;
+	tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP);
+	tpm_buf_append_u16(&buf, startup_type);
+	rc = tpm_transmit_cmd(chip, buf.data, TPM_BUF_SIZE,
+			      "attempting to start the TPM");
 
-	cmd.params.startup_in.startup_type = cpu_to_be16(startup_type);
-	return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
-				"attempting to start the TPM");
+	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm2_startup);
 
-#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
  * @chip:		TPM chip to use.
@@ -459,20 +394,11 @@  static const struct tpm_input_header tpm2_shutdown_header = {
  */
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 {
-	struct tpm2_cmd cmd;
-	int rc;
-
-	cmd.header.in = tpm2_shutdown_header;
-	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
+	struct tpm_buf buf;
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), "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)
-		dev_warn(chip->pdev, "transmit returned %d while stopping the TPM",
-			 rc);
+	tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+	tpm_buf_append_u16(&buf, shutdown_type);
+	tpm_transmit_cmd(chip, buf.data, TPM_BUF_SIZE, "stopping the TPM");
 }
 EXPORT_SYMBOL_GPL(tpm2_shutdown);
 
@@ -503,16 +429,6 @@  unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm2_calc_ordinal_duration);
 
-#define TPM2_SELF_TEST_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_self_test_in))
-
-static const struct tpm_input_header tpm2_selftest_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_SELF_TEST_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_SELF_TEST)
-};
-
 /**
  * tpm2_continue_selftest() - start a self test
  * @chip: TPM chip to use
@@ -525,13 +441,13 @@  static const struct tpm_input_header tpm2_selftest_header = {
  */
 static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
 {
+	struct tpm_buf buf;
 	int rc;
-	struct tpm2_cmd cmd;
 
-	cmd.header.in = tpm2_selftest_header;
-	cmd.params.selftest_in.full_test = full;
+	tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+	tpm_buf_append_u8(&buf, full);
 
-	rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE,
+	rc = tpm_transmit_cmd(chip, buf.data, TPM_BUF_SIZE,
 			      "continue selftest");
 
 	/* At least some prototype chips seem to give RC_TESTING error
@@ -562,11 +478,10 @@  int tpm2_do_selftest(struct tpm_chip *chip)
 	unsigned int loops;
 	unsigned int delay_msec = 100;
 	unsigned long duration;
-	struct tpm2_cmd cmd;
+	u8 dig[TPM_DIGEST_SIZE];
 	int i;
 
 	duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
-
 	loops = jiffies_to_msecs(duration) / delay_msec;
 
 	rc = tpm2_start_selftest(chip, true);
@@ -574,21 +489,8 @@  int tpm2_do_selftest(struct tpm_chip *chip)
 		return rc;
 
 	for (i = 0; i < loops; i++) {
-		/* Attempt to read a PCR value */
-		cmd.header.in = tpm2_pcrread_header;
-		cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
-		cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
-		cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
-		cmd.params.pcrread_in.pcr_select[0] = 0x01;
-		cmd.params.pcrread_in.pcr_select[1] = 0x00;
-		cmd.params.pcrread_in.pcr_select[2] = 0x00;
-
-		rc = tpm_transmit_cmd(chip, (u8 *) &cmd, sizeof(cmd), NULL);
-		if (rc < 0)
-			break;
-
-		rc = be32_to_cpu(cmd.header.out.return_code);
-		if (rc != TPM2_RC_TESTING)
+		rc = tpm2_pcr_read(chip, 1, dig);
+		if (rc < 0 || rc != TPM2_RC_TESTING)
 			break;
 
 		msleep(delay_msec);
@@ -624,21 +526,24 @@  EXPORT_SYMBOL_GPL(tpm2_gen_interrupt);
  */
 int tpm2_probe(struct tpm_chip *chip)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
+	struct tpm2_get_tpm_pt_in in;
 	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);
+	in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
+	in.property_id = cpu_to_be32(0x100);
+	in.property_cnt = cpu_to_be32(1);
+
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	tpm_buf_append(&buf, (u8 *) &in, sizeof(in));
 
-	rc = tpm_transmit(chip, (const char *) &cmd, sizeof(cmd));
+	rc = tpm_transmit(chip, buf.data, TPM_BUF_SIZE);
 	if (rc <  0)
 		return rc;
 	else if (rc < TPM_HEADER_SIZE)
 		return -EFAULT;
 
-	if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
+	if (tpm_buf_tag(&buf) == TPM2_ST_NO_SESSIONS)
 		chip->flags |= TPM_CHIP_FLAG_TPM2;
 
 	return 0;