[PATCH v2 00/11] add integrity and security to TPM2 transactions

[PATCH v2 00/11] add integrity and security to TPM2 transactions

[James Bottomley]

The interest in securing the TPM against interposers, both active and
passive has risen to fever pitch with the demonstration of key
recovery against windows bitlocker:

https://dolosgroup.io/blog/2021/7/9/from-stolen-laptop-to-inside-the-company-network

And subsequently the same attack being successful against all the
Linux TPM based security solutions:

https://www.secura.com/blog/tpm-sniffing-attacks-against-non-bitlocker-targets

The attacks fall into two categories:

1. Passive Interposers, which sit on the bus and merely observe
2. Active Interposers, which try to manipulate TPM transactions on the
   bus using man in the middle and packet stealing to create TPM state
   the interposer owner desires.

Our broadest interposer target is the use of TPM_RS_PW for password
authorization which sends the actual password to the TPM without any
obfuscation and effectively hands it to any interposer. The way to fix
this is to use real sessions for HMAC capabilities to ensure integrity
and to use parameter and response encryption to ensure confidentiality
of the data flowing over the TPM bus.  HMAC sessions by agreeing a
challenge with the TPM and then giving a response which is a HMAC of
the password and the challenge, so the application proves knowledge of
the password to the TPM without ever transmitting the password itself.
Using HMAC sessions when sending commands to the TPM also provides
some measure of protection against active interposers, since the
interposer can't interfere with or delete a HMAC'd command (because
they can't manufacture a response with the correct HMAC).

To protect TPM transactions where there isn't a shared secret
(i.e. the command is something like a PCR extension which doesn't
involve a TPM object with a password) we have to do a bit more work to
set up sessions with a passed in encrypted secret (called a salt) to
act in place of the shared secret in the HMAC.  This secret salt is
effectively a random number encrypted to a public key of the TPM.  The
final piece of the puzzle is using parameter input and response return
encryption, so any interposer can't see the data passing from the
application to the TPM and vice versa.

The most insidious interposer attack of all is a reset attack: since
the interposer has access to the TPM bus, it can assert the TPM reset
line any time it wants.  When a TPM resets it mostly comes back in the
same state except that all the PCRs are reset to their initial values.
Controlling the reset line allows the interposer to change the PCR
state after the fact by resetting the TPM and then replaying PCR
extends to get the PCRs into a valid state to release secrets, so even
if an attack event was recorded, the record is erased.  This reset
attack violates the fundamental princible of non-repudiability of TPM
logs.  Defeating the reset attack involves tying all TPM operations
within the kernel to a property which will change detectably if the
TPM is reset.  For that reason, we tie all TPM sessions to the null
hierarchy we obtain at start of day and whose seed changes on every
reset.  If an active interposer asserts a TPM reset, the new null
primary won't match the kernel's stored one and all TPM operations
will start failing because of HMAC mismatches in the sessions.  So if
the kernel TPM code keeps operating, it guarantees that a reset hasn't
occurred.

The final part of the puzzle is that the machine owner must have a
fixed idea of the EK of their TPM and should have certified this with
the TPM manufacturer.  On every boot, the certified EK public key
should be used to do a make credential/activate credential attestation
key insertion and then the null key certified with the attestation
key.  We can follow a trust on first use model where an OS
installation will extract and verify a public EK and save it to a read
only file.

This patch series adds a simple API which can ensure the above
properties as a layered addition to the existing TPM handling code.
This series now includes protections for PCR extend, getting random
numbers from the TPM and data sealing and unsealing.  It therefore
eliminates all uses of TPM2_RS_PW in the kernel and adds encryption
protection to sensitive data flowing into and out of the TPM.  The
first four patches add more sophisticated buffer handling to the TPM
which is needed to build the more complex encryption and
authentication based commands.  Patch 6 adds all the generic
cryptography primitives and patches 7-9 use them in critical TPM
operations where we want to avoid or detect interposers.  Patch 10
exports the name of the null key we used for boot/run time
verification and patch 11 documents the security guarantees and
expectations.

This was originally sent over four years ago, with the last iteration
being:

https://lore.kernel.org/linux-integrity/1568031515.6613.31.camel@HansenPartnership.com/

I'm dusting it off now because various forces at Microsoft and Google
via the Open Compute Platform are making a lot of noise about
interposers and we in the linux kernel look critically lacking in that
regard, particularly for TPM trusted keys.

---
v2 fixes the problems smatch reported and adds more explanation about
the code motion in the first few patches

James

---

James Bottomley (11):
  tpm: move buffer handling from static inlines to real functions
  tpm: add buffer handling for TPM2B types
  tpm: add cursor based buffer functions for response parsing
  tpm: add buffer function to point to returned parameters
  tpm: export the context save and load commands
  tpm: Add full HMAC and encrypt/decrypt session handling code
  tpm: add hmac checks to tpm2_pcr_extend()
  tpm: add session encryption protection to tpm2_get_random()
  KEYS: trusted: Add session encryption protection to the seal/unseal
    path
  tpm: add the null key name as a sysfs export
  Documentation: add tpm-security.rst

 Documentation/security/tpm/tpm-security.rst |  214 ++++
 drivers/char/tpm/Kconfig                    |   13 +
 drivers/char/tpm/Makefile                   |    2 +
 drivers/char/tpm/tpm-buf.c                  |  197 +++
 drivers/char/tpm/tpm-sysfs.c                |   14 +
 drivers/char/tpm/tpm.h                      |   14 +
 drivers/char/tpm/tpm2-cmd.c                 |   54 +-
 drivers/char/tpm/tpm2-sessions.c            | 1216 +++++++++++++++++++
 drivers/char/tpm/tpm2-space.c               |    8 +-
 include/linux/tpm.h                         |  252 ++--
 security/keys/trusted-keys/trusted_tpm2.c   |   85 +-
 11 files changed, 1944 insertions(+), 125 deletions(-)
 create mode 100644 Documentation/security/tpm/tpm-security.rst
 create mode 100644 drivers/char/tpm/tpm-buf.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.c

-- 
2.35.3

[PATCH v2 01/11] tpm: move buffer handling from static inlines to real functions

[James Bottomley]

separate out the tpm_buf_... handling functions from static inlines in
tpm.h and move them to their own tpm-buf.c file.  This is a precursor
to adding new functions for other TPM type handling becuase the amount
of code will grow from the current 70 lines in tpm.h to about 200
lines when the additions are done.  200 lines of inline functions is a
bit too much to keep in a header file.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/Makefile  |  1 +
 drivers/char/tpm/tpm-buf.c | 96 ++++++++++++++++++++++++++++++++++++++
 include/linux/tpm.h        | 86 ++++------------------------------
 3 files changed, 106 insertions(+), 77 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 0222b1ddb310..ad3594e383e1 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -15,6 +15,7 @@ tpm-y += tpm-sysfs.o
 tpm-y += eventlog/common.o
 tpm-y += eventlog/tpm1.o
 tpm-y += eventlog/tpm2.o
+tpm-y += tpm-buf.o
 
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
new file mode 100644
index 000000000000..4e6e8b281f6a
--- /dev/null
+++ b/drivers/char/tpm/tpm-buf.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Handing for tpm_buf structures to facilitate the building of commands
+ */
+
+#include <linux/module.h>
+#include <linux/tpm.h>
+
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	buf->data = (u8 *)__get_free_page(GFP_KERNEL);
+	if (!buf->data)
+		return -ENOMEM;
+
+	buf->flags = 0;
+	tpm_buf_reset(buf, tag, ordinal);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
+
+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	struct tpm_header *head = (struct tpm_header *) buf->data;
+
+	head->tag = cpu_to_be16(tag);
+	head->length = cpu_to_be32(sizeof(*head));
+	head->ordinal = cpu_to_be32(ordinal);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_reset);
+
+void tpm_buf_destroy(struct tpm_buf *buf)
+{
+	free_page((unsigned long)buf->data);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_destroy);
+
+u32 tpm_buf_length(struct tpm_buf *buf)
+{
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+
+	return be32_to_cpu(head->length);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_length);
+
+u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+
+	return be16_to_cpu(head->tag);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_tag);
+
+void tpm_buf_append(struct tpm_buf *buf,
+		    const unsigned char *new_data,
+		    unsigned int new_len)
+{
+	struct tpm_header *head = (struct tpm_header *) buf->data;
+	u32 len = tpm_buf_length(buf);
+
+	/* Return silently if overflow has already happened. */
+	if (buf->flags & TPM_BUF_OVERFLOW)
+		return;
+
+	if ((len + new_len) > PAGE_SIZE) {
+		WARN(1, "tpm_buf: overflow\n");
+		buf->flags |= TPM_BUF_OVERFLOW;
+		return;
+	}
+
+	memcpy(&buf->data[len], new_data, new_len);
+	head->length = cpu_to_be32(len + new_len);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append);
+
+void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+	tpm_buf_append(buf, &value, 1);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u8);
+
+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);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u16);
+
+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);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index dfeb25a0362d..150b39b6190e 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -322,84 +322,16 @@ struct tpm2_hash {
 	unsigned int tpm_id;
 };
 
-static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
-	struct tpm_header *head = (struct tpm_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 = (u8 *)__get_free_page(GFP_KERNEL);
-	if (!buf->data)
-		return -ENOMEM;
-
-	buf->flags = 0;
-	tpm_buf_reset(buf, tag, ordinal);
-	return 0;
-}
-
-static inline void tpm_buf_destroy(struct tpm_buf *buf)
-{
-	free_page((unsigned long)buf->data);
-}
-
-static inline u32 tpm_buf_length(struct tpm_buf *buf)
-{
-	struct tpm_header *head = (struct tpm_header *)buf->data;
-
-	return be32_to_cpu(head->length);
-}
-
-static inline u16 tpm_buf_tag(struct tpm_buf *buf)
-{
-	struct tpm_header *head = (struct tpm_header *)buf->data;
-
-	return be16_to_cpu(head->tag);
-}
-
-static inline void tpm_buf_append(struct tpm_buf *buf,
-				  const unsigned char *new_data,
-				  unsigned int new_len)
-{
-	struct tpm_header *head = (struct tpm_header *)buf->data;
-	u32 len = tpm_buf_length(buf);
-
-	/* Return silently if overflow has already happened. */
-	if (buf->flags & TPM_BUF_OVERFLOW)
-		return;
-
-	if ((len + new_len) > PAGE_SIZE) {
-		WARN(1, "tpm_buf: overflow\n");
-		buf->flags |= TPM_BUF_OVERFLOW;
-		return;
-	}
 
-	memcpy(&buf->data[len], new_data, new_len);
-	head->length = cpu_to_be32(len + new_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);
-}
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal);
+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal);
+void tpm_buf_destroy(struct tpm_buf *buf);
+u32 tpm_buf_length(struct tpm_buf *buf);
+void tpm_buf_append(struct tpm_buf *buf, const unsigned char *new_data,
+		    unsigned int new_len);
+void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value);
+void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value);
+void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value);
 
 /*
  * Check if TPM device is in the firmware upgrade mode.
-- 
2.35.3

[PATCH v2 02/11] tpm: add buffer handling for TPM2B types

[James Bottomley]

Most complex TPM commands require appending TPM2B buffers to the
command body.  Since TPM2B types are essentially variable size arrays,
it makes it impossible to represent these complex command arguments as
structures and we simply have to build them up using append primitives
like these.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm-buf.c | 71 +++++++++++++++++++++++++++++++++++---
 include/linux/tpm.h        |  3 ++
 2 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index 4e6e8b281f6a..f1d48f22d4b4 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -7,17 +7,16 @@
 #include <linux/module.h>
 #include <linux/tpm.h>
 
-int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static int __tpm_buf_init(struct tpm_buf *buf)
 {
 	buf->data = (u8 *)__get_free_page(GFP_KERNEL);
 	if (!buf->data)
 		return -ENOMEM;
 
 	buf->flags = 0;
-	tpm_buf_reset(buf, tag, ordinal);
+
 	return 0;
 }
-EXPORT_SYMBOL_GPL(tpm_buf_init);
 
 void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
 {
@@ -29,17 +28,60 @@ void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm_buf_reset);
 
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	int rc;
+
+	rc = __tpm_buf_init(buf);
+	if (rc)
+		return rc;
+
+	tpm_buf_reset(buf, tag, ordinal);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
+
+int tpm_buf_init_2b(struct tpm_buf *buf)
+{
+	struct tpm_header *head;
+	int rc;
+
+	rc = __tpm_buf_init(buf);
+	if (rc)
+		return rc;
+
+	head = (struct tpm_header *) buf->data;
+
+	head->length = cpu_to_be32(sizeof(*head));
+
+	buf->flags = TPM_BUF_2B;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init_2b);
+
 void tpm_buf_destroy(struct tpm_buf *buf)
 {
 	free_page((unsigned long)buf->data);
 }
 EXPORT_SYMBOL_GPL(tpm_buf_destroy);
 
+static void *tpm_buf_data(struct tpm_buf *buf)
+{
+	if (buf->flags & TPM_BUF_2B)
+		return buf->data + TPM_HEADER_SIZE;
+	return buf->data;
+}
+
 u32 tpm_buf_length(struct tpm_buf *buf)
 {
 	struct tpm_header *head = (struct tpm_header *)buf->data;
+	u32 len;
 
-	return be32_to_cpu(head->length);
+	len = be32_to_cpu(head->length);
+	if (buf->flags & TPM_BUF_2B)
+		len -= sizeof(*head);
+	return len;
 }
 EXPORT_SYMBOL_GPL(tpm_buf_length);
 
@@ -56,7 +98,7 @@ void tpm_buf_append(struct tpm_buf *buf,
 		    unsigned int new_len)
 {
 	struct tpm_header *head = (struct tpm_header *) buf->data;
-	u32 len = tpm_buf_length(buf);
+	u32 len = be32_to_cpu(head->length);
 
 	/* Return silently if overflow has already happened. */
 	if (buf->flags & TPM_BUF_OVERFLOW)
@@ -94,3 +136,22 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 	tpm_buf_append(buf, (u8 *) &value2, 4);
 }
 EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
+
+static void tpm_buf_reset_int(struct tpm_buf *buf)
+{
+	struct tpm_header *head;
+
+	head = (struct tpm_header *)buf->data;
+	head->length = cpu_to_be32(sizeof(*head));
+}
+
+void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b)
+{
+	u16 len = tpm_buf_length(tpm2b);
+
+	tpm_buf_append_u16(buf, len);
+	tpm_buf_append(buf, tpm_buf_data(tpm2b), len);
+	/* clear the buf for reuse */
+	tpm_buf_reset_int(tpm2b);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_2b);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 150b39b6190e..f2d4dab6d832 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -300,6 +300,7 @@ struct tpm_header {
 
 enum tpm_buf_flags {
 	TPM_BUF_OVERFLOW	= BIT(0),
+	TPM_BUF_2B		= BIT(1),
 };
 
 struct tpm_buf {
@@ -324,6 +325,7 @@ struct tpm2_hash {
 
 
 int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal);
+int tpm_buf_init_2b(struct tpm_buf *buf);
 void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal);
 void tpm_buf_destroy(struct tpm_buf *buf);
 u32 tpm_buf_length(struct tpm_buf *buf);
@@ -332,6 +334,7 @@ void tpm_buf_append(struct tpm_buf *buf, const unsigned char *new_data,
 void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value);
 void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value);
 void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value);
+void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b);
 
 /*
  * Check if TPM device is in the firmware upgrade mode.
-- 
2.35.3

[PATCH v2 03/11] tpm: add cursor based buffer functions for response parsing

[James Bottomley]

Once we have encryption and authentication, marshalling and
unmarshalling sessions becomes hugely complex.  Add cursor based
functions which update the current pointer to make response parsing
easier.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm-buf.c | 29 +++++++++++++++++++++++++++++
 include/linux/tpm.h        |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index f1d48f22d4b4..046b00bf7a81 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -7,6 +7,8 @@
 #include <linux/module.h>
 #include <linux/tpm.h>
 
+#include <asm/unaligned.h>
+
 static int __tpm_buf_init(struct tpm_buf *buf)
 {
 	buf->data = (u8 *)__get_free_page(GFP_KERNEL);
@@ -155,3 +157,30 @@ void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b)
 	tpm_buf_reset_int(tpm2b);
 }
 EXPORT_SYMBOL_GPL(tpm_buf_append_2b);
+
+/* functions for unmarshalling data and moving the cursor */
+u8 tpm_get_inc_u8(const u8 **ptr)
+{
+	return *((*ptr)++);
+}
+EXPORT_SYMBOL_GPL(tpm_get_inc_u8);
+
+u16 tpm_get_inc_u16(const u8 **ptr)
+{
+	u16 val;
+
+	val = get_unaligned_be16(*ptr);
+	*ptr += sizeof(val);
+	return val;
+}
+EXPORT_SYMBOL_GPL(tpm_get_inc_u16);
+
+u32 tpm_get_inc_u32(const u8 **ptr)
+{
+	u32 val;
+
+	val = get_unaligned_be32(*ptr);
+	*ptr += sizeof(val);
+	return val;
+}
+EXPORT_SYMBOL_GPL(tpm_get_inc_u32);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index f2d4dab6d832..f7cff1d114b0 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -335,6 +335,9 @@ void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value);
 void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value);
 void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value);
 void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b);
+u8 tpm_get_inc_u8(const u8 **ptr);
+u16 tpm_get_inc_u16(const u8 **ptr);
+u32 tpm_get_inc_u32(const u8 **ptr);
 
 /*
  * Check if TPM device is in the firmware upgrade mode.
-- 
2.35.3

[PATCH v2 04/11] tpm: add buffer function to point to returned parameters

[James Bottomley]

Introducing encryption sessions changes where the return parameters
are located in the buffer because if a return session is present
they're 4 bytes beyond the header with those 4 bytes showing the
parameter length.  If there is no return session, then they're in the
usual place immediately after the header.  The tpm_buf_parameters()
encapsulates this calculation and should be used everywhere
&buf.data[TPM_HEADER_SIZE] is used now

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm-buf.c | 10 ++++++++++
 include/linux/tpm.h        |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index 046b00bf7a81..b054aa2fc84b 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -184,3 +184,13 @@ u32 tpm_get_inc_u32(const u8 **ptr)
 	return val;
 }
 EXPORT_SYMBOL_GPL(tpm_get_inc_u32);
+
+u8 *tpm_buf_parameters(struct tpm_buf *buf)
+{
+	int offset = TPM_HEADER_SIZE;
+
+	if (tpm_buf_tag(buf) == TPM2_ST_SESSIONS)
+		offset += 4;
+
+	return &buf->data[offset];
+}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index f7cff1d114b0..fa8d1f932c0f 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -339,6 +339,8 @@ u8 tpm_get_inc_u8(const u8 **ptr);
 u16 tpm_get_inc_u16(const u8 **ptr);
 u32 tpm_get_inc_u32(const u8 **ptr);
 
+u8 *tpm_buf_parameters(struct tpm_buf *buf);
+
 /*
  * Check if TPM device is in the firmware upgrade mode.
  */
-- 
2.35.3

[PATCH v2 05/11] tpm: export the context save and load commands

[James Bottomley]

The TPM2 session HMAC and encryption handling code needs to save and
restore a single volatile context for the elliptic curve version of
the NULL seed, so export the APIs which do this for internal use.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm.h        | 4 ++++
 drivers/char/tpm/tpm2-space.c | 8 ++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 24ee4e1cc452..a5fe37977103 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -237,6 +237,10 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, void *buf,
 		      size_t *bufsiz);
 int tpm_devs_add(struct tpm_chip *chip);
 void tpm_devs_remove(struct tpm_chip *chip);
+int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
+		      unsigned int buf_size, unsigned int *offset);
+int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
+		      unsigned int *offset, u32 *handle);
 
 void tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index ffb35f0154c1..d77ee4af9d65 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -68,8 +68,8 @@ void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
 	kfree(space->session_buf);
 }
 
-static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
-			     unsigned int *offset, u32 *handle)
+int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
+		      unsigned int *offset, u32 *handle)
 {
 	struct tpm_buf tbuf;
 	struct tpm2_context *ctx;
@@ -119,8 +119,8 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
 	return 0;
 }
 
-static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
-			     unsigned int buf_size, unsigned int *offset)
+int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
+		      unsigned int buf_size, unsigned int *offset)
 {
 	struct tpm_buf tbuf;
 	unsigned int body_size;
-- 
2.35.3

[PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[James Bottomley]

Add true session based HMAC authentication plus parameter decryption
and response encryption using AES. The basic design is to segregate
all the nasty crypto, hash and hmac code into tpm2-sessions.c and
export a usable API.  The API first of all starts off by gaining a
session with

tpm2_start_auth_session()

Which initiates a session with the TPM and allocates an opaque
tpm2_auth structure to handle the session parameters.  Then the use is
simply:

* tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
  handles

* tpm_buf_append_hmac_session() where tpm2_append_auth() would go

* tpm_buf_fill_hmac_session() called after the entire command buffer
  is finished but before tpm_transmit_cmd() is called which computes
  the correct HMAC and places it in the command at the correct
  location.

Finally, after tpm_transmit_cmd() is called,
tpm_buf_check_hmac_response() is called to check that the returned
HMAC matched and collect the new state for the next use of the
session, if any.

The features of the session is controlled by the session attributes
set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
not specified, the session will be flushed and the tpm2_auth structure
freed in tpm_buf_check_hmac_response(); otherwise the session may be
used again.  Parameter encryption is specified by or'ing the flag
TPM2_SA_DECRYPT and response encryption by or'ing the flag
TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
respectively.

To get all of this to work securely, the Kernel needs a primary key to
encrypt the session salt to, so an EC key from the NULL seed is
derived and its context saved in the tpm_chip structure.  The context
is loaded on demand into an available volatile handle when
tpm_start_auth_session() is called, but is flushed before that
function exits to conserve handles.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # crypto API parts

---

v2: fix memory leaks from smatch; adjust for name hash size
---
 drivers/char/tpm/Kconfig         |   13 +
 drivers/char/tpm/Makefile        |    1 +
 drivers/char/tpm/tpm-buf.c       |    1 +
 drivers/char/tpm/tpm.h           |   10 +
 drivers/char/tpm/tpm2-cmd.c      |    5 +
 drivers/char/tpm/tpm2-sessions.c | 1216 ++++++++++++++++++++++++++++++
 include/linux/tpm.h              |  160 ++++
 7 files changed, 1406 insertions(+)
 create mode 100644 drivers/char/tpm/tpm2-sessions.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 927088b2c3d3..74bd174f1a7b 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -9,6 +9,9 @@ menuconfig TCG_TPM
 	imply SECURITYFS
 	select CRYPTO
 	select CRYPTO_HASH_INFO
+	select CRYPTO_ECDH
+	select CRYPTO_AES
+	select CRYPTO_CFB
 	help
 	  If you have a TPM security chip in your system, which
 	  implements the Trusted Computing Group's specification,
@@ -27,6 +30,16 @@ menuconfig TCG_TPM
 
 if TCG_TPM
 
+config TPM_BUS_SECURITY
+       bool "Use secure transactions on the TPM bus"
+       default y
+       help
+         Setting this causes us to deploy a tamper resistent scheme
+	 for communicating with the TPM to prevent or detect bus
+	 snooping and iterposer attacks like TPM Genie.  Saying Y here
+	 adds some encryption overhead to all kernel to TPM
+	 transactions.
+
 config HW_RANDOM_TPM
 	bool "TPM HW Random Number Generator support"
 	depends on TCG_TPM && HW_RANDOM && !(TCG_TPM=y && HW_RANDOM=m)
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index ad3594e383e1..10dc214aa093 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -17,6 +17,7 @@ tpm-y += eventlog/tpm1.o
 tpm-y += eventlog/tpm2.o
 tpm-y += tpm-buf.o
 
+tpm-$(CONFIG_TPM_BUS_SECURITY) += tpm2-sessions.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index b054aa2fc84b..fe3320731217 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -27,6 +27,7 @@ void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
 	head->tag = cpu_to_be16(tag);
 	head->length = cpu_to_be32(sizeof(*head));
 	head->ordinal = cpu_to_be32(ordinal);
+	buf->handles = 0;
 }
 EXPORT_SYMBOL_GPL(tpm_buf_reset);
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a5fe37977103..b3eb0f31bfd9 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -246,4 +246,14 @@ void tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
 int tpm_dev_common_init(void);
 void tpm_dev_common_exit(void);
+
+#ifdef CONFIG_TPM_BUS_SECURITY
+int tpm2_sessions_init(struct tpm_chip *chip);
+#else
+static inline int tpm2_sessions_init(struct tpm_chip *chip)
+{
+	return 0;
+}
+#endif
+
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 65d03867e114..056dad3dd5c9 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -759,6 +759,11 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		rc = 0;
 	}
 
+	if (rc)
+		goto out;
+
+	rc = tpm2_sessions_init(chip);
+
 out:
 	/*
 	 * Infineon TPM in field upgrade mode will return no data for the number
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
new file mode 100644
index 000000000000..23a4aa8874ab
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -0,0 +1,1216 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2018 James.Bottomley@HansenPartnership.com
+ *
+ * Cryptographic helper routines for handling TPM2 sessions for
+ * authorization HMAC and request response encryption.
+ *
+ * The idea is to ensure that every TPM command is HMAC protected by a
+ * session, meaning in-flight tampering would be detected and in
+ * addition all sensitive inputs and responses should be encrypted.
+ *
+ * The basic way this works is to use a TPM feature called salted
+ * sessions where a random secret used in session construction is
+ * encrypted to the public part of a known TPM key.  The problem is we
+ * have no known keys, so initially a primary Elliptic Curve key is
+ * derived from the NULL seed (we use EC because most TPMs generate
+ * these keys much faster than RSA ones).  The curve used is NIST_P256
+ * because that's now mandated to be present in 'TCG TPM v2.0
+ * Provisioning Guidance'
+ *
+ * Threat problems: the initial TPM2_CreatePrimary is not (and cannot
+ * be) session protected, so a clever Man in the Middle could return a
+ * public key they control to this command and from there intercept
+ * and decode all subsequent session based transactions.  The kernel
+ * cannot mitigate this threat but, after boot, userspace can get
+ * proof this has not happened by asking the TPM to certify the NULL
+ * key.  This certification would chain back to the TPM Endorsement
+ * Certificate and prove the NULL seed primary had not been tampered
+ * with and thus all sessions must have been cryptographically secure.
+ * To assist with this, the initial NULL seed public key name is made
+ * available in a sysfs file.
+ *
+ * Use of these functions:
+ *
+ * The design is all the crypto, hash and hmac gunk is confined in this
+ * file and never needs to be seen even by the kernel internal user.  To
+ * the user there's an init function tpm2_sessions_init() that needs to
+ * be called once per TPM which generates the NULL seed primary key.
+ *
+ * Then there are six usage functions:
+ *
+ * tpm2_start_auth_session() which allocates the opaque auth structure
+ *	and gets a session from the TPM.  This must be called before
+ *	any of the following functions.  The session is protected by a
+ *	session_key which is derived from a random salt value
+ *	encrypted to the NULL seed.
+ * tpm2_end_auth_session() kills the session and frees the resources.
+ *	Under normal operation this function is done by
+ *	tpm_buf_check_hmac_response(), so this is only to be used on
+ *	error legs where the latter is not executed.
+ * tpm_buf_append_name() to add a handle to the buffer.  This must be
+ *	used in place of the usual tpm_buf_append_u32() for adding
+ *	handles because handles have to be processed specially when
+ *	calculating the HMAC.  In particular, for NV, volatile and
+ *	permanent objects you now need to provide the name.
+ * tpm_buf_append_hmac_session() which appends the hmac session to the
+ *	buf in the same way tpm_buf_append_auth does().
+ * tpm_buf_fill_hmac_session() This calculates the correct hash and
+ *	places it in the buffer.  It must be called after the complete
+ *	command buffer is finalized so it can fill in the correct HMAC
+ *	based on the parameters.
+ * tpm_buf_check_hmac_response() which checks the session response in
+ *	the buffer and calculates what it should be.  If there's a
+ *	mismatch it will log a warning and return an error.  If
+ *	tpm_buf_append_hmac_session() did not specify
+ *	TPM_SA_CONTINUE_SESSION then the session will be closed (if it
+ *	hasn't been consumed) and the auth structure freed.
+ */
+
+#include "tpm.h"
+
+#include <linux/random.h>
+#include <linux/scatterlist.h>
+
+#include <asm/unaligned.h>
+
+#include <crypto/aes.h>
+#include <crypto/kpp.h>
+#include <crypto/ecdh.h>
+#include <crypto/hash.h>
+#include <crypto/hmac.h>
+#include <crypto/skcipher.h>
+
+/* if you change to AES256, you only need change this */
+#define AES_KEYBYTES	AES_KEYSIZE_128
+
+#define AES_KEYBITS	(AES_KEYBYTES*8)
+#define AUTH_MAX_NAMES	3
+
+/*
+ * This is the structure that carries all the auth information (like
+ * session handle, nonces, session key and auth) from use to use it is
+ * designed to be opaque to anything outside.
+ */
+struct tpm2_auth {
+	u32 handle;
+	/*
+	 * This has two meanings: before tpm_buf_fill_hmac_session()
+	 * it marks the offset in the buffer of the start of the
+	 * sessions (i.e. after all the handles).  Once the buffer has
+	 * been filled it markes the session number of our auth
+	 * session so we can find it again in the response buffer.
+	 *
+	 * The two cases are distinguished because the first offset
+	 * must always be greater than TPM_HEADER_SIZE and the second
+	 * must be less than or equal to 5.
+	 */
+	u32 session;
+	/*
+	 * the size here is variable and set by the size of our_nonce
+	 * which must be between 16 and the name hash length. we set
+	 * the maximum sha256 size for the greatest protection
+	 */
+	u8 our_nonce[SHA256_DIGEST_SIZE];
+	u8 tpm_nonce[SHA256_DIGEST_SIZE];
+	/*
+	 * the salt is only used across the session command/response
+	 * after that it can be used as a scratch area
+	 */
+	union {
+		u8 salt[EC_PT_SZ];
+		/* scratch for key + IV */
+		u8 scratch[AES_KEYBYTES + AES_BLOCK_SIZE];
+	};
+	/*
+	 * the session key and passphrase are the same size as the
+	 * name digest (sha256 again).  The session key is constant
+	 * for the use of the session and the passphrase can change
+	 * with every invocation.
+	 *
+	 * Note: these fields must be adjacent and in this order
+	 * because several HMAC/KDF schemes use the combination of the
+	 * session_key and passphrase.
+	 */
+	u8 session_key[SHA256_DIGEST_SIZE];
+	u8 passphrase[SHA256_DIGEST_SIZE];
+	int passphraselen;
+	/* saved session attributes */
+	u8 attrs;
+	__be32 ordinal;
+	struct crypto_sync_skcipher *aes;
+	struct tpm_chip *chip;
+	/* 3 names of handles: name_h is handle, name is name of handle */
+	u32 name_h[AUTH_MAX_NAMES];
+	u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
+};
+
+/*
+ * this is our static crypto shash.  This is possible because the hash
+ * is multi-threaded and all the state stored in the desc
+ */
+static struct crypto_shash *sha256_hash;
+
+/*
+ * Name Size based on TPM algorithm (assumes no hash bigger than 255)
+ */
+static u8 name_size(const u8 *name)
+{
+	static u8 size_map[] = {
+		[TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
+		[TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
+		[TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
+		[TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
+	};
+	u16 alg = get_unaligned_be16(name);
+	return size_map[alg] + 2;
+}
+
+/*
+ * It turns out the crypto hmac(sha256) is hard for us to consume
+ * because it assumes a fixed key and the TPM seems to change the key
+ * on every operation, so we weld the hmac init and final functions in
+ * here to give it the same usage characteristics as a regular hash
+ */
+static void hmac_init(struct shash_desc *desc, u8 *key, int keylen)
+{
+	u8 pad[SHA256_BLOCK_SIZE];
+	int i;
+
+	desc->tfm = sha256_hash;
+	crypto_shash_init(desc);
+	for (i = 0; i < sizeof(pad); i++) {
+		if (i < keylen)
+			pad[i] = key[i];
+		else
+			pad[i] = 0;
+		pad[i] ^= HMAC_IPAD_VALUE;
+	}
+	crypto_shash_update(desc, pad, sizeof(pad));
+}
+
+static void hmac_final(struct shash_desc *desc, u8 *key, int keylen, u8 *out)
+{
+	u8 pad[SHA256_BLOCK_SIZE];
+	int i;
+
+	for (i = 0; i < sizeof(pad); i++) {
+		if (i < keylen)
+			pad[i] = key[i];
+		else
+			pad[i] = 0;
+		pad[i] ^= HMAC_OPAD_VALUE;
+	}
+
+	/* collect the final hash;  use out as temporary storage */
+	crypto_shash_final(desc, out);
+
+	/* reuse the desc */
+	crypto_shash_init(desc);
+	crypto_shash_update(desc, pad, sizeof(pad));
+	crypto_shash_update(desc, out, SHA256_DIGEST_SIZE);
+	crypto_shash_final(desc, out);
+}
+
+/*
+ * assume hash sha256 and nonces u, v of size SHA256_DIGEST_SIZE but
+ * otherwise standard KDFa.  Note output is in bytes not bits.
+ */
+static void KDFa(u8 *key, int keylen, const char *label, u8 *u,
+		 u8 *v, int bytes, u8 *out)
+{
+	u32 counter;
+	const __be32 bits = cpu_to_be32(bytes * 8);
+
+	for (counter = 1; bytes > 0; bytes -= SHA256_DIGEST_SIZE, counter++,
+		     out += SHA256_DIGEST_SIZE) {
+		SHASH_DESC_ON_STACK(desc, sha256_hash);
+		__be32 c = cpu_to_be32(counter);
+
+		hmac_init(desc, key, keylen);
+		crypto_shash_update(desc, (u8 *)&c, sizeof(c));
+		crypto_shash_update(desc, label, strlen(label)+1);
+		crypto_shash_update(desc, u, SHA256_DIGEST_SIZE);
+		crypto_shash_update(desc, v, SHA256_DIGEST_SIZE);
+		crypto_shash_update(desc, (u8 *)&bits, sizeof(bits));
+		hmac_final(desc, key, keylen, out);
+	}
+}
+
+/*
+ * Somewhat of a bastardization of the real KDFe.  We're assuming
+ * we're working with known point sizes for the input parameters and
+ * the hash algorithm is fixed at sha256.  Because we know that the
+ * point size is 32 bytes like the hash size, there's no need to loop
+ * in this KDF.
+ */
+static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8 *pt_v,
+		 u8 *keyout)
+{
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+	/*
+	 * this should be an iterative counter, but because we know
+	 *  we're only taking 32 bytes for the point using a sha256
+	 *  hash which is also 32 bytes, there's only one loop
+	 */
+	__be32 c = cpu_to_be32(1);
+
+	desc->tfm = sha256_hash;
+
+	crypto_shash_init(desc);
+	/* counter (BE) */
+	crypto_shash_update(desc, (u8 *)&c, sizeof(c));
+	/* secret value */
+	crypto_shash_update(desc, z, EC_PT_SZ);
+	/* string including trailing zero */
+	crypto_shash_update(desc, str, strlen(str)+1);
+	crypto_shash_update(desc, pt_u, EC_PT_SZ);
+	crypto_shash_update(desc, pt_v, EC_PT_SZ);
+	crypto_shash_final(desc, keyout);
+}
+
+static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
+				struct tpm2_auth *auth)
+{
+	struct crypto_kpp *kpp;
+	struct kpp_request *req;
+	struct scatterlist s[2], d[1];
+	struct ecdh p = {0};
+	u8 encoded_key[EC_PT_SZ], *x, *y;
+	unsigned int buf_len;
+	u8 *secret;
+
+	secret = kmalloc(EC_PT_SZ, GFP_KERNEL);
+	if (!secret)
+		return;
+
+	/* secret is two sized points */
+	tpm_buf_append_u16(buf, (EC_PT_SZ + 2)*2);
+	/*
+	 * we cheat here and append uninitialized data to form
+	 * the points.  All we care about is getting the two
+	 * co-ordinate pointers, which will be used to overwrite
+	 * the uninitialized data
+	 */
+	tpm_buf_append_u16(buf, EC_PT_SZ);
+	x = &buf->data[tpm_buf_length(buf)];
+	tpm_buf_append(buf, encoded_key, EC_PT_SZ);
+	tpm_buf_append_u16(buf, EC_PT_SZ);
+	y = &buf->data[tpm_buf_length(buf)];
+	tpm_buf_append(buf, encoded_key, EC_PT_SZ);
+	sg_init_table(s, 2);
+	sg_set_buf(&s[0], x, EC_PT_SZ);
+	sg_set_buf(&s[1], y, EC_PT_SZ);
+
+	kpp = crypto_alloc_kpp("ecdh-nist-p256", CRYPTO_ALG_INTERNAL, 0);
+	if (IS_ERR(kpp)) {
+		dev_err(&chip->dev, "crypto ecdh allocation failed\n");
+		goto out_free;
+	}
+
+	buf_len = crypto_ecdh_key_len(&p);
+	if (sizeof(encoded_key) < buf_len) {
+		dev_err(&chip->dev, "salt buffer too small needs %d\n",
+			buf_len);
+		goto out;
+	}
+	crypto_ecdh_encode_key(encoded_key, buf_len, &p);
+	/* this generates a random private key */
+	crypto_kpp_set_secret(kpp, encoded_key, buf_len);
+
+	/* salt is now the public point of this private key */
+	req = kpp_request_alloc(kpp, GFP_KERNEL);
+	if (!req)
+		goto out;
+	kpp_request_set_input(req, NULL, 0);
+	kpp_request_set_output(req, s, EC_PT_SZ*2);
+	crypto_kpp_generate_public_key(req);
+	/*
+	 * we're not done: now we have to compute the shared secret
+	 * which is our private key multiplied by the tpm_key public
+	 * point, we actually only take the x point and discard the y
+	 * point and feed it through KDFe to get the final secret salt
+	 */
+	sg_set_buf(&s[0], chip->ec_point_x, EC_PT_SZ);
+	sg_set_buf(&s[1], chip->ec_point_y, EC_PT_SZ);
+	kpp_request_set_input(req, s, EC_PT_SZ*2);
+	sg_init_one(d, secret, EC_PT_SZ);
+	kpp_request_set_output(req, d, EC_PT_SZ);
+	crypto_kpp_compute_shared_secret(req);
+	kpp_request_free(req);
+
+	/* pass the shared secret through KDFe for salt */
+	KDFe(secret, "SECRET", x, chip->ec_point_x, auth->salt);
+ out:
+	crypto_free_kpp(kpp);
+ out_free:
+	kfree(secret);
+}
+
+/**
+ * tpm_buf_append_hmac_session() append a TPM session element
+ * @buf: The buffer to be appended
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ * @attributes: The session attributes
+ * @passphrase: The session authority (NULL if none)
+ * @passphraselen: The length of the session authority (0 if none)
+ *
+ * This fills in a session structure in the TPM command buffer, except
+ * for the HMAC which cannot be computed until the command buffer is
+ * complete.  The type of session is controlled by the @attributes,
+ * the main ones of which are TPM2_SA_CONTINUE_SESSION which means the
+ * session won't terminate after tpm_buf_check_hmac_response(),
+ * TPM2_SA_DECRYPT which means this buffers first parameter should be
+ * encrypted with a session key and TPM2_SA_ENCRYPT, which means the
+ * response buffer's first parameter needs to be decrypted (confusing,
+ * but the defines are written from the point of view of the TPM).
+ *
+ * Any session appended by this command must be finalized by calling
+ * tpm_buf_fill_hmac_session() otherwise the HMAC will be incorrect
+ * and the TPM will reject the command.
+ *
+ * As with most tpm_buf operations, success is assumed because failure
+ * will be caused by an incorrect programming model and indicated by a
+ * kernel message.
+ */
+void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth,
+				 u8 attributes, u8 *passphrase,
+				 int passphraselen)
+{
+	u8 nonce[SHA256_DIGEST_SIZE];
+	u32 len;
+
+	/*
+	 * The Architecture Guide requires us to strip trailing zeros
+	 * before computing the HMAC
+	 */
+	while (passphrase && passphraselen > 0
+	       && passphrase[passphraselen - 1] == '\0')
+		passphraselen--;
+
+	auth->attrs = attributes;
+	auth->passphraselen = passphraselen;
+	if (passphraselen)
+		memcpy(auth->passphrase, passphrase, passphraselen);
+
+	if (auth->session != tpm_buf_length(buf)) {
+		/* we're not the first session */
+		len = get_unaligned_be32(&buf->data[auth->session]);
+		if (4 + len + auth->session != tpm_buf_length(buf)) {
+			WARN(1, "session length mismatch, cannot append");
+			return;
+		}
+
+		/* add our new session */
+		len += 9 + 2 * SHA256_DIGEST_SIZE;
+		put_unaligned_be32(len, &buf->data[auth->session]);
+	} else {
+		tpm_buf_append_u32(buf, 9 + 2 * SHA256_DIGEST_SIZE);
+	}
+
+	/* random number for our nonce */
+	get_random_bytes(nonce, sizeof(nonce));
+	memcpy(auth->our_nonce, nonce, sizeof(nonce));
+	tpm_buf_append_u32(buf, auth->handle);
+	/* our new nonce */
+	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
+	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
+	tpm_buf_append_u8(buf, auth->attrs);
+	/* and put a placeholder for the hmac */
+	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
+	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
+}
+EXPORT_SYMBOL(tpm_buf_append_hmac_session);
+
+/**
+ * tpm_buf_fill_hmac_session() - finalize the session HMAC
+ * @buf: The buffer to be appended
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ *
+ * This command must not be called until all of the parameters have
+ * been appended to @buf otherwise the computed HMAC will be
+ * incorrect.
+ *
+ * This function computes and fills in the session HMAC using the
+ * session key and, if TPM2_SA_DECRYPT was specified, computes the
+ * encryption key and encrypts the first parameter of the command
+ * buffer with it.
+ *
+ * As with most tpm_buf operations, success is assumed because failure
+ * will be caused by an incorrect programming model and indicated by a
+ * kernel message.
+ */
+void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
+{
+	u32 cc, handles, val;
+	struct tpm_chip *chip = auth->chip;
+	int i;
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+	const u8 *s, *p;
+	u8 *hmac = NULL;
+	u32 attrs;
+	u8 cphash[SHA256_DIGEST_SIZE];
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+
+	/* save the command code in BE format */
+	auth->ordinal = head->ordinal;
+
+	desc->tfm = sha256_hash;
+
+	cc = be32_to_cpu(head->ordinal);
+
+	i = tpm2_find_cc(chip, cc);
+	if (i < 0) {
+		dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
+		return;
+	}
+	attrs = chip->cc_attrs_tbl[i];
+
+	handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+
+	s = &buf->data[TPM_HEADER_SIZE];
+	/*
+	 * just check the names, it's easy to make mistakes.  This
+	 * would happen if someone added a handle via
+	 * tpm_buf_append_u32() instead of tpm_buf_append_name()
+	 */
+	for (i = 0; i < handles; i++) {
+		u32 handle = tpm_get_inc_u32(&s);
+
+		if (auth->name_h[i] != handle) {
+			dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
+				  i);
+			return;
+		}
+	}
+	/* point s to the start of the sessions */
+	val = tpm_get_inc_u32(&s);
+	/* point p to the start of the parameters */
+	p = s + val;
+	for (i = 1; s < p; i++) {
+		u32 handle = tpm_get_inc_u32(&s);
+		u16 len;
+		u8 a;
+
+		/* nonce (already in auth) */
+		len = tpm_get_inc_u16(&s);
+		s += len;
+
+		a = *s++;
+
+		len = tpm_get_inc_u16(&s);
+		if (handle == auth->handle && auth->attrs == a) {
+			hmac = (u8 *)s;
+			/*
+			 * save our session number so we know which
+			 * session in the response belongs to us
+			 */
+			auth->session = i;
+		}
+
+		s += len;
+	}
+	if (s != p) {
+		dev_err(&chip->dev, "TPM session length is incorrect\n");
+		return;
+	}
+	if (!hmac) {
+		dev_err(&chip->dev, "TPM could not find HMAC session\n");
+		return;
+	}
+
+	/* encrypt before HMAC */
+	if (auth->attrs & TPM2_SA_DECRYPT) {
+		struct scatterlist sg[1];
+		u16 len;
+		SYNC_SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
+		DECLARE_CRYPTO_WAIT(wait);
+
+		skcipher_request_set_sync_tfm(req, auth->aes);
+		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
+					      crypto_req_done, &wait);
+
+		/* need key and IV */
+		KDFa(auth->session_key, SHA256_DIGEST_SIZE
+		     + auth->passphraselen, "CFB", auth->our_nonce,
+		     auth->tpm_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
+		     auth->scratch);
+		crypto_sync_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
+		len = tpm_get_inc_u16(&p);
+		sg_init_one(sg, p, len);
+		skcipher_request_set_crypt(req, sg, sg, len,
+					   auth->scratch + AES_KEYBYTES);
+		crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
+		/* reset p to beginning of parameters for HMAC */
+		p -= 2;
+	}
+
+	crypto_shash_init(desc);
+	/* ordinal is already BE */
+	crypto_shash_update(desc, (u8 *)&head->ordinal, sizeof(head->ordinal));
+	/* add the handle names */
+	for (i = 0; i < handles; i++) {
+		u8 mso = auth->name_h[i] >> 24;
+
+		if (mso == 0x81 || mso == 0x80 || mso == 0x01) {
+			crypto_shash_update(desc, auth->name[i],
+					    name_size(auth->name[i]));
+		} else {
+			__be32 h = cpu_to_be32(auth->name_h[i]);
+
+			crypto_shash_update(desc, (u8 *)&h, 4);
+		}
+	}
+	if (buf->data - s != tpm_buf_length(buf))
+		crypto_shash_update(desc, s, buf->data
+				    + tpm_buf_length(buf) - s);
+	crypto_shash_final(desc, cphash);
+
+	/* now calculate the hmac */
+	hmac_init(desc, auth->session_key, sizeof(auth->session_key)
+		  + auth->passphraselen);
+	crypto_shash_update(desc, cphash, sizeof(cphash));
+	crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
+	crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
+	crypto_shash_update(desc, &auth->attrs, 1);
+	hmac_final(desc, auth->session_key, sizeof(auth->session_key)
+		   + auth->passphraselen, hmac);
+}
+EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
+
+static int parse_read_public(char *name, const u8 *data)
+{
+	struct tpm_header *head = (struct tpm_header *)data;
+	u32 tot_len = be32_to_cpu(head->length);
+	u32 val;
+
+	data += TPM_HEADER_SIZE;
+	/* we're starting after the header so adjust the length */
+	tot_len -= TPM_HEADER_SIZE;
+
+	/* skip public */
+	val = tpm_get_inc_u16(&data);
+	if (val > tot_len)
+		return -EINVAL;
+	data += val;
+	/* name */
+	val = tpm_get_inc_u16(&data);
+	if (val != name_size(data))
+		return -EINVAL;
+	memcpy(name, data, name_size(data));
+	/* forget the rest */
+	return 0;
+}
+
+static int tpm2_readpublic(struct tpm_chip *chip, u32 handle, char *name)
+{
+	struct tpm_buf buf;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, handle);
+	rc = tpm_transmit_cmd(chip, &buf, 0, "read public");
+	if (rc == TPM2_RC_SUCCESS)
+		rc = parse_read_public(name, buf.data);
+
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
+
+/**
+ * tpm_buf_append_name() - add a handle area to the buffer
+ * @buf: The buffer to be appended
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ * @handle: The handle to be appended
+ * @name: The name of the handle (may be NULL)
+ *
+ * In order to compute session HMACs, we need to know the names of the
+ * objects pointed to by the handles.  For most objects, this is simly
+ * the actual 4 byte handle or an empty buf (in these cases @name
+ * should be NULL) but for volatile objects, permanent objects and NV
+ * areas, the name is defined as the hash (according to the name
+ * algorithm which should be set to sha256) of the public area to
+ * which the two byte algorithm id has been appended.  For these
+ * objects, the @name pointer should point to this.  If a name is
+ * required but @name is NULL, then TPM2_ReadPublic() will be called
+ * on the handle to obtain the name.
+ *
+ * As with most tpm_buf operations, success is assumed because failure
+ * will be caused by an incorrect programming model and indicated by a
+ * kernel message.
+ */
+void tpm_buf_append_name(struct tpm_buf *buf, struct tpm2_auth *auth,
+			 u32 handle, u8 *name)
+{
+	int slot;
+	u8 mso = handle >> 24;
+
+	slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE)/4;
+	if (slot >= AUTH_MAX_NAMES) {
+		dev_err(&auth->chip->dev, "TPM: too many handles\n");
+		return;
+	}
+	WARN(auth->session != tpm_buf_length(buf),
+	     "name added in wrong place\n");
+	tpm_buf_append_u32(buf, handle);
+	auth->session += 4;
+
+	if (mso == 0x81 || mso == 0x80 || mso == 0x01) {
+		if (!name)
+			tpm2_readpublic(auth->chip, handle, auth->name[slot]);
+	} else {
+		if (name)
+			dev_err(&auth->chip->dev, "TPM: Handle does not require name but one is specified\n");
+	}
+
+	auth->name_h[slot] = handle;
+	if (name)
+		memcpy(auth->name[slot], name, name_size(name));
+}
+EXPORT_SYMBOL(tpm_buf_append_name);
+
+/**
+ * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
+ * @buf: the original command buffer (which now contains the response)
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ * @rc: the return code from tpm_transmit_cmd
+ *
+ * If @rc is non zero, @buf may not contain an actual return, so @rc
+ * is passed through as the return and the session cleaned up and
+ * de-allocated if required (this is required if
+ * TPM2_SA_CONTINUE_SESSION was not specified as a session flag).
+ *
+ * If @rc is zero, the response HMAC is computed against the returned
+ * @buf and matched to the TPM one in the session area.  If there is a
+ * mismatch, an error is logged and -EINVAL returned.
+ *
+ * The reason for this is that the command issue and HMAC check
+ * sequence should look like:
+ *
+ *	rc = tpm_transmit_cmd(...);
+ *	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
+ *	if (rc)
+ *		...
+ *
+ * Which is easily layered into the current contrl flow.
+ *
+ * Returns: 0 on success or an error.
+ */
+int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
+				int rc)
+{
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+	struct tpm_chip *chip = auth->chip;
+	const u8 *s, *p;
+	u8 rphash[SHA256_DIGEST_SIZE];
+	u32 attrs;
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+	u16 tag = be16_to_cpu(head->tag);
+	u32 cc = be32_to_cpu(auth->ordinal);
+	int parm_len, len, i, handles;
+
+	if (auth->session >= TPM_HEADER_SIZE) {
+		WARN(1, "tpm session not filled correctly\n");
+		goto out;
+	}
+
+	if (rc != 0)
+		/* pass non success rc through and close the session */
+		goto out;
+
+	rc = -EINVAL;
+	if (tag != TPM2_ST_SESSIONS) {
+		dev_err(&chip->dev, "TPM: HMAC response check has no sessions tag\n");
+		goto out;
+	}
+
+	i = tpm2_find_cc(chip, cc);
+	if (i < 0)
+		goto out;
+	attrs = chip->cc_attrs_tbl[i];
+	handles = (attrs >> TPM2_CC_ATTR_RHANDLE) & 1;
+
+	/* point to area beyond handles */
+	s = &buf->data[TPM_HEADER_SIZE + handles * 4];
+	parm_len = tpm_get_inc_u32(&s);
+	p = s;
+	s += parm_len;
+	/* skip over any sessions before ours */
+	for (i = 0; i < auth->session - 1; i++) {
+		len = tpm_get_inc_u16(&s);
+		s += len + 1;
+		len = tpm_get_inc_u16(&s);
+		s += len;
+	}
+	/* TPM nonce */
+	len = tpm_get_inc_u16(&s);
+	if (s - buf->data + len > tpm_buf_length(buf))
+		goto out;
+	if (len != SHA256_DIGEST_SIZE)
+		goto out;
+	memcpy(auth->tpm_nonce, s, len);
+	s += len;
+	attrs = *s++;
+	len = tpm_get_inc_u16(&s);
+	if (s - buf->data + len != tpm_buf_length(buf))
+		goto out;
+	if (len != SHA256_DIGEST_SIZE)
+		goto out;
+	/*
+	 * s points to the HMAC. now calculate comparison, beginning
+	 * with rphash
+	 */
+	desc->tfm = sha256_hash;
+	crypto_shash_init(desc);
+	/* yes, I know this is now zero, but it's what the standard says */
+	crypto_shash_update(desc, (u8 *)&head->return_code,
+			    sizeof(head->return_code));
+	/* ordinal is already BE */
+	crypto_shash_update(desc, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
+	crypto_shash_update(desc, p, parm_len);
+	crypto_shash_final(desc, rphash);
+
+	/* now calculate the hmac */
+	hmac_init(desc, auth->session_key, sizeof(auth->session_key)
+		  + auth->passphraselen);
+	crypto_shash_update(desc, rphash, sizeof(rphash));
+	crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
+	crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
+	crypto_shash_update(desc, &auth->attrs, 1);
+	/* we're done with the rphash, so put our idea of the hmac there */
+	hmac_final(desc, auth->session_key, sizeof(auth->session_key)
+		   + auth->passphraselen, rphash);
+	if (memcmp(rphash, s, SHA256_DIGEST_SIZE) == 0) {
+		rc = 0;
+	} else {
+		dev_err(&auth->chip->dev, "TPM: HMAC check failed\n");
+		goto out;
+	}
+
+	/* now do response decryption */
+	if (auth->attrs & TPM2_SA_ENCRYPT) {
+		struct scatterlist sg[1];
+		SYNC_SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
+		DECLARE_CRYPTO_WAIT(wait);
+
+		skcipher_request_set_sync_tfm(req, auth->aes);
+		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
+					      crypto_req_done, &wait);
+
+		/* need key and IV */
+		KDFa(auth->session_key, SHA256_DIGEST_SIZE
+		     + auth->passphraselen, "CFB", auth->tpm_nonce,
+		     auth->our_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
+		     auth->scratch);
+		crypto_sync_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
+		len = tpm_get_inc_u16(&p);
+		sg_init_one(sg, p, len);
+		skcipher_request_set_crypt(req, sg, sg, len,
+					   auth->scratch + AES_KEYBYTES);
+		crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
+	}
+
+ out:
+	if ((auth->attrs & TPM2_SA_CONTINUE_SESSION) == 0) {
+		/* manually close the session if it wasn't consumed */
+		if (rc)
+			tpm2_flush_context(chip, auth->handle);
+		crypto_free_sync_skcipher(auth->aes);
+		kfree(auth);
+	} else {
+		/* reset for next use  */
+		auth->session = TPM_HEADER_SIZE;
+	}
+
+	return rc;
+}
+EXPORT_SYMBOL(tpm_buf_check_hmac_response);
+
+/**
+ * tpm2_end_auth_session - kill the allocated auth session
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ *
+ * ends the session started by tpm2_start_auth_session and frees all
+ * the resources.  Under normal conditions,
+ * tpm_buf_check_hmac_response() will correctly end the session if
+ * required, so this function is only for use in error legs that will
+ * bypass the normal invocation of tpm_buf_check_hmac_respons().
+ */
+void tpm2_end_auth_session(struct tpm2_auth *auth)
+{
+	tpm2_flush_context(auth->chip, auth->handle);
+	crypto_free_sync_skcipher(auth->aes);
+	kfree(auth);
+}
+EXPORT_SYMBOL(tpm2_end_auth_session);
+
+static int parse_start_auth_session(struct tpm2_auth *auth, const u8 *data)
+{
+	struct tpm_header *head = (struct tpm_header *)data;
+	u32 tot_len = be32_to_cpu(head->length);
+	u32 val;
+
+	data += TPM_HEADER_SIZE;
+	/* we're starting after the header so adjust the length */
+	tot_len -= TPM_HEADER_SIZE;
+
+	/* should have handle plus nonce */
+	if (tot_len != 4 + 2 + sizeof(auth->tpm_nonce))
+		return -EINVAL;
+
+	auth->handle = tpm_get_inc_u32(&data);
+	val = tpm_get_inc_u16(&data);
+	if (val != sizeof(auth->tpm_nonce))
+		return -EINVAL;
+	memcpy(auth->tpm_nonce, data, sizeof(auth->tpm_nonce));
+	/* now compute the session key from the nonces */
+	KDFa(auth->salt, sizeof(auth->salt), "ATH", auth->tpm_nonce,
+	     auth->our_nonce, sizeof(auth->session_key), auth->session_key);
+
+	return 0;
+}
+
+/**
+ * tpm2_start_auth_session - create a HMAC authentication session with the TPM
+ * @chip: the TPM chip structure to create the session with
+ * @authp: A pointer to an opaque tpm2_auth structure to be allocated
+ *
+ * This function loads the NULL seed from its saved context and starts
+ * an authentication session on the null seed, allocates a tpm2_auth
+ * structure to contain all the session details necessary for
+ * performing the HMAC, encrypt and decrypt operations, fills it in
+ * and returns.  The NULL seed is flushed before this function returns.
+ *
+ * Return: zero on success or actual error encountered.  If return is
+ * zero, @authp will be allocated.
+ */
+int tpm2_start_auth_session(struct tpm_chip *chip, struct tpm2_auth **authp)
+{
+	struct tpm_buf buf;
+	struct tpm2_auth *auth;
+	int rc;
+	unsigned int offset = 0; /* dummy offset for null seed context */
+	u32 nullkey;
+
+	auth = kmalloc(sizeof(**authp), GFP_KERNEL);
+	if (!auth)
+		return -ENOMEM;
+
+	rc = tpm2_load_context(chip, chip->tpmkeycontext, &offset,
+			       &nullkey);
+	if (rc)
+		goto out;
+
+	auth->chip = chip;
+	auth->session = TPM_HEADER_SIZE;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
+	if (rc)
+		goto out;
+
+	/* salt key handle */
+	tpm_buf_append_u32(&buf, nullkey);
+	/* bind key handle */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+	/* nonce caller */
+	get_random_bytes(auth->our_nonce, sizeof(auth->our_nonce));
+	tpm_buf_append_u16(&buf, sizeof(auth->our_nonce));
+	tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
+
+	/* append encrypted salt and squirrel away unencrypted in auth */
+	tpm_buf_append_salt(&buf, chip, auth);
+	/* session type (HMAC, audit or policy) */
+	tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
+
+	/* symmetric encryption parameters */
+	/* symmetric algorithm */
+	tpm_buf_append_u16(&buf, TPM_ALG_AES);
+	/* bits for symmetric algorithm */
+	tpm_buf_append_u16(&buf, AES_KEYBITS);
+	/* symmetric algorithm mode (must be CFB) */
+	tpm_buf_append_u16(&buf, TPM_ALG_CFB);
+	/* hash algorithm for session */
+	tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
+
+	rc = tpm_transmit_cmd(chip, &buf, 0, "start auth session");
+	tpm2_flush_context(chip, nullkey);
+
+	if (rc == TPM2_RC_SUCCESS)
+		rc = parse_start_auth_session(auth, buf.data);
+
+	tpm_buf_destroy(&buf);
+
+	if (rc)
+		goto out;
+
+	auth->aes = crypto_alloc_sync_skcipher("cfb(aes)", 0, 0);
+	if (IS_ERR(auth->aes)) {
+		rc = PTR_ERR(auth->aes);
+		dev_err(&chip->dev, "TPM: error getting cfb(aes): %d\n", rc);
+	}
+ out:
+	if (rc)
+		kfree(auth);
+	else
+		*authp = auth;
+
+	return rc;
+}
+EXPORT_SYMBOL(tpm2_start_auth_session);
+
+static int parse_create_primary(struct tpm_chip *chip, u8 *data, u32 *nullkey)
+{
+	struct tpm_header *head = (struct tpm_header *)data;
+	u16 len;
+	u32 tot_len = be32_to_cpu(head->length);
+	u32 val, parm_len;
+	const u8 *resp, *tmp;
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+
+	data += TPM_HEADER_SIZE;
+	/* we're starting after the header so adjust the length */
+	tot_len -= TPM_HEADER_SIZE;
+
+	resp = data;
+	*nullkey = tpm_get_inc_u32(&resp);
+	parm_len = tpm_get_inc_u32(&resp);
+	if (parm_len + 8 > tot_len)
+		return -EINVAL;
+	len = tpm_get_inc_u16(&resp);
+	tmp = resp;
+	/* now we have the public area, compute the name of the object */
+	desc->tfm = sha256_hash;
+	put_unaligned_be16(TPM_ALG_SHA256, chip->tpmkeyname);
+	crypto_shash_init(desc);
+	crypto_shash_update(desc, resp, len);
+	crypto_shash_final(desc, chip->tpmkeyname + 2);
+	/* validate the public key */
+	val = tpm_get_inc_u16(&tmp);
+	/* key type (must be what we asked for) */
+	if (val != TPM_ALG_ECC)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* name algorithm */
+	if (val != TPM_ALG_SHA256)
+		return -EINVAL;
+	val = tpm_get_inc_u32(&tmp);
+	/* object properties */
+	if (val != (TPM2_OA_NO_DA |
+		    TPM2_OA_FIXED_TPM |
+		    TPM2_OA_FIXED_PARENT |
+		    TPM2_OA_SENSITIVE_DATA_ORIGIN |
+		    TPM2_OA_USER_WITH_AUTH |
+		    TPM2_OA_DECRYPT |
+		    TPM2_OA_RESTRICTED))
+		return -EINVAL;
+	/* auth policy (empty) */
+	val = tpm_get_inc_u16(&tmp);
+	if (val != 0)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* symmetric key parameters */
+	if (val != TPM_ALG_AES)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* symmetric key length */
+	if (val != AES_KEYBITS)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* symmetric encryption scheme */
+	if (val != TPM_ALG_CFB)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* signing scheme */
+	if (val != TPM_ALG_NULL)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* ECC Curve */
+	if (val != TPM2_ECC_NIST_P256)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* KDF Scheme */
+	if (val != TPM_ALG_NULL)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* x point */
+	if (val != 32)
+		return -EINVAL;
+	memcpy(chip->ec_point_x, tmp, val);
+	tmp += val;
+	val = tpm_get_inc_u16(&tmp);
+	if (val != 32)
+		return -EINVAL;
+	memcpy(chip->ec_point_y, tmp, val);
+	tmp += val;
+	resp += len;
+	/* should have exactly consumed the tpm2b public structure */
+	if (tmp != resp)
+		return -EINVAL;
+	if (resp - data > parm_len)
+		return -EINVAL;
+	/* creation data (skip) */
+	len = tpm_get_inc_u16(&resp);
+	resp += len;
+	if (resp - data > parm_len)
+		return -EINVAL;
+	/* creation digest (must be sha256) */
+	len = tpm_get_inc_u16(&resp);
+	resp += len;
+	if (len != SHA256_DIGEST_SIZE || resp - data > parm_len)
+		return -EINVAL;
+	/* TPMT_TK_CREATION follows */
+	/* tag, must be TPM_ST_CREATION (0x8021) */
+	val = tpm_get_inc_u16(&resp);
+	if (val != TPM2_ST_CREATION || resp - data > parm_len)
+		return -EINVAL;
+	/* hierarchy (must be NULL) */
+	val = tpm_get_inc_u32(&resp);
+	if (val != TPM2_RH_NULL || resp - data > parm_len)
+		return -EINVAL;
+	/* the ticket digest HMAC (might not be sha256) */
+	len = tpm_get_inc_u16(&resp);
+	resp += len;
+	if (resp - data > parm_len)
+		return -EINVAL;
+	/*
+	 * finally we have the name, which is a sha256 digest plus a 2
+	 * byte algorithm type
+	 */
+	len = tpm_get_inc_u16(&resp);
+	if (resp + len - data != parm_len + 8)
+		return -EINVAL;
+	if (len != SHA256_DIGEST_SIZE + 2)
+		return -EINVAL;
+
+	if (memcmp(chip->tpmkeyname, resp, SHA256_DIGEST_SIZE + 2) != 0) {
+		dev_err(&chip->dev, "NULL Seed name comparison failed\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy, u32 *handle)
+{
+	int rc;
+	struct tpm_buf buf;
+	struct tpm_buf template;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
+	if (rc)
+		return rc;
+
+	rc = tpm_buf_init_2b(&template);
+	if (rc) {
+		tpm_buf_destroy(&buf);
+		return rc;
+	}
+
+	/*
+	 * create the template.  Note: in order for userspace to
+	 * verify the security of the system, it will have to create
+	 * and certify this NULL primary, meaning all the template
+	 * parameters will have to be identical, so conform exactly to
+	 * the TCG TPM v2.0 Provisioning Guidance for the SRK ECC
+	 * key
+	 */
+
+	/* key type */
+	tpm_buf_append_u16(&template, TPM_ALG_ECC);
+	/* name algorithm */
+	tpm_buf_append_u16(&template, TPM_ALG_SHA256);
+	/* object properties */
+	tpm_buf_append_u32(&template, TPM2_OA_NO_DA |
+			   TPM2_OA_FIXED_TPM |
+			   TPM2_OA_FIXED_PARENT |
+			   TPM2_OA_SENSITIVE_DATA_ORIGIN |
+			   TPM2_OA_USER_WITH_AUTH |
+			   TPM2_OA_DECRYPT |
+			   TPM2_OA_RESTRICTED);
+	/* sauth policy (empty) */
+	tpm_buf_append_u16(&template, 0);
+
+	/* BEGIN parameters: key specific; for ECC*/
+	/* symmetric algorithm */
+	tpm_buf_append_u16(&template, TPM_ALG_AES);
+	/* bits for symmetric algorithm */
+	tpm_buf_append_u16(&template, 128);
+	/* algorithm mode (must be CFB) */
+	tpm_buf_append_u16(&template, TPM_ALG_CFB);
+	/* scheme (NULL means any scheme) */
+	tpm_buf_append_u16(&template, TPM_ALG_NULL);
+	/* ECC Curve ID */
+	tpm_buf_append_u16(&template, TPM2_ECC_NIST_P256);
+	/* KDF Scheme */
+	tpm_buf_append_u16(&template, TPM_ALG_NULL);
+	/* unique: key specific; for ECC it is two points */
+	tpm_buf_append_u16(&template, 0);
+	tpm_buf_append_u16(&template, 0);
+	/* END parameters */
+
+	/* primary handle */
+	tpm_buf_append_u32(&buf, hierarchy);
+	tpm_buf_append_empty_auth(&buf, TPM2_RS_PW);
+	/* sensitive create size is 4 for two empty buffers */
+	tpm_buf_append_u16(&buf, 4);
+	/* sensitive create auth data (empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* sensitive create sensitive data (empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* the public template */
+	tpm_buf_append_2b(&buf, &template);
+	tpm_buf_destroy(&template);
+	/* outside info (empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* creation PCR (none) */
+	tpm_buf_append_u32(&buf, 0);
+
+	rc = tpm_transmit_cmd(chip, &buf, 0,
+			      "attempting to create NULL primary");
+
+	if (rc == TPM2_RC_SUCCESS)
+		rc = parse_create_primary(chip, buf.data, handle);
+
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
+
+int tpm2_create_null_primary(struct tpm_chip *chip) {
+	u32 nullkey;
+	int rc;
+
+	rc = tpm2_create_primary(chip, TPM2_RH_NULL, &nullkey);
+
+	if (rc == TPM2_RC_SUCCESS) {
+		unsigned int offset = 0; /* dummy offset for tpmkeycontext */
+
+		rc = tpm2_save_context(chip, nullkey, chip->tpmkeycontext,
+				       sizeof(chip->tpmkeycontext), &offset);
+		tpm2_flush_context(chip, nullkey);
+	}
+
+	return rc;
+}
+
+int tpm2_sessions_init(struct tpm_chip *chip)
+{
+	int rc;
+
+	sha256_hash = crypto_alloc_shash("sha256", 0, 0);
+	if (!sha256_hash) {
+		dev_err(&chip->dev, "TPM: failed to allocate hash\n");
+		return -ENOMEM;
+	}
+
+	rc = tpm2_create_null_primary(chip);
+	if (rc)
+		dev_err(&chip->dev, "TPM: security failed (NULL seed derivation): %d\n", rc);
+	return rc;
+}
+EXPORT_SYMBOL(tpm2_sessions_init);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index fa8d1f932c0f..bdf19538b103 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -30,17 +30,28 @@
 struct tpm_chip;
 struct trusted_key_payload;
 struct trusted_key_options;
+/* opaque structure, holds auth session parameters like the session key */
+struct tpm2_auth;
+
+enum tpm2_session_types {
+	TPM2_SE_HMAC	= 0x00,
+	TPM2_SE_POLICY	= 0x01,
+	TPM2_SE_TRIAL	= 0x02,
+};
 
 /* if you add a new hash to this, increment TPM_MAX_HASHES below */
 enum tpm_algorithms {
 	TPM_ALG_ERROR		= 0x0000,
 	TPM_ALG_SHA1		= 0x0004,
+	TPM_ALG_AES		= 0x0006,
 	TPM_ALG_KEYEDHASH	= 0x0008,
 	TPM_ALG_SHA256		= 0x000B,
 	TPM_ALG_SHA384		= 0x000C,
 	TPM_ALG_SHA512		= 0x000D,
 	TPM_ALG_NULL		= 0x0010,
 	TPM_ALG_SM3_256		= 0x0012,
+	TPM_ALG_ECC		= 0x0023,
+	TPM_ALG_CFB		= 0x0043,
 };
 
 /*
@@ -49,6 +60,11 @@ enum tpm_algorithms {
  */
 #define TPM_MAX_HASHES	5
 
+enum tpm2_curves {
+	TPM2_ECC_NONE		= 0x0000,
+	TPM2_ECC_NIST_P256	= 0x0003,
+};
+
 struct tpm_digest {
 	u16 alg_id;
 	u8 digest[TPM_MAX_DIGEST_SIZE];
@@ -116,6 +132,20 @@ struct tpm_chip_seqops {
 	const struct seq_operations *seqops;
 };
 
+/* fixed define for the curve we use which is NIST_P256 */
+#define EC_PT_SZ	32
+
+/*
+ * fixed define for the size of a name.  This is actually HASHALG size
+ * plus 2, so 32 for SHA256
+ */
+#define TPM2_NAME_SIZE	34
+
+/*
+ * The maximum size for an object context
+ */
+#define TPM2_MAX_CONTEXT_SIZE 4096
+
 struct tpm_chip {
 	struct device dev;
 	struct device devs;
@@ -170,6 +200,12 @@ struct tpm_chip {
 
 	/* active locality */
 	int locality;
+
+	/* details for communication security via sessions */
+	u8 tpmkeycontext[TPM2_MAX_CONTEXT_SIZE]; /* context for NULL seed */
+	u8 tpmkeyname[TPM2_NAME_SIZE];		 /* name of NULL seed */
+	u8 ec_point_x[EC_PT_SZ];
+	u8 ec_point_y[EC_PT_SZ];
 };
 
 #define TPM_HEADER_SIZE		10
@@ -194,6 +230,7 @@ enum tpm2_timeouts {
 enum tpm2_structures {
 	TPM2_ST_NO_SESSIONS	= 0x8001,
 	TPM2_ST_SESSIONS	= 0x8002,
+	TPM2_ST_CREATION	= 0x8021,
 };
 
 /* Indicates from what layer of the software stack the error comes from */
@@ -231,6 +268,10 @@ enum tpm2_command_codes {
 	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
 	TPM2_CC_CONTEXT_SAVE	        = 0x0162,
 	TPM2_CC_FLUSH_CONTEXT	        = 0x0165,
+	TPM2_CC_POLICY_AUTHVALUE	= 0x016B,
+	TPM2_CC_POLICY_COUNTER_TIMER	= 0x016D,
+	TPM2_CC_READ_PUBLIC		= 0x0173,
+	TPM2_CC_START_AUTH_SESS		= 0x0176,
 	TPM2_CC_VERIFY_SIGNATURE        = 0x0177,
 	TPM2_CC_GET_CAPABILITY	        = 0x017A,
 	TPM2_CC_GET_RANDOM	        = 0x017B,
@@ -243,6 +284,7 @@ enum tpm2_command_codes {
 };
 
 enum tpm2_permanent_handles {
+	TPM2_RH_NULL		= 0x40000007,
 	TPM2_RS_PW		= 0x40000009,
 };
 
@@ -306,16 +348,30 @@ enum tpm_buf_flags {
 struct tpm_buf {
 	unsigned int flags;
 	u8 *data;
+	u8 handles;
 };
 
 enum tpm2_object_attributes {
 	TPM2_OA_FIXED_TPM		= BIT(1),
+	TPM2_OA_ST_CLEAR		= BIT(2),
 	TPM2_OA_FIXED_PARENT		= BIT(4),
+	TPM2_OA_SENSITIVE_DATA_ORIGIN	= BIT(5),
 	TPM2_OA_USER_WITH_AUTH		= BIT(6),
+	TPM2_OA_ADMIN_WITH_POLICY	= BIT(7),
+	TPM2_OA_NO_DA			= BIT(10),
+	TPM2_OA_ENCRYPTED_DUPLICATION	= BIT(11),
+	TPM2_OA_RESTRICTED		= BIT(16),
+	TPM2_OA_DECRYPT			= BIT(17),
+	TPM2_OA_SIGN			= BIT(18),
 };
 
 enum tpm2_session_attributes {
 	TPM2_SA_CONTINUE_SESSION	= BIT(0),
+	TPM2_SA_AUDIT_EXCLUSIVE		= BIT(1),
+	TPM2_SA_AUDIT_RESET		= BIT(3),
+	TPM2_SA_DECRYPT			= BIT(5),
+	TPM2_SA_ENCRYPT			= BIT(6),
+	TPM2_SA_AUDIT			= BIT(7),
 };
 
 struct tpm2_hash {
@@ -369,6 +425,15 @@ extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
 extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
 extern struct tpm_chip *tpm_default_chip(void);
 void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
+static inline void tpm_buf_append_empty_auth(struct tpm_buf *buf, u32 handle)
+{
+	/* simple authorization for empty auth */
+	tpm_buf_append_u32(buf, 9);		/* total length of auth */
+	tpm_buf_append_u32(buf, handle);
+	tpm_buf_append_u16(buf, 0);		/* nonce len */
+	tpm_buf_append_u8(buf, 0);		/* attributes */
+	tpm_buf_append_u16(buf, 0);		/* hmac len */
+}
 #else
 static inline int tpm_is_tpm2(struct tpm_chip *chip)
 {
@@ -399,5 +464,100 @@ static inline struct tpm_chip *tpm_default_chip(void)
 {
 	return NULL;
 }
+
+static inline void tpm_buf_append_empty_auth(struct tpm_buf *buf, u32 handle)
+{
+}
 #endif
+#ifdef CONFIG_TPM_BUS_SECURITY
+
+int tpm2_start_auth_session(struct tpm_chip *chip, struct tpm2_auth **authp);
+void tpm_buf_append_name(struct tpm_buf *buf, struct tpm2_auth *auth,
+			 u32 handle, u8 *name);
+void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth,
+				 u8 attributes, u8 *passphrase,
+				 int passphraselen);
+static inline void tpm_buf_append_hmac_session_opt(struct tpm_buf *buf,
+						   struct tpm2_auth *auth,
+						   u8 attributes,
+						   u8 *passphrase,
+						   int passphraselen)
+{
+	tpm_buf_append_hmac_session(buf, auth, attributes, passphrase,
+				    passphraselen);
+}
+void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth);
+int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
+				int rc);
+void tpm2_end_auth_session(struct tpm2_auth *auth);
+#else
+#include <asm/unaligned.h>
+
+static inline int tpm2_start_auth_session(struct tpm_chip *chip,
+					  struct tpm2_auth **authp)
+{
+	return 0;
+}
+static inline void tpm2_end_auth_session(struct tpm2_auth *auth)
+{
+}
+static inline void tpm_buf_append_name(struct tpm_buf *buf,
+				       struct tpm2_auth *auth,
+				       u32 handle, u8 *name)
+{
+	tpm_buf_append_u32(buf, handle);
+	/* count the number of handles in the upper bits of flags */
+	buf->handles++;
+}
+static inline void tpm_buf_append_hmac_session(struct tpm_buf *buf,
+					       struct tpm2_auth *auth,
+					       u8 attributes, u8 *passphrase,
+					       int passphraselen)
+{
+	/* offset tells us where the sessions area begins */
+	int offset = buf->handles * 4 + TPM_HEADER_SIZE;
+	u32 len = 9 + passphraselen;
+	if (tpm_buf_length(buf) != offset) {
+		/* not the first session so update the existing length */
+		len += get_unaligned_be32(&buf->data[offset]);
+		put_unaligned_be32(len, &buf->data[offset]);
+	} else {
+		tpm_buf_append_u32(buf, len);
+	}
+	/* auth handle */
+	tpm_buf_append_u32(buf, TPM2_RS_PW);
+	/* nonce */
+	tpm_buf_append_u16(buf, 0);
+	/* attributes */
+	tpm_buf_append_u8(buf, 0);
+	/* passphrase */
+	tpm_buf_append_u16(buf, passphraselen);
+	tpm_buf_append(buf, passphrase, passphraselen);
+}
+static inline void tpm_buf_append_hmac_session_opt(struct tpm_buf *buf,
+						   struct tpm2_auth *auth,
+						   u8 attributes,
+						   u8 *passphrase,
+						   int passphraselen)
+{
+	int offset = buf->handles * 4 + TPM_HEADER_SIZE;
+	struct tpm_header *head = (struct tpm_header *) buf->data;
+
+	/* if the only sessions are optional, the command tag
+	 * must change to TPM2_ST_NO_SESSIONS */
+	if (tpm_buf_length(buf) == offset)
+		head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+}
+static inline void tpm_buf_fill_hmac_session(struct tpm_buf *buf,
+					     struct tpm2_auth *auth)
+{
+}
+static inline int tpm_buf_check_hmac_response(struct tpm_buf *buf,
+					      struct tpm2_auth *auth,
+					      int rc)
+{
+	return rc;
+}
+#endif	/* CONFIG_TPM_BUS_SECURITY */
+
 #endif
-- 
2.35.3

[PATCH v2 07/11] tpm: add hmac checks to tpm2_pcr_extend()

[James Bottomley]

We use tpm2_pcr_extend() in trusted keys to extend a PCR to prevent a
key from being re-loaded until the next reboot.  To use this
functionality securely, that extend must be protected by a session
hmac.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm2-cmd.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 056dad3dd5c9..001360d5956d 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -216,13 +216,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 	return rc;
 }
 
-struct tpm2_null_auth_area {
-	__be32  handle;
-	__be16  nonce_size;
-	u8  attributes;
-	__be16  auth_size;
-} __packed;
-
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
@@ -236,24 +229,23 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 		    struct tpm_digest *digests)
 {
 	struct tpm_buf buf;
-	struct tpm2_null_auth_area auth_area;
+	struct tpm2_auth *auth;
 	int rc;
 	int i;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+	rc = tpm2_start_auth_session(chip, &auth);
 	if (rc)
 		return rc;
 
-	tpm_buf_append_u32(&buf, pcr_idx);
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+	if (rc) {
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
 
-	auth_area.handle = cpu_to_be32(TPM2_RS_PW);
-	auth_area.nonce_size = 0;
-	auth_area.attributes = 0;
-	auth_area.auth_size = 0;
+	tpm_buf_append_name(&buf, auth, pcr_idx, NULL);
+	tpm_buf_append_hmac_session(&buf, auth, 0, NULL, 0);
 
-	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
-	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
-		       sizeof(auth_area));
 	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
 
 	for (i = 0; i < chip->nr_allocated_banks; i++) {
@@ -262,7 +254,9 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 			       chip->allocated_banks[i].digest_size);
 	}
 
+	tpm_buf_fill_hmac_session(&buf, auth);
 	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 
 	tpm_buf_destroy(&buf);
 
-- 
2.35.3

[PATCH v2 08/11] tpm: add session encryption protection to tpm2_get_random()

[James Bottomley]

If some entity is snooping the TPM bus, they can see the random
numbers we're extracting from the TPM and do prediction attacks
against their consumers.  Foil this attack by using response
encryption to prevent the attacker from seeing the random sequence.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm2-cmd.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 001360d5956d..3ac438e8ffbb 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -289,29 +289,40 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	int total = 0;
 	int retries = 5;
 	u8 *dest_ptr = dest;
+	struct tpm2_auth *auth;
 
 	if (!num_bytes || max > TPM_MAX_RNG_DATA)
 		return -EINVAL;
 
-	err = tpm_buf_init(&buf, 0, 0);
+	err = tpm2_start_auth_session(chip, &auth);
 	if (err)
 		return err;
 
+	err = tpm_buf_init(&buf, 0, 0);
+	if (err) {
+		tpm2_end_auth_session(auth);
+		return err;
+	}
+
 	do {
-		tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
+		tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+		tpm_buf_append_hmac_session_opt(&buf, auth, TPM2_SA_ENCRYPT
+						| TPM2_SA_CONTINUE_SESSION,
+						NULL, 0);
 		tpm_buf_append_u16(&buf, num_bytes);
+		tpm_buf_fill_hmac_session(&buf, auth);
 		err = tpm_transmit_cmd(chip, &buf,
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
 				       "attempting get random");
+		err = tpm_buf_check_hmac_response(&buf, auth, err);
 		if (err) {
 			if (err > 0)
 				err = -EIO;
 			goto out;
 		}
 
-		out = (struct tpm2_get_random_out *)
-			&buf.data[TPM_HEADER_SIZE];
+		out = (struct tpm2_get_random_out *)tpm_buf_parameters(&buf);
 		recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
 		if (tpm_buf_length(&buf) <
 		    TPM_HEADER_SIZE +
@@ -328,6 +339,8 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	} while (retries-- && total < max);
 
 	tpm_buf_destroy(&buf);
+	tpm2_end_auth_session(auth);
+
 	return total ? total : -EIO;
 out:
 	tpm_buf_destroy(&buf);
-- 
2.35.3

[PATCH v2 09/11] KEYS: trusted: Add session encryption protection to the seal/unseal path

[James Bottomley]

If some entity is snooping the TPM bus, the can see the data going in
to be sealed and the data coming out as it is unsealed.  Add parameter
and response encryption to these cases to ensure that no secrets are
leaked even if the bus is snooped.

As part of doing this conversion it was discovered that policy
sessions can't work with HMAC protected authority because of missing
pieces (the tpm Nonce).  I've added code to work the same way as
before, which will result in potential authority exposure (while still
adding security for the command and the returned blob), and a fixme to
redo the API to get rid of this security hole.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

v2: fix unseal with policy and password
---
 security/keys/trusted-keys/trusted_tpm2.c | 85 ++++++++++++++++-------
 1 file changed, 61 insertions(+), 24 deletions(-)

diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 2b2c8eb258d5..0ed308d400da 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -230,6 +230,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	int blob_len = 0;
 	struct tpm_buf buf;
+	struct tpm2_auth *auth;
 	u32 hash;
 	u32 flags;
 	int i;
@@ -252,18 +253,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (rc)
 		return rc;
 
+	rc = tpm2_start_auth_session(chip, &auth);
+	if (rc)
+		goto out_put;
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 	if (rc) {
-		tpm_put_ops(chip);
-		return rc;
+		tpm2_end_auth_session(auth);
+		goto out_put;
 	}
 
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     0 /* session_attributes */,
-			     options->keyauth /* hmac */,
-			     TPM_DIGEST_SIZE);
+	tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
+	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_DECRYPT,
+				    options->keyauth, TPM_DIGEST_SIZE);
 
 	/* sensitive */
 	tpm_buf_append_u16(&buf, 4 + options->blobauth_len + payload->key_len);
@@ -308,7 +310,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
+	tpm_buf_fill_hmac_session(&buf, auth);
 	rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
+	if (rc)
+		goto out;
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 	if (rc)
 		goto out;
 
@@ -340,6 +346,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	else
 		payload->blob_len = blob_len;
 
+out_put:
 	tpm_put_ops(chip);
 	return rc;
 }
@@ -363,6 +370,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 			 u32 *blob_handle)
 {
 	struct tpm_buf buf;
+	struct tpm2_auth *auth;
 	unsigned int private_len;
 	unsigned int public_len;
 	unsigned int blob_len;
@@ -409,16 +417,19 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	if (blob_len > payload->blob_len)
 		return -E2BIG;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	rc = tpm2_start_auth_session(chip, &auth);
 	if (rc)
 		return rc;
 
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     0 /* session_attributes */,
-			     options->keyauth /* hmac */,
-			     TPM_DIGEST_SIZE);
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	if (rc) {
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
+
+	tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
+	tpm_buf_append_hmac_session(&buf, auth, 0, options->keyauth,
+				    TPM_DIGEST_SIZE);
 
 	tpm_buf_append(&buf, blob, blob_len);
 
@@ -427,7 +438,9 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
+	tpm_buf_fill_hmac_session(&buf, auth);
 	rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -461,24 +474,48 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   u32 blob_handle)
 {
 	struct tpm_buf buf;
+	struct tpm2_auth *auth;
 	u16 data_len;
 	u8 *data;
 	int rc;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+	rc = tpm2_start_auth_session(chip, &auth);
 	if (rc)
 		return rc;
 
-	tpm_buf_append_u32(&buf, blob_handle);
-	tpm2_buf_append_auth(&buf,
-			     options->policyhandle ?
-			     options->policyhandle : TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     TPM2_SA_CONTINUE_SESSION,
-			     options->blobauth /* hmac */,
-			     options->blobauth_len);
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+	if (rc) {
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
+
+	tpm_buf_append_name(&buf, auth, blob_handle, NULL);
+
+	if (!options->policyhandle) {
+		tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
+					    options->blobauth, TPM_DIGEST_SIZE);
+	} else {
+		/*
+		 * FIXME: The policy session was generated outside the
+		 * kernel so we don't known the nonce and thus can't
+		 * calculate a HMAC on it.  Therefore, the user can
+		 * only really use TPM2_PolicyPassword and we must
+		 * send down the plain text password, which could be
+		 * intercepted.  We can still encrypt the returned
+		 * key, but that's small comfort since the interposer
+		 * could repeat our actions with the exfiltrated
+		 * password.
+		 */
+		tpm2_buf_append_auth(&buf, options->policyhandle,
+				     NULL /* nonce */, 0, 0,
+				     options->blobauth, options->blobauth_len);
+		tpm_buf_append_hmac_session_opt(&buf, auth, TPM2_SA_ENCRYPT,
+						NULL, 0);
+	}
 
+	tpm_buf_fill_hmac_session(&buf, auth);
 	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 	if (rc > 0)
 		rc = -EPERM;
 
-- 
2.35.3

[PATCH v2 10/11] tpm: add the null key name as a sysfs export

[James Bottomley]

This is the last component of encrypted tpm2 session handling that
allows us to verify from userspace that the key derived from the NULL
seed genuinely belongs to the TPM and has not been spoofed.

The procedure for doing this involves creating an attestation identity
key (which requires verification of the TPM EK certificate) and then
using that AIK to sign a certification of the Elliptic Curve key over
the NULL seed.  Userspace must create this EC Key using the parameters
prescribed in TCG TPM v2.0 Provisioning Guidance for the SRK ECC; if
this is done correctly the names will match and the TPM can then run a
TPM2_Certify operation on this derived primary key using the newly
created AIK.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm-sysfs.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 54c71473aa29..27a16addab93 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -309,6 +309,19 @@ static ssize_t tpm_version_major_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(tpm_version_major);
 
+static ssize_t null_name_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct tpm_chip *chip = to_tpm_chip(dev);
+	int size = TPM2_NAME_SIZE;
+
+	bin2hex(buf, chip->tpmkeyname, size);
+	size *= 2;
+	buf[size++] = '\n';
+	return size;
+}
+static DEVICE_ATTR_RO(null_name);
+
 static struct attribute *tpm1_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
@@ -326,6 +339,7 @@ static struct attribute *tpm1_dev_attrs[] = {
 
 static struct attribute *tpm2_dev_attrs[] = {
 	&dev_attr_tpm_version_major.attr,
+	&dev_attr_null_name.attr,
 	NULL
 };
 
-- 
2.35.3

[PATCH v2 11/11] Documentation: add tpm-security.rst

[James Bottomley]

Document how the new encrypted secure interface for TPM2 works and how
security can be assured after boot by certifying the NULL seed.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 Documentation/security/tpm/tpm-security.rst | 214 ++++++++++++++++++++
 1 file changed, 214 insertions(+)
 create mode 100644 Documentation/security/tpm/tpm-security.rst

diff --git a/Documentation/security/tpm/tpm-security.rst b/Documentation/security/tpm/tpm-security.rst
new file mode 100644
index 000000000000..922b80db3d48
--- /dev/null
+++ b/Documentation/security/tpm/tpm-security.rst
@@ -0,0 +1,214 @@
+TPM Security
+============
+
+The object of this document is to describe how we make the kernel's
+use of the TPM reasonably robust in the face of external snooping and
+packet alteration attacks (called passive and active interposer attack
+in the literature).  The current security document is for TPM 2.0.
+
+Introduction
+------------
+
+The TPM is usually a discrete chip attached to a PC via some type of
+low bandwidth bus.  There are exceptions to this such as the Intel
+PTT, which is a software TPM running inside a software environment
+close to the CPU, which are subject to different attacks, but right at
+the moment, most hardened security environments require a discrete
+hardware TPM, which is the use case discussed here.
+
+Snooping and Alteration Attacks against the bus
+-----------------------------------------------
+
+The current state of the art for snooping the `TPM Genie`_ hardware
+interposer which is a simple external device that can be installed in
+a couple of seconds on any system or laptop.  Recently attacks were
+successfully demonstrated against the `Windows Bitlocker TPM`_ system.
+Most recently the same `attack against TPM based Linux disk
+encryption`_ schemes.  The next phase of research seems to be hacking
+existing devices on the bus to act as interposers, so the fact that
+the attacker requires physical access for a few seconds might
+evaporate.  However, the goal of this document is to protect TPM
+secrets and integrity as far as we are able in this environment and to
+try to insure that if we can't prevent the attack then at least we can
+detect it.
+
+Unfortunately, most of the TPM functionality, including the hardware
+reset capability can be controlled by an attacker who has access to
+the bus, so we'll discuss some of the disruption possibilities below.
+
+Measurement (PCR) Integrity
+---------------------------
+
+Since the attacker can send their own commands to the TPM, they can
+send arbitrary PCR extends and thus disrupt the measurement system,
+which would be an annoying denial of service attack.  However, there
+are two, more serious, classes of attack aimed at entities sealed to
+trust measurements.
+
+1. The attacker could intercept all PCR extends coming from the system
+   and completely substitute their own values, producing a replay of
+   an untampered state that would cause PCR measurements to attest to
+   a trusted state and release secrets
+
+2. At some point in time the attacker could reset the TPM, clearing
+   the PCRs and then send down their own measurements which would
+   effectively overwrite the boot time measurements the TPM has
+   already done.
+
+The first can be thwarted by always doing HMAC protection of the PCR
+extend and read command meaning measurement values cannot be
+substituted without producing a detectable HMAC failure in the
+response.  However, the second can only really be detected by relying
+on some sort of mechanism for protection which would change over TPM
+reset.
+
+Secrets Guarding
+----------------
+
+Certain information passing in and out of the TPM, such as key sealing
+and private key import and random number generation, is vulnerable to
+interception which HMAC protection alone cannot protect against, so
+for these types of command we must also employ request and response
+encryption to prevent the loss of secret information.
+
+Establishing Initial Trust with the TPM
+---------------------------------------
+
+In order to provide security from the beginning, an initial shared or
+asymmetric secret must be established which must also be unknown to
+the attacker.  The most obvious avenues for this are the endorsement
+and storage seeds, which can be used to derive asymmetric keys.
+However, using these keys is difficult because the only way to pass
+them into the kernel would be on the command line, which requires
+extensive support in the boot system, and there's no guarantee that
+either hierarchy would not have some type of authorization.
+
+The mechanism chosen for the Linux Kernel is to derive the primary
+elliptic curve key from the null seed using the standard storage seed
+parameters.  The null seed has two advantages: firstly the hierarchy
+physically cannot have an authorization, so we are always able to use
+it and secondly, the null seed changes across TPM resets, meaning if
+we establish trust on the null seed at start of day, all sessions
+salted with the derived key will fail if the TPM is reset and the seed
+changes.
+
+Obviously using the null seed without any other prior shared secrets,
+we have to create and read the initial public key which could, of
+course, be intercepted and substituted by the bus interposer.
+However, the TPM has a key certification mechanism (using the EK
+endorsement certificate, creating an attestation identity key and
+certifying the null seed primary with that key) which is too complex
+to run within the kernel, so we keep a copy of the null primary key
+name, which is what is exported via sysfs so user-space can run the
+full certification when it boots.  The definitive guarantee here is
+that if the null primary key certifies correctly, you know all your
+TPM transactions since start of day were secure and if it doesn't, you
+know there's an interposer on your system (and that any secret used
+during boot may have been leaked).
+
+Stacking Trust
+--------------
+
+In the current null primary scenario, the TPM must be completely
+cleared before handing it on to the next consumer.  However the kernel
+hands to user-space the name of the derived null seed key which can
+then be verified by certification in user-space.  Therefore, this chain
+of name handoff can be used between the various boot components as
+well (via an unspecified mechanism).  For instance, grub could use the
+null seed scheme for security and hand the name off to the kernel in
+the boot area.  The kernel could make its own derivation of the key
+and the name and know definitively that if they differ from the handed
+off version that tampering has occurred.  Thus it becomes possible to
+chain arbitrary boot components together (UEFI to grub to kernel) via
+the name handoff provided each successive component knows how to
+collect the name and verifies it against its derived key.
+
+Session Properties
+------------------
+
+All TPM commands the kernel uses allow sessions.  HMAC sessions may be
+used to check the integrity of requests and responses and decrypt and
+encrypt flags may be used to shield parameters and responses.  The
+HMAC and encryption keys are usually derived from the shared
+authorization secret, but for a lot of kernel operations that is well
+known (and usually empty).  Thus, every HMAC session used by the
+kernel must be created using the null primary key as the salt key
+which thus provides a cryptographic input into the session key
+derivation.  Thus, the kernel creates the null primary key once (as a
+volatile TPM handle) and keeps it around in a saved context stored in
+tpm_chip for every in-kernel use of the TPM.  Currently, because of a
+lack of de-gapping in the in-kernel resource manager, the session must
+be created and destroyed for each operation, but, in future, a single
+session may also be reused for the in-kernel HMAC, encryption and
+decryption sessions.
+
+Protection Types
+----------------
+
+For every in-kernel operation we use null primary salted HMAC to
+protect the integrity.  Additionally, we use parameter encryption to
+protect key sealing and parameter decryption to protect key unsealing
+and random number generation.
+
+Null Primary Key Certification in Userspace
+===========================================
+
+Every TPM comes shipped with a couple of X.509 certificates for the
+primary endorsement key.  This document assumes that the Elliptic
+Curve version of the certificate exists at 01C00002, but will work
+equally well with the RSA certificate (at 01C00001).
+
+The first step in the certification is primary creation using the
+template from the `TCG EK Credential Profile`_ which allows comparison
+of the generated primary key against the one in the certificate (the
+public key must match).  Note that generation of the EK primary
+requires the EK hierarchy password, but a pre-generated version of the
+EC primary should exist at 81010002 and a TPM2_ReadPublic() may be
+performed on this without needing the key authority.  Next, the
+certificate itself must be verified to chain back to the manufacturer
+root (which should be published on the manufacturer website).  Once
+this is done, an attestation key (AK) is generated within the TPM and
+it's name and the EK public key can be used to encrypt a secret using
+TPM2_MakeCredential.  The TPM then runs TPM2_ActivateCredential which
+will only recover the secret if the binding between the TPM, the EK
+and the AK is true. the generated AK may now be used to run a
+certification of the null primary key whose name the kernel has
+exported.  Since TPM2_MakeCredential/ActivateCredential are somewhat
+complicated, a more simplified process involving an externally
+generated private key is described below.
+
+This process is a simplified abbreviation of the usual privacy CA
+based attestation process.  The assumption here is that the
+attestation is done by the TPM owner who thus has access to only the
+owner hierarchy.  The owner creates an external public/private key
+pair (assume elliptic curve in this case) and wraps the private key
+for import using an inner wrapping process and parented to the EC
+derived storage primary.  The TPM2_Import() is done using a parameter
+decryption HMAC session salted to the EK primary (which also does not
+require the EK key authority) meaning that the inner wrapping key is
+the encrypted parameter and thus the TPM will not be able to perform
+the import unless is possesses the certified EK so if the command
+succeeds and the HMAC verifies on return we know we have a loadable
+copy of the private key only for the certified TPM.  This key is now
+loaded into the TPM and the Storage primary flushed (to free up space
+for the null key generation).
+
+The null EC primary is now generated using the Storage profile
+outlined in the `TCG TPM v2.0 Provisioning Guidance`_; the name of
+this key (the hash of the public area) is computed and compared to the
+null seed name presented by the kernel in
+/sys/class/tpm/tpm0/null_name.  If the names do not match, the TPM is
+compromised.  If the names match, the user performs a TPM2_Certify()
+using the null primary as the object handle and the loaded private key
+as the sign handle and providing randomized qualifying data.  The
+signature of the returned certifyInfo is verified against the public
+part of the loaded private key and the qualifying data checked to
+prevent replay.  If all of these tests pass, the user is now assured
+that TPM integrity and privacy was preserved across the entire boot
+sequence of this kernel.
+
+.. _TPM Genie: https://www.nccgroup.trust/globalassets/about-us/us/documents/tpm-genie.pdf
+.. _Windows Bitlocker TPM: https://dolosgroup.io/blog/2021/7/9/from-stolen-laptop-to-inside-the-company-network
+.. _attack against TPM based Linux disk encryption: https://www.secura.com/blog/tpm-sniffing-attacks-against-non-bitlocker-targets
+.. _TCG EK Credential Profile: https://trustedcomputinggroup.org/resource/tcg-ek-credential-profile-for-tpm-family-2-0/
+.. _TCG TPM v2.0 Provisioning Guidance: https://trustedcomputinggroup.org/resource/tcg-tpm-v2-0-provisioning-guidance/
-- 
2.35.3

Re: [PATCH v2 01/11] tpm: move buffer handling from static inlines to real functions

[kernel test robot]

Hi James,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-20230124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
patch link:    https://lore.kernel.org/r/20230124175516.5984-2-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH v2 01/11] tpm: move buffer handling from static inlines to real functions
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230125/202301250315.ZgtsNzSm-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/484b879d0bfceec899fea147a76d12f7072d9f23
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
        git checkout 484b879d0bfceec899fea147a76d12f7072d9f23
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/char/tpm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/char/tpm/tpm-buf.c:46:5: warning: no previous prototype for 'tpm_buf_tag' [-Wmissing-prototypes]
      46 | u16 tpm_buf_tag(struct tpm_buf *buf)
         |     ^~~~~~~~~~~


vim +/tpm_buf_tag +46 drivers/char/tpm/tpm-buf.c

    45	
  > 46	u16 tpm_buf_tag(struct tpm_buf *buf)
    47	{
    48		struct tpm_header *head = (struct tpm_header *)buf->data;
    49	
    50		return be16_to_cpu(head->tag);
    51	}
    52	EXPORT_SYMBOL_GPL(tpm_buf_tag);
    53	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[kernel test robot]

Hi James,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-20230124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
patch link:    https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230125/202301250432.kZ4b6794-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
        git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/char/tpm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
    1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
         |     ^~~~~~~~~~~~~~~~~~~~~~~~
--
>> drivers/char/tpm/tpm2-sessions.c:352: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * tpm_buf_append_hmac_session() append a TPM session element


vim +/tpm2_create_null_primary +1184 drivers/char/tpm/tpm2-sessions.c

  1183	
> 1184	int tpm2_create_null_primary(struct tpm_chip *chip) {
  1185		u32 nullkey;
  1186		int rc;
  1187	
  1188		rc = tpm2_create_primary(chip, TPM2_RH_NULL, &nullkey);
  1189	
  1190		if (rc == TPM2_RC_SUCCESS) {
  1191			unsigned int offset = 0; /* dummy offset for tpmkeycontext */
  1192	
  1193			rc = tpm2_save_context(chip, nullkey, chip->tpmkeycontext,
  1194					       sizeof(chip->tpmkeycontext), &offset);
  1195			tpm2_flush_context(chip, nullkey);
  1196		}
  1197	
  1198		return rc;
  1199	}
  1200	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[kernel test robot]

Hi James,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-20230124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
patch link:    https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20230125/202301250706.deGvd0yq-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
        git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/char/tpm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
    1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
         |     ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/char/tpm/tpm2-sessions.c: In function 'tpm_buf_check_hmac_response':
>> drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame size of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
     831 | }
         | ^
   drivers/char/tpm/tpm2-sessions.c: In function 'tpm_buf_fill_hmac_session':
   drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame size of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
     579 | }
         | ^


vim +831 drivers/char/tpm/tpm2-sessions.c

   676	
   677	/**
   678	 * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
   679	 * @buf: the original command buffer (which now contains the response)
   680	 * @auth: the auth structure allocated by tpm2_start_auth_session()
   681	 * @rc: the return code from tpm_transmit_cmd
   682	 *
   683	 * If @rc is non zero, @buf may not contain an actual return, so @rc
   684	 * is passed through as the return and the session cleaned up and
   685	 * de-allocated if required (this is required if
   686	 * TPM2_SA_CONTINUE_SESSION was not specified as a session flag).
   687	 *
   688	 * If @rc is zero, the response HMAC is computed against the returned
   689	 * @buf and matched to the TPM one in the session area.  If there is a
   690	 * mismatch, an error is logged and -EINVAL returned.
   691	 *
   692	 * The reason for this is that the command issue and HMAC check
   693	 * sequence should look like:
   694	 *
   695	 *	rc = tpm_transmit_cmd(...);
   696	 *	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
   697	 *	if (rc)
   698	 *		...
   699	 *
   700	 * Which is easily layered into the current contrl flow.
   701	 *
   702	 * Returns: 0 on success or an error.
   703	 */
   704	int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
   705					int rc)
   706	{
   707		struct tpm_header *head = (struct tpm_header *)buf->data;
   708		struct tpm_chip *chip = auth->chip;
   709		const u8 *s, *p;
   710		u8 rphash[SHA256_DIGEST_SIZE];
   711		u32 attrs;
   712		SHASH_DESC_ON_STACK(desc, sha256_hash);
   713		u16 tag = be16_to_cpu(head->tag);
   714		u32 cc = be32_to_cpu(auth->ordinal);
   715		int parm_len, len, i, handles;
   716	
   717		if (auth->session >= TPM_HEADER_SIZE) {
   718			WARN(1, "tpm session not filled correctly\n");
   719			goto out;
   720		}
   721	
   722		if (rc != 0)
   723			/* pass non success rc through and close the session */
   724			goto out;
   725	
   726		rc = -EINVAL;
   727		if (tag != TPM2_ST_SESSIONS) {
   728			dev_err(&chip->dev, "TPM: HMAC response check has no sessions tag\n");
   729			goto out;
   730		}
   731	
   732		i = tpm2_find_cc(chip, cc);
   733		if (i < 0)
   734			goto out;
   735		attrs = chip->cc_attrs_tbl[i];
   736		handles = (attrs >> TPM2_CC_ATTR_RHANDLE) & 1;
   737	
   738		/* point to area beyond handles */
   739		s = &buf->data[TPM_HEADER_SIZE + handles * 4];
   740		parm_len = tpm_get_inc_u32(&s);
   741		p = s;
   742		s += parm_len;
   743		/* skip over any sessions before ours */
   744		for (i = 0; i < auth->session - 1; i++) {
   745			len = tpm_get_inc_u16(&s);
   746			s += len + 1;
   747			len = tpm_get_inc_u16(&s);
   748			s += len;
   749		}
   750		/* TPM nonce */
   751		len = tpm_get_inc_u16(&s);
   752		if (s - buf->data + len > tpm_buf_length(buf))
   753			goto out;
   754		if (len != SHA256_DIGEST_SIZE)
   755			goto out;
   756		memcpy(auth->tpm_nonce, s, len);
   757		s += len;
   758		attrs = *s++;
   759		len = tpm_get_inc_u16(&s);
   760		if (s - buf->data + len != tpm_buf_length(buf))
   761			goto out;
   762		if (len != SHA256_DIGEST_SIZE)
   763			goto out;
   764		/*
   765		 * s points to the HMAC. now calculate comparison, beginning
   766		 * with rphash
   767		 */
   768		desc->tfm = sha256_hash;
   769		crypto_shash_init(desc);
   770		/* yes, I know this is now zero, but it's what the standard says */
   771		crypto_shash_update(desc, (u8 *)&head->return_code,
   772				    sizeof(head->return_code));
   773		/* ordinal is already BE */
   774		crypto_shash_update(desc, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
   775		crypto_shash_update(desc, p, parm_len);
   776		crypto_shash_final(desc, rphash);
   777	
   778		/* now calculate the hmac */
   779		hmac_init(desc, auth->session_key, sizeof(auth->session_key)
   780			  + auth->passphraselen);
   781		crypto_shash_update(desc, rphash, sizeof(rphash));
   782		crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
   783		crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
   784		crypto_shash_update(desc, &auth->attrs, 1);
   785		/* we're done with the rphash, so put our idea of the hmac there */
   786		hmac_final(desc, auth->session_key, sizeof(auth->session_key)
   787			   + auth->passphraselen, rphash);
   788		if (memcmp(rphash, s, SHA256_DIGEST_SIZE) == 0) {
   789			rc = 0;
   790		} else {
   791			dev_err(&auth->chip->dev, "TPM: HMAC check failed\n");
   792			goto out;
   793		}
   794	
   795		/* now do response decryption */
   796		if (auth->attrs & TPM2_SA_ENCRYPT) {
   797			struct scatterlist sg[1];
   798			SYNC_SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
   799			DECLARE_CRYPTO_WAIT(wait);
   800	
   801			skcipher_request_set_sync_tfm(req, auth->aes);
   802			skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
   803						      crypto_req_done, &wait);
   804	
   805			/* need key and IV */
   806			KDFa(auth->session_key, SHA256_DIGEST_SIZE
   807			     + auth->passphraselen, "CFB", auth->tpm_nonce,
   808			     auth->our_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
   809			     auth->scratch);
   810			crypto_sync_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
   811			len = tpm_get_inc_u16(&p);
   812			sg_init_one(sg, p, len);
   813			skcipher_request_set_crypt(req, sg, sg, len,
   814						   auth->scratch + AES_KEYBYTES);
   815			crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
   816		}
   817	
   818	 out:
   819		if ((auth->attrs & TPM2_SA_CONTINUE_SESSION) == 0) {
   820			/* manually close the session if it wasn't consumed */
   821			if (rc)
   822				tpm2_flush_context(chip, auth->handle);
   823			crypto_free_sync_skcipher(auth->aes);
   824			kfree(auth);
   825		} else {
   826			/* reset for next use  */
   827			auth->session = TPM_HEADER_SIZE;
   828		}
   829	
   830		return rc;
 > 831	}
   832	EXPORT_SYMBOL(tpm_buf_check_hmac_response);
   833	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[kernel test robot]

Hi James,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-20230124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
patch link:    https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code
config: i386-randconfig-s002-20230123 (https://download.01.org/0day-ci/archive/20230125/202301251326.r0t0nGZc-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
        git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 olddefconfig
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/char/tpm/tpm2-sessions.c:1184:5: sparse: sparse: symbol 'tpm2_create_null_primary' was not declared. Should it be static?

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[James Bottomley]

On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> Hi James,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on char-misc/char-misc-testing]
> [also build test WARNING on char-misc/char-misc-next char-misc/char-
> misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-
> 20230124]
> [If your patch is applied to the wrong git tree, kindly drop us a
> note.
> And when submitting patch, we suggest to use '--base' as documented
> in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:   
> https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> patch link:   
> https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> encrypt/decrypt session handling code
> config: arc-allyesconfig
> (https://download.01.org/0day-ci/archive/20230125/202301250706.deGvd0
> yq-lkp@intel.com/config)
> compiler: arceb-elf-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
>         wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>  -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         #
> https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
>         git remote add linux-review
> https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review James-Bottomley/tpm-move-
> buffer-handling-from-static-inlines-to-real-functions/20230125-020146
>         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> make.cross W=1 O=build_dir ARCH=arc olddefconfig
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/char/tpm/
> 
> If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous
> prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
>     1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
>          |     ^~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/char/tpm/tpm2-sessions.c: In function
> 'tpm_buf_check_hmac_response':
> > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame size
> > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>      831 | }
>          | ^
>    drivers/char/tpm/tpm2-sessions.c: In function
> 'tpm_buf_fill_hmac_session':
>    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame size of
> 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>      579 | }
>          | ^

Is this a test problem?  I can't see why the code would only blow the
stack on the arc architecture and not on any other ... does it have
something funny with on stack crypto structures?

James

Re: [PATCH v2 01/11] tpm: move buffer handling from static inlines to real functions

[James Bottomley]

On Wed, 2023-01-25 at 03:57 +0800, kernel test robot wrote:
> Hi James,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on char-misc/char-misc-testing]
> [also build test WARNING on char-misc/char-misc-next char-misc/char-
> misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-
> 20230124]
> [If your patch is applied to the wrong git tree, kindly drop us a
> note.
> And when submitting patch, we suggest to use '--base' as documented
> in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:   
> https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> patch link:   
> https://lore.kernel.org/r/20230124175516.5984-2-James.Bottomley%40HansenPartnership.com
> patch subject: [PATCH v2 01/11] tpm: move buffer handling from static
> inlines to real functions
> config: sparc-allyesconfig
> (https://download.01.org/0day-ci/archive/20230125/202301250315.ZgtsNz
> Sm-lkp@intel.com/config)
> compiler: sparc64-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
>         wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>  -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         #
> https://github.com/intel-lab-lkp/linux/commit/484b879d0bfceec899fea147a76d12f7072d9f23
>         git remote add linux-review
> https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review James-Bottomley/tpm-move-
> buffer-handling-from-static-inlines-to-real-functions/20230125-020146
>         git checkout 484b879d0bfceec899fea147a76d12f7072d9f23
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> make.cross W=1 O=build_dir ARCH=sparc olddefconfig
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash
> drivers/char/tpm/
> 
> If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> > > drivers/char/tpm/tpm-buf.c:46:5: warning: no previous prototype
> > > for 'tpm_buf_tag' [-Wmissing-prototypes]
>       46 | u16 tpm_buf_tag(struct tpm_buf *buf)
>          |     ^~~~~~~~~~~


This looks like an initial header got missed in the code motion. 
However, the fact that nothing broke shows that tpm_buf_tag isn't used
outside of the tpm-buf.c functions, so I think I can make it static to
them.

James

Re: [PATCH v2 09/11] KEYS: trusted: Add session encryption protection to the seal/unseal path

[Ben Boeckel]

On Tue, Jan 24, 2023 at 12:55:14 -0500, James Bottomley wrote:
> If some entity is snooping the TPM bus, the can see the data going in
                                          ^^^ they
> to be sealed and the data coming out as it is unsealed.  Add parameter
> and response encryption to these cases to ensure that no secrets are
> leaked even if the bus is snooped.
> 
> As part of doing this conversion it was discovered that policy
> sessions can't work with HMAC protected authority because of missing
> pieces (the tpm Nonce).  I've added code to work the same way as
> before, which will result in potential authority exposure (while still
> adding security for the command and the returned blob), and a fixme to
> redo the API to get rid of this security hole.

--Ben

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Yujie Liu]

Hi James,

On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > Hi James,
> > 
> > I love your patch! Perhaps something to improve:
> > 
> > [auto build test WARNING on char-misc/char-misc-testing]
> > [also build test WARNING on char-misc/char-misc-next char-misc/char-
> > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-
> > 20230124]
> > [If your patch is applied to the wrong git tree, kindly drop us a
> > note.
> > And when submitting patch, we suggest to use '--base' as documented
> > in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:   
> > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > patch link:   
> > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > encrypt/decrypt session handling code
> > config: arc-allyesconfig
> > (https://download.01.org/0day-ci/archive/20230125/202301250706.deGvd0
> > yq-lkp@intel.com/config)
> > compiler: arceb-elf-gcc (GCC) 12.1.0
> > reproduce (this is a W=1 build):
> >         wget
> > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> >  -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         #
> > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> >         git remote add linux-review
> > https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review James-Bottomley/tpm-move-
> > buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/char/tpm/
> > 
> > If you fix the issue, kindly add following tag where applicable
> > > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous
> > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
> >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> >    drivers/char/tpm/tpm2-sessions.c: In function
> > 'tpm_buf_check_hmac_response':
> > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame size
> > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> >      831 | }
> >          | ^
> >    drivers/char/tpm/tpm2-sessions.c: In function
> > 'tpm_buf_fill_hmac_session':
> >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame size of
> > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> >      579 | }
> >          | ^
> 
> Is this a test problem?  I can't see why the code would only blow the
> stack on the arc architecture and not on any other ... does it have
> something funny with on stack crypto structures?

This warning is controlled by the value of CONFIG_FRAME_WARN.

For "make ARCH=arc allyesconfig", the default value is 1024, so this
frame warning shows up during the build.

For other arch such as "make ARCH=x86_64 allyesconfig", the default
value would be 2048 and won't have this warning.

Not sure if this is a real problem that need to be fixed, here just
providing above information for your reference.

--
Best Regards,
Yujie

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Jarkko Sakkinen]

On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> Hi James,
> 
> On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > Hi James,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on char-misc/char-misc-testing]
> > > [also build test WARNING on char-misc/char-misc-next char-misc/char-
> > > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-
> > > 20230124]
> > > [If your patch is applied to the wrong git tree, kindly drop us a
> > > note.
> > > And when submitting patch, we suggest to use '--base' as documented
> > > in
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > 
> > > url:   
> > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > patch link:   
> > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > encrypt/decrypt session handling code
> > > config: arc-allyesconfig
> > > (https://download.01.org/0day-ci/archive/20230125/202301250706.deGvd0
> > > yq-lkp@intel.com/config)
> > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > reproduce (this is a W=1 build):
> > >         wget
> > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > >  -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         #
> > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > >         git remote add linux-review
> > > https://github.com/intel-lab-lkp/linux
> > >         git fetch --no-tags linux-review James-Bottomley/tpm-move-
> > > buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > >         # save the config file
> > >         mkdir build_dir && cp config build_dir/.config
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/char/tpm/
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous
> > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
> > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > 'tpm_buf_check_hmac_response':
> > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame size
> > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > >      831 | }
> > >          | ^
> > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > 'tpm_buf_fill_hmac_session':
> > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame size of
> > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > >      579 | }
> > >          | ^
> > 
> > Is this a test problem?  I can't see why the code would only blow the
> > stack on the arc architecture and not on any other ... does it have
> > something funny with on stack crypto structures?
> 
> This warning is controlled by the value of CONFIG_FRAME_WARN.
> 
> For "make ARCH=arc allyesconfig", the default value is 1024, so this
> frame warning shows up during the build.
> 
> For other arch such as "make ARCH=x86_64 allyesconfig", the default
> value would be 2048 and won't have this warning.
> 
> Not sure if this is a real problem that need to be fixed, here just
> providing above information for your reference.
> 
> --
> Best Regards,
> Yujie

*Must* be fixed given that it is how the default value is set now. This
is wrong place to reconsider.

And we do not want to add functions that bloat the stack this way.

Shash just needs to be allocated from heap instead of stack.

BR, Jarkko

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[James Bottomley]

On Fri, 2023-02-03 at 14:06 +0800, Yujie Liu wrote:
> Hi James,
> 
> On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > Hi James,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on char-misc/char-misc-testing]
> > > [also build test WARNING on char-misc/char-misc-next char-
> > > misc/char-
> > > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5
> > > next-
> > > 20230124]
> > > [If your patch is applied to the wrong git tree, kindly drop us a
> > > note.
> > > And when submitting patch, we suggest to use '--base' as
> > > documented
> > > in
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > 
> > > url:   
> > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > patch link:   
> > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > encrypt/decrypt session handling code
> > > config: arc-allyesconfig
> > > (
> > > https://download.01.org/0day-ci/archive/20230125/202301250706.deGv
> > > d0
> > > yq-lkp@intel.com/config)
> > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > reproduce (this is a W=1 build):
> > >         wget
> > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > >  -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         #
> > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > >         git remote add linux-review
> > > https://github.com/intel-lab-lkp/linux
> > >         git fetch --no-tags linux-review James-Bottomley/tpm-
> > > move-
> > > buffer-handling-from-static-inlines-to-real-functions/20230125-
> > > 020146
> > >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > >         # save the config file
> > >         mkdir build_dir && cp config build_dir/.config
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> > > drivers/char/tpm/
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous
> > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
> > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > 'tpm_buf_check_hmac_response':
> > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame
> > > > > size
> > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > than=]
> > >      831 | }
> > >          | ^
> > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > 'tpm_buf_fill_hmac_session':
> > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame
> > > size of
> > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > >      579 | }
> > >          | ^
> > 
> > Is this a test problem?  I can't see why the code would only blow
> > the stack on the arc architecture and not on any other ... does it
> > have something funny with on stack crypto structures?
> 
> This warning is controlled by the value of CONFIG_FRAME_WARN.
> 
> For "make ARCH=arc allyesconfig", the default value is 1024, so this
> frame warning shows up during the build.
> 
> For other arch such as "make ARCH=x86_64 allyesconfig", the default
> value would be 2048 and won't have this warning.
> 
> Not sure if this is a real problem that need to be fixed, here just
> providing above information for your reference.

Is there something weird about arc, though, since a quick trawl through
defconfigs shows quite a few systems (including i386) have it set to
1024:

mips/configs/lemote2f_defconfig:CONFIG_FRAME_WARN=1024
mips/configs/loongson2k_defconfig:CONFIG_FRAME_WARN=1024
powerpc/configs/fsl-emb-nonhw.config:CONFIG_FRAME_WARN=1024
um/configs/x86_64_defconfig:CONFIG_FRAME_WARN=1024
x86/configs/i386_defconfig:CONFIG_FRAME_WARN=1024

That also seems to show, by the way, that the default on arc should be
the standard 2048.

James

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[James Bottomley]

On Wed, 2023-02-08 at 04:49 +0200, Jarkko Sakkinen wrote:
> On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> > Hi James,
> > 
> > On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > > Hi James,
> > > > 
> > > > I love your patch! Perhaps something to improve:
> > > > 
> > > > [auto build test WARNING on char-misc/char-misc-testing]
> > > > [also build test WARNING on char-misc/char-misc-next char-
> > > > misc/char-
> > > > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5
> > > > next-
> > > > 20230124]
> > > > [If your patch is applied to the wrong git tree, kindly drop us
> > > > a
> > > > note.
> > > > And when submitting patch, we suggest to use '--base' as
> > > > documented
> > > > in
> > > > https://git-scm.com/docs/git-format-patch#_base_tree_information
> > > > ]
> > > > 
> > > > url:   
> > > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > > patch link:   
> > > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > > encrypt/decrypt session handling code
> > > > config: arc-allyesconfig
> > > > (
> > > > https://download.01.org/0day-ci/archive/20230125/202301250706.de
> > > > Gvd0
> > > > yq-lkp@intel.com/config)
> > > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > > reproduce (this is a W=1 build):
> > > >         wget
> > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > >  -O ~/bin/make.cross
> > > >         chmod +x ~/bin/make.cross
> > > >         #
> > > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > >         git remote add linux-review
> > > > https://github.com/intel-lab-lkp/linux
> > > >         git fetch --no-tags linux-review James-Bottomley/tpm-
> > > > move-
> > > > buffer-handling-from-static-inlines-to-real-functions/20230125-
> > > > 020146
> > > >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > >         # save the config file
> > > >         mkdir build_dir && cp config build_dir/.config
> > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> > > > drivers/char/tpm/
> > > > 
> > > > If you fix the issue, kindly add following tag where applicable
> > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > 
> > > > All warnings (new ones prefixed by >>):
> > > > 
> > > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no
> > > > previous
> > > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip)
> > > > {
> > > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > 'tpm_buf_check_hmac_response':
> > > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame
> > > > > > size
> > > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > > than=]
> > > >      831 | }
> > > >          | ^
> > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > 'tpm_buf_fill_hmac_session':
> > > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame
> > > > size of
> > > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > >      579 | }
> > > >          | ^
> > > 
> > > Is this a test problem?  I can't see why the code would only blow
> > > the
> > > stack on the arc architecture and not on any other ... does it
> > > have
> > > something funny with on stack crypto structures?
> > 
> > This warning is controlled by the value of CONFIG_FRAME_WARN.
> > 
> > For "make ARCH=arc allyesconfig", the default value is 1024, so
> > this frame warning shows up during the build.
> > 
> > For other arch such as "make ARCH=x86_64 allyesconfig", the default
> > value would be 2048 and won't have this warning.
> > 
> > Not sure if this is a real problem that need to be fixed, here just
> > providing above information for your reference.
> > 
> > --
> > Best Regards,
> > Yujie
> 
> *Must* be fixed given that it is how the default value is set now.
> This is wrong place to reconsider.
>
> 
> And we do not want to add functions that bloat the stack this way.
> 
> Shash just needs to be allocated from heap instead of stack.

On x86_64 the stack usage is measured at 984 bytes, so rather than
jumping to conclusions let's root cause why this is a problem only on
the arc architecture.  I suspect it's something to do with the
alignment constraints of shash.  I've also noted it shouldn't actually
warn on arc because the default stack warning size there should be 2048
(like x86_64).

James

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Jarkko Sakkinen]

On Fri, Feb 10, 2023 at 09:48:15AM -0500, James Bottomley wrote:
> On Wed, 2023-02-08 at 04:49 +0200, Jarkko Sakkinen wrote:
> > On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> > > Hi James,
> > > 
> > > On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > > > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > > > Hi James,
> > > > > 
> > > > > I love your patch! Perhaps something to improve:
> > > > > 
> > > > > [auto build test WARNING on char-misc/char-misc-testing]
> > > > > [also build test WARNING on char-misc/char-misc-next char-
> > > > > misc/char-
> > > > > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5
> > > > > next-
> > > > > 20230124]
> > > > > [If your patch is applied to the wrong git tree, kindly drop us
> > > > > a
> > > > > note.
> > > > > And when submitting patch, we suggest to use '--base' as
> > > > > documented
> > > > > in
> > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information
> > > > > ]
> > > > > 
> > > > > url:   
> > > > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > > > patch link:   
> > > > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > > > encrypt/decrypt session handling code
> > > > > config: arc-allyesconfig
> > > > > (
> > > > > https://download.01.org/0day-ci/archive/20230125/202301250706.de
> > > > > Gvd0
> > > > > yq-lkp@intel.com/config)
> > > > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > > > reproduce (this is a W=1 build):
> > > > >         wget
> > > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > > >  -O ~/bin/make.cross
> > > > >         chmod +x ~/bin/make.cross
> > > > >         #
> > > > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > >         git remote add linux-review
> > > > > https://github.com/intel-lab-lkp/linux
> > > > >         git fetch --no-tags linux-review James-Bottomley/tpm-
> > > > > move-
> > > > > buffer-handling-from-static-inlines-to-real-functions/20230125-
> > > > > 020146
> > > > >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > >         # save the config file
> > > > >         mkdir build_dir && cp config build_dir/.config
> > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> > > > > drivers/char/tpm/
> > > > > 
> > > > > If you fix the issue, kindly add following tag where applicable
> > > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > 
> > > > > All warnings (new ones prefixed by >>):
> > > > > 
> > > > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no
> > > > > previous
> > > > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > > > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip)
> > > > > {
> > > > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > 'tpm_buf_check_hmac_response':
> > > > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame
> > > > > > > size
> > > > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > > > than=]
> > > > >      831 | }
> > > > >          | ^
> > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > 'tpm_buf_fill_hmac_session':
> > > > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame
> > > > > size of
> > > > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > > >      579 | }
> > > > >          | ^
> > > > 
> > > > Is this a test problem?  I can't see why the code would only blow
> > > > the
> > > > stack on the arc architecture and not on any other ... does it
> > > > have
> > > > something funny with on stack crypto structures?
> > > 
> > > This warning is controlled by the value of CONFIG_FRAME_WARN.
> > > 
> > > For "make ARCH=arc allyesconfig", the default value is 1024, so
> > > this frame warning shows up during the build.
> > > 
> > > For other arch such as "make ARCH=x86_64 allyesconfig", the default
> > > value would be 2048 and won't have this warning.
> > > 
> > > Not sure if this is a real problem that need to be fixed, here just
> > > providing above information for your reference.
> > > 
> > > --
> > > Best Regards,
> > > Yujie
> > 
> > *Must* be fixed given that it is how the default value is set now.
> > This is wrong place to reconsider.
> >
> > 
> > And we do not want to add functions that bloat the stack this way.
> > 
> > Shash just needs to be allocated from heap instead of stack.
> 
> On x86_64 the stack usage is measured at 984 bytes, so rather than
> jumping to conclusions let's root cause why this is a problem only on
> the arc architecture.  I suspect it's something to do with the
> alignment constraints of shash.  I've also noted it shouldn't actually
> warn on arc because the default stack warning size there should be 2048
> (like x86_64).

Would it such a big deal to allocate shash from heap? That would
be IMHO more robust in the end.

BR, Jarkko

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Yujie Liu]

On Mon, Feb 13, 2023 at 09:45:40AM +0200, Jarkko Sakkinen wrote:
> On Fri, Feb 10, 2023 at 09:48:15AM -0500, James Bottomley wrote:
> > On Wed, 2023-02-08 at 04:49 +0200, Jarkko Sakkinen wrote:
> > > On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> > > > Hi James,
> > > > 
> > > > On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > > > > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > > > > 
> > > > > > If you fix the issue, kindly add following tag where applicable
> > > > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > > 
> > > > > > All warnings (new ones prefixed by >>):
> > > > > > 
> > > > > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no
> > > > > > previous
> > > > > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > > > > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip)
> > > > > > {
> > > > > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > 'tpm_buf_check_hmac_response':
> > > > > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame
> > > > > > > > size
> > > > > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > > > > than=]
> > > > > >      831 | }
> > > > > >          | ^
> > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > 'tpm_buf_fill_hmac_session':
> > > > > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame
> > > > > > size of
> > > > > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > > > >      579 | }
> > > > > >          | ^
> > > > > 
> > > > > Is this a test problem?  I can't see why the code would only blow
> > > > > the
> > > > > stack on the arc architecture and not on any other ... does it
> > > > > have
> > > > > something funny with on stack crypto structures?
> > > > 
> > > > This warning is controlled by the value of CONFIG_FRAME_WARN.
> > > > 
> > > > For "make ARCH=arc allyesconfig", the default value is 1024, so
> > > > this frame warning shows up during the build.
> > > > 
> > > > For other arch such as "make ARCH=x86_64 allyesconfig", the default
> > > > value would be 2048 and won't have this warning.
> > > > 
> > > > Not sure if this is a real problem that need to be fixed, here just
> > > > providing above information for your reference.
> > > > 
> > > > --
> > > > Best Regards,
> > > > Yujie
> > > 
> > > *Must* be fixed given that it is how the default value is set now.
> > > This is wrong place to reconsider.
> > >
> > > 
> > > And we do not want to add functions that bloat the stack this way.
> > > 
> > > Shash just needs to be allocated from heap instead of stack.
> > 
> > On x86_64 the stack usage is measured at 984 bytes, so rather than
> > jumping to conclusions let's root cause why this is a problem only on
> > the arc architecture.  I suspect it's something to do with the
> > alignment constraints of shash.  I've also noted it shouldn't actually
> > warn on arc because the default stack warning size there should be 2048
> > (like x86_64).

The stack usage varies on different architectures, so does the default
value of CONFIG_FRAME_WARN. The warning shows up when the stack usage
exceeds the default warning size.

For arc arch, I reconfirmed that the default stack warning size is 1024
no matter allyesconfig or defconfig, while the usage could reach 1132
bytes.

$ make ARCH=arc allyesconfig
$ grep FRAME_WARN .config
CONFIG_FRAME_WARN=1024

$ make ARCH=arc defconfig
$ grep FRAME_WARN .config
CONFIG_FRAME_WARN=1024

> Would it such a big deal to allocate shash from heap? That would
> be IMHO more robust in the end.

Thanks Jarkko for the suggestion. This would be a faster and better fix
without having to look into this arc-specific problem.

Best Regards,
Yujie
 
> BR, Jarkko

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Ard Biesheuvel]

On Mon, 13 Feb 2023 at 08:45, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Fri, Feb 10, 2023 at 09:48:15AM -0500, James Bottomley wrote:
> > On Wed, 2023-02-08 at 04:49 +0200, Jarkko Sakkinen wrote:
> > > On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> > > > Hi James,
> > > >
> > > > On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > > > > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > > > > Hi James,
> > > > > >
> > > > > > I love your patch! Perhaps something to improve:
> > > > > >
> > > > > > [auto build test WARNING on char-misc/char-misc-testing]
> > > > > > [also build test WARNING on char-misc/char-misc-next char-
> > > > > > misc/char-
> > > > > > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5
> > > > > > next-
> > > > > > 20230124]
> > > > > > [If your patch is applied to the wrong git tree, kindly drop us
> > > > > > a
> > > > > > note.
> > > > > > And when submitting patch, we suggest to use '--base' as
> > > > > > documented
> > > > > > in
> > > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information
> > > > > > ]
> > > > > >
> > > > > > url:
> > > > > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > > > > patch link:
> > > > > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > > > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > > > > encrypt/decrypt session handling code
> > > > > > config: arc-allyesconfig
> > > > > > (
> > > > > > https://download.01.org/0day-ci/archive/20230125/202301250706.de
> > > > > > Gvd0
> > > > > > yq-lkp@intel.com/config)
> > > > > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > > > > reproduce (this is a W=1 build):
> > > > > >         wget
> > > > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > > > >  -O ~/bin/make.cross
> > > > > >         chmod +x ~/bin/make.cross
> > > > > >         #
> > > > > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > >         git remote add linux-review
> > > > > > https://github.com/intel-lab-lkp/linux
> > > > > >         git fetch --no-tags linux-review James-Bottomley/tpm-
> > > > > > move-
> > > > > > buffer-handling-from-static-inlines-to-real-functions/20230125-
> > > > > > 020146
> > > > > >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > >         # save the config file
> > > > > >         mkdir build_dir && cp config build_dir/.config
> > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> > > > > > drivers/char/tpm/
> > > > > >
> > > > > > If you fix the issue, kindly add following tag where applicable
> > > > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > >
> > > > > > All warnings (new ones prefixed by >>):
> > > > > >
> > > > > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no
> > > > > > previous
> > > > > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > > > > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip)
> > > > > > {
> > > > > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > 'tpm_buf_check_hmac_response':
> > > > > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame
> > > > > > > > size
> > > > > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > > > > than=]
> > > > > >      831 | }
> > > > > >          | ^
> > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > 'tpm_buf_fill_hmac_session':
> > > > > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame
> > > > > > size of
> > > > > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > > > >      579 | }
> > > > > >          | ^
> > > > >
> > > > > Is this a test problem?  I can't see why the code would only blow
> > > > > the
> > > > > stack on the arc architecture and not on any other ... does it
> > > > > have
> > > > > something funny with on stack crypto structures?
> > > >
> > > > This warning is controlled by the value of CONFIG_FRAME_WARN.
> > > >
> > > > For "make ARCH=arc allyesconfig", the default value is 1024, so
> > > > this frame warning shows up during the build.
> > > >
> > > > For other arch such as "make ARCH=x86_64 allyesconfig", the default
> > > > value would be 2048 and won't have this warning.
> > > >
> > > > Not sure if this is a real problem that need to be fixed, here just
> > > > providing above information for your reference.
> > > >
> > > > --
> > > > Best Regards,
> > > > Yujie
> > >
> > > *Must* be fixed given that it is how the default value is set now.
> > > This is wrong place to reconsider.
> > >
> > >
> > > And we do not want to add functions that bloat the stack this way.
> > >
> > > Shash just needs to be allocated from heap instead of stack.
> >
> > On x86_64 the stack usage is measured at 984 bytes, so rather than
> > jumping to conclusions let's root cause why this is a problem only on
> > the arc architecture.  I suspect it's something to do with the
> > alignment constraints of shash.  I've also noted it shouldn't actually
> > warn on arc because the default stack warning size there should be 2048
> > (like x86_64).
>
> Would it such a big deal to allocate shash from heap? That would
> be IMHO more robust in the end.
>

Can we avoid shashes and sync skciphers at all? We have sha256 and AES
library routines these days, and AES in CFB mode seems like a good
candidate for a library implementation as well - it uses AES
encryption only, and is quite straight forward to implement. [0]

The crypto API is far too clunky for synchronous operations of
algorithms that are known at compile time, and the requirement to use
scatterlists for skciphers is especially horrid.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=crypto-aes-cfb-library

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[James Bottomley]

On Tue, 2023-02-14 at 14:54 +0100, Ard Biesheuvel wrote:
> On Mon, 13 Feb 2023 at 08:45, Jarkko Sakkinen <jarkko@kernel.org>
> wrote:
> > 
> > On Fri, Feb 10, 2023 at 09:48:15AM -0500, James Bottomley wrote:
> > > On Wed, 2023-02-08 at 04:49 +0200, Jarkko Sakkinen wrote:
> > > > On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> > > > > Hi James,
> > > > > 
> > > > > On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley
> > > > > wrote:
> > > > > > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > > > > > Hi James,
> > > > > > > 
> > > > > > > I love your patch! Perhaps something to improve:
> > > > > > > 
> > > > > > > [auto build test WARNING on char-misc/char-misc-testing]
> > > > > > > [also build test WARNING on char-misc/char-misc-next
> > > > > > > char-
> > > > > > > misc/char-
> > > > > > > misc-linus zohar-integrity/next-integrity linus/master
> > > > > > > v6.2-rc5
> > > > > > > next-
> > > > > > > 20230124]
> > > > > > > [If your patch is applied to the wrong git tree, kindly
> > > > > > > drop us
> > > > > > > a
> > > > > > > note.
> > > > > > > And when submitting patch, we suggest to use '--base' as
> > > > > > > documented
> > > > > > > in
> > > > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information
> > > > > > > ]
> > > > > > > 
> > > > > > > url:
> > > > > > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > > > > > patch link:
> > > > > > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > > > > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > > > > > encrypt/decrypt session handling code
> > > > > > > config: arc-allyesconfig
> > > > > > > (
> > > > > > > https://download.01.org/0day-ci/archive/20230125/202301250706.de
> > > > > > > Gvd0
> > > > > > > yq-lkp@intel.com/config)
> > > > > > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > > > > > reproduce (this is a W=1 build):
> > > > > > >         wget
> > > > > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > > > > >  -O ~/bin/make.cross
> > > > > > >         chmod +x ~/bin/make.cross
> > > > > > >         #
> > > > > > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > > >         git remote add linux-review
> > > > > > > https://github.com/intel-lab-lkp/linux
> > > > > > >         git fetch --no-tags linux-review James-
> > > > > > > Bottomley/tpm-
> > > > > > > move-
> > > > > > > buffer-handling-from-static-inlines-to-real-
> > > > > > > functions/20230125-
> > > > > > > 020146
> > > > > > >         git checkout
> > > > > > > dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > > >         # save the config file
> > > > > > >         mkdir build_dir && cp config build_dir/.config
> > > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-
> > > > > > > 12.1.0
> > > > > > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-
> > > > > > > 12.1.0
> > > > > > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> > > > > > > drivers/char/tpm/
> > > > > > > 
> > > > > > > If you fix the issue, kindly add following tag where
> > > > > > > applicable
> > > > > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > > > 
> > > > > > > All warnings (new ones prefixed by >>):
> > > > > > > 
> > > > > > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no
> > > > > > > previous
> > > > > > > prototype for 'tpm2_create_null_primary' [-Wmissing-
> > > > > > > prototypes]
> > > > > > >     1184 | int tpm2_create_null_primary(struct tpm_chip
> > > > > > > *chip)
> > > > > > > {
> > > > > > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > > 'tpm_buf_check_hmac_response':
> > > > > > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the
> > > > > > > > > frame
> > > > > > > > > size
> > > > > > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-
> > > > > > > > > larger-
> > > > > > > > > than=]
> > > > > > >      831 | }
> > > > > > >          | ^
> > > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > > 'tpm_buf_fill_hmac_session':
> > > > > > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the
> > > > > > > frame
> > > > > > > size of
> > > > > > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > > > than=]
> > > > > > >      579 | }
> > > > > > >          | ^
> > > > > > 
> > > > > > Is this a test problem?  I can't see why the code would
> > > > > > only blow
> > > > > > the
> > > > > > stack on the arc architecture and not on any other ... does
> > > > > > it
> > > > > > have
> > > > > > something funny with on stack crypto structures?
> > > > > 
> > > > > This warning is controlled by the value of CONFIG_FRAME_WARN.
> > > > > 
> > > > > For "make ARCH=arc allyesconfig", the default value is 1024,
> > > > > so
> > > > > this frame warning shows up during the build.
> > > > > 
> > > > > For other arch such as "make ARCH=x86_64 allyesconfig", the
> > > > > default
> > > > > value would be 2048 and won't have this warning.
> > > > > 
> > > > > Not sure if this is a real problem that need to be fixed,
> > > > > here just
> > > > > providing above information for your reference.
> > > > > 
> > > > > --
> > > > > Best Regards,
> > > > > Yujie
> > > > 
> > > > *Must* be fixed given that it is how the default value is set
> > > > now.
> > > > This is wrong place to reconsider.
> > > > 
> > > > 
> > > > And we do not want to add functions that bloat the stack this
> > > > way.
> > > > 
> > > > Shash just needs to be allocated from heap instead of stack.
> > > 
> > > On x86_64 the stack usage is measured at 984 bytes, so rather
> > > than
> > > jumping to conclusions let's root cause why this is a problem
> > > only on
> > > the arc architecture.  I suspect it's something to do with the
> > > alignment constraints of shash.  I've also noted it shouldn't
> > > actually
> > > warn on arc because the default stack warning size there should
> > > be 2048
> > > (like x86_64).
> > 
> > Would it such a big deal to allocate shash from heap? That would
> > be IMHO more robust in the end.

Heap allocation is time indeterminate and eventually Mimi is going to
want me to make this go faster.

> 
> Can we avoid shashes and sync skciphers at all? We have sha256 and
> AES library routines these days, and AES in CFB mode seems like a
> good candidate for a library implementation as well - it uses AES
> encryption only, and is quite straight forward to implement. [0]

Yes, sure.  I originally suggested something like this way back four
years ago, but it got overruled on the grounds that if I didn't use
shashes and skciphers some architectures would be unable to use crypto
acceleration.  If that's no longer a consideration, I'm all for
simplification of static cipher types.

> The crypto API is far too clunky for synchronous operations of
> algorithms that are known at compile time, and the requirement to use
> scatterlists for skciphers is especially horrid.
> 
> [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=crypto-aes-cfb-library

OK, let me have a go at respinning based on this.

Regards,

James

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[James Bottomley]

On Tue, 2023-02-14 at 14:54 +0100, Ard Biesheuvel wrote:
[...]
> and the requirement to use
> scatterlists for skciphers is especially horrid.

Just by way of irony, we do have one place that does use a two element
scatterlist.  It's how I found this bug: 95ec01ba1ef0 ("crypto: ecdh -
fix to allow multi segment scatterlists").  And that does mean I could
do with a library version of ecdh with the nist P-256 curve ... pretty
please ...

Regards,

James

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Ard Biesheuvel]

On Tue, 14 Feb 2023 at 15:28, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Tue, 2023-02-14 at 14:54 +0100, Ard Biesheuvel wrote:
> > On Mon, 13 Feb 2023 at 08:45, Jarkko Sakkinen <jarkko@kernel.org>
> > wrote:
> > >
> > > On Fri, Feb 10, 2023 at 09:48:15AM -0500, James Bottomley wrote:
> > > > On Wed, 2023-02-08 at 04:49 +0200, Jarkko Sakkinen wrote:
> > > > > On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> > > > > > Hi James,
> > > > > >
> > > > > > On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley
> > > > > > wrote:
> > > > > > > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > > > > > > Hi James,
> > > > > > > >
> > > > > > > > I love your patch! Perhaps something to improve:
> > > > > > > >
> > > > > > > > [auto build test WARNING on char-misc/char-misc-testing]
> > > > > > > > [also build test WARNING on char-misc/char-misc-next
> > > > > > > > char-
> > > > > > > > misc/char-
> > > > > > > > misc-linus zohar-integrity/next-integrity linus/master
> > > > > > > > v6.2-rc5
> > > > > > > > next-
> > > > > > > > 20230124]
> > > > > > > > [If your patch is applied to the wrong git tree, kindly
> > > > > > > > drop us
> > > > > > > > a
> > > > > > > > note.
> > > > > > > > And when submitting patch, we suggest to use '--base' as
> > > > > > > > documented
> > > > > > > > in
> > > > > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information
> > > > > > > > ]
> > > > > > > >
> > > > > > > > url:
> > > > > > > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > > > > > > patch link:
> > > > > > > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > > > > > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > > > > > > encrypt/decrypt session handling code
> > > > > > > > config: arc-allyesconfig
> > > > > > > > (
> > > > > > > > https://download.01.org/0day-ci/archive/20230125/202301250706.de
> > > > > > > > Gvd0
> > > > > > > > yq-lkp@intel.com/config)
> > > > > > > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > > > > > > reproduce (this is a W=1 build):
> > > > > > > >         wget
> > > > > > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > > > > > >  -O ~/bin/make.cross
> > > > > > > >         chmod +x ~/bin/make.cross
> > > > > > > >         #
> > > > > > > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > > > >         git remote add linux-review
> > > > > > > > https://github.com/intel-lab-lkp/linux
> > > > > > > >         git fetch --no-tags linux-review James-
> > > > > > > > Bottomley/tpm-
> > > > > > > > move-
> > > > > > > > buffer-handling-from-static-inlines-to-real-
> > > > > > > > functions/20230125-
> > > > > > > > 020146
> > > > > > > >         git checkout
> > > > > > > > dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > > > >         # save the config file
> > > > > > > >         mkdir build_dir && cp config build_dir/.config
> > > > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-
> > > > > > > > 12.1.0
> > > > > > > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > > > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-
> > > > > > > > 12.1.0
> > > > > > > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> > > > > > > > drivers/char/tpm/
> > > > > > > >
> > > > > > > > If you fix the issue, kindly add following tag where
> > > > > > > > applicable
> > > > > > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > > > >
> > > > > > > > All warnings (new ones prefixed by >>):
> > > > > > > >
> > > > > > > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no
> > > > > > > > previous
> > > > > > > > prototype for 'tpm2_create_null_primary' [-Wmissing-
> > > > > > > > prototypes]
> > > > > > > >     1184 | int tpm2_create_null_primary(struct tpm_chip
> > > > > > > > *chip)
> > > > > > > > {
> > > > > > > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > > > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > > > 'tpm_buf_check_hmac_response':
> > > > > > > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the
> > > > > > > > > > frame
> > > > > > > > > > size
> > > > > > > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-
> > > > > > > > > > larger-
> > > > > > > > > > than=]
> > > > > > > >      831 | }
> > > > > > > >          | ^
> > > > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > > > 'tpm_buf_fill_hmac_session':
> > > > > > > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the
> > > > > > > > frame
> > > > > > > > size of
> > > > > > > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > > > > than=]
> > > > > > > >      579 | }
> > > > > > > >          | ^
> > > > > > >
> > > > > > > Is this a test problem?  I can't see why the code would
> > > > > > > only blow
> > > > > > > the
> > > > > > > stack on the arc architecture and not on any other ... does
> > > > > > > it
> > > > > > > have
> > > > > > > something funny with on stack crypto structures?
> > > > > >
> > > > > > This warning is controlled by the value of CONFIG_FRAME_WARN.
> > > > > >
> > > > > > For "make ARCH=arc allyesconfig", the default value is 1024,
> > > > > > so
> > > > > > this frame warning shows up during the build.
> > > > > >
> > > > > > For other arch such as "make ARCH=x86_64 allyesconfig", the
> > > > > > default
> > > > > > value would be 2048 and won't have this warning.
> > > > > >
> > > > > > Not sure if this is a real problem that need to be fixed,
> > > > > > here just
> > > > > > providing above information for your reference.
> > > > > >
> > > > > > --
> > > > > > Best Regards,
> > > > > > Yujie
> > > > >
> > > > > *Must* be fixed given that it is how the default value is set
> > > > > now.
> > > > > This is wrong place to reconsider.
> > > > >
> > > > >
> > > > > And we do not want to add functions that bloat the stack this
> > > > > way.
> > > > >
> > > > > Shash just needs to be allocated from heap instead of stack.
> > > >
> > > > On x86_64 the stack usage is measured at 984 bytes, so rather
> > > > than
> > > > jumping to conclusions let's root cause why this is a problem
> > > > only on
> > > > the arc architecture.  I suspect it's something to do with the
> > > > alignment constraints of shash.  I've also noted it shouldn't
> > > > actually
> > > > warn on arc because the default stack warning size there should
> > > > be 2048
> > > > (like x86_64).
> > >
> > > Would it such a big deal to allocate shash from heap? That would
> > > be IMHO more robust in the end.
>
> Heap allocation is time indeterminate and eventually Mimi is going to
> want me to make this go faster.
>
> >
> > Can we avoid shashes and sync skciphers at all? We have sha256 and
> > AES library routines these days, and AES in CFB mode seems like a
> > good candidate for a library implementation as well - it uses AES
> > encryption only, and is quite straight forward to implement. [0]
>
> Yes, sure.  I originally suggested something like this way back four
> years ago, but it got overruled on the grounds that if I didn't use
> shashes and skciphers some architectures would be unable to use crypto
> acceleration.  If that's no longer a consideration, I'm all for
> simplification of static cipher types.
>

I don't know if that is a consideration or not. The AES library code
is generic C code that was written to be constant-time, rather than
fast. The fact that CFB only uses the encryption side of it is
fortunate, because decryption is even slower.

So the question is whether this will actually be a bottleneck in this
particular scenario. The synchronous accelerated AES implementations
are all SIMD based, which means there is some overhead, and some
degree of parallelism is also needed to take full advantage, and CFB
only allows this for decryption to begin with, as encryption uses
ciphertext block N-1 as AES input for encrypting block N.

So maybe this is terrible advice, but the code will look so much
better for it, and we can always add back the performance later if it
is really an impediment.


> > The crypto API is far too clunky for synchronous operations of
> > algorithms that are known at compile time, and the requirement to use
> > scatterlists for skciphers is especially horrid.
> >
> > [0]
> > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=crypto-aes-cfb-library
>
> OK, let me have a go at respinning based on this.
>
> Regards,
>
> James
>

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[James Bottomley]

On Tue, 2023-02-14 at 15:36 +0100, Ard Biesheuvel wrote:
> On Tue, 14 Feb 2023 at 15:28, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Tue, 2023-02-14 at 14:54 +0100, Ard Biesheuvel wrote:
[...]
> > > 
> > > Can we avoid shashes and sync skciphers at all? We have sha256
> > > and AES library routines these days, and AES in CFB mode seems
> > > like a good candidate for a library implementation as well - it
> > > uses AES encryption only, and is quite straight forward to
> > > implement. [0]
> > 
> > Yes, sure.  I originally suggested something like this way back
> > four years ago, but it got overruled on the grounds that if I
> > didn't use shashes and skciphers some architectures would be unable
> > to use crypto acceleration.  If that's no longer a consideration,
> > I'm all for simplification of static cipher types.
> > 

I now have this all implemented, and I looked over your code, so you
can add my tested/reviewed-by to the aescfb implementation.  On the
acceleration issue, I'm happy to ignore external accelerators because
they're a huge pain for small fragments of encryption like the TPM, but
it would be nice if we could integrate CPU instruction acceleration
(like AES-NI on x86) into the library functions. 

I also got a test rig to investigate arc.  It seems there is a huge
problem with the SKCIPHER stack structure on that platform.  For
reasons I still can't fathom, the compiler thinks it needs at least
0.5k of stack for this one structure.  I'm sure its something to do
with an incorrect crypto alignment on arc, but I can't yet find the
root cause.

> I don't know if that is a consideration or not. The AES library code
> is generic C code that was written to be constant-time, rather than
> fast. The fact that CFB only uses the encryption side of it is
> fortunate, because decryption is even slower.

I think for the TPM, since the encryption isn't exactly bulk (it's
really under 1k for command and response encryption) it doesn't matter
... in fact setting up the accelerator is likely a bigger overhead.

> So the question is whether this will actually be a bottleneck in this
> particular scenario. The synchronous accelerated AES implementations
> are all SIMD based, which means there is some overhead, and some
> degree of parallelism is also needed to take full advantage, and CFB
> only allows this for decryption to begin with, as encryption uses
> ciphertext block N-1 as AES input for encrypting block N.
> 
> So maybe this is terrible advice, but the code will look so much
> better for it, and we can always add back the performance later if it
> is really an impediment.

It's definitely smaller and neater, yes.  I'll post a v3 based on this,
but when might it go upstream?  In my post I'll put your aescfb as
patch 1 so the static checkers don't go haywire about missing function
exports, and we can drop that patch when it is upstream.

James

> 
> 
> > > The crypto API is far too clunky for synchronous operations of
> > > algorithms that are known at compile time, and the requirement to
> > > use scatterlists for skciphers is especially horrid.
> > > 
> > > [0]
> > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=crypto-aes-cfb-library
> > 
> > OK, let me have a go at respinning based on this.

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Ard Biesheuvel]

On Thu, 16 Feb 2023 at 15:52, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Tue, 2023-02-14 at 15:36 +0100, Ard Biesheuvel wrote:
> > On Tue, 14 Feb 2023 at 15:28, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > > On Tue, 2023-02-14 at 14:54 +0100, Ard Biesheuvel wrote:
> [...]
> > > >
> > > > Can we avoid shashes and sync skciphers at all? We have sha256
> > > > and AES library routines these days, and AES in CFB mode seems
> > > > like a good candidate for a library implementation as well - it
> > > > uses AES encryption only, and is quite straight forward to
> > > > implement. [0]
> > >
> > > Yes, sure.  I originally suggested something like this way back
> > > four years ago, but it got overruled on the grounds that if I
> > > didn't use shashes and skciphers some architectures would be unable
> > > to use crypto acceleration.  If that's no longer a consideration,
> > > I'm all for simplification of static cipher types.
> > >
>
> I now have this all implemented, and I looked over your code, so you
> can add my tested/reviewed-by to the aescfb implementation.  On the
> acceleration issue, I'm happy to ignore external accelerators because
> they're a huge pain for small fragments of encryption like the TPM, but
> it would be nice if we could integrate CPU instruction acceleration
> (like AES-NI on x86) into the library functions.
>

Agreed that async crypto makes no sense here, and it is rather
unfortunate that even use cases such as this one require the
scatterlist handling, which requires direct mapped memory etc etc

As for the accelerated algos: it wouldn't be too complicated to build
the CFB library interface around the 'AES' crypto cipher, which is
synchronous and operates on virtual addresses directly. But it should
only use ones that are constant time (like AES-NI) and not use generic
AES or the asm accelerated ones, and so this would require an
additional annotation (or an allowlist) which makes things a bit
clunky.

However, doing the math on the back of an envelope: taking arm64 as an
example, which manages ~1 cycle per byte for AES instructions and 25
cycles per byte for AES encryption using this library, processing 1k
of data takes an additional 24k cycles, which comes down to 10
microseconds on a 2.4 GHz CPU.

Given that this particular use case is about communicating with off
chip discrete components, I wonder whether spending 10 microseconds
more is going to have a noticeable impact.

> I also got a test rig to investigate arc.  It seems there is a huge
> problem with the SKCIPHER stack structure on that platform.  For
> reasons I still can't fathom, the compiler thinks it needs at least
> 0.5k of stack for this one structure.  I'm sure its something to do
> with an incorrect crypto alignment on arc, but I can't yet find the
> root cause.
>

Maybe SKCIPHER_ON_STACK() needs the same treatment as
660d2062190db131d2feaf19914e90f868fe285c?

The catch here is that if we reduce the alignment of the buffer, the
req pointer will not have the alignment of the typedef, and so we will
be lying to the compiler.

This is all a result of the way we abuse alignment to pad out data
fields that may be used for inbound non-coherent DMA, and this is
something that is being fixed at the moment.


> > I don't know if that is a consideration or not. The AES library code
> > is generic C code that was written to be constant-time, rather than
> > fast. The fact that CFB only uses the encryption side of it is
> > fortunate, because decryption is even slower.
>
> I think for the TPM, since the encryption isn't exactly bulk (it's
> really under 1k for command and response encryption) it doesn't matter
> ... in fact setting up the accelerator is likely a bigger overhead.
>
> > So the question is whether this will actually be a bottleneck in this
> > particular scenario. The synchronous accelerated AES implementations
> > are all SIMD based, which means there is some overhead, and some
> > degree of parallelism is also needed to take full advantage, and CFB
> > only allows this for decryption to begin with, as encryption uses
> > ciphertext block N-1 as AES input for encrypting block N.
> >
> > So maybe this is terrible advice, but the code will look so much
> > better for it, and we can always add back the performance later if it
> > is really an impediment.
>
> It's definitely smaller and neater, yes.  I'll post a v3 based on this,
> but when might it go upstream?  In my post I'll put your aescfb as
> patch 1 so the static checkers don't go haywire about missing function
> exports, and we can drop that patch when it is upstream.
>

I'll add some test cases and send it to the list.

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Jarkko Sakkinen]

On Tue, Feb 14, 2023 at 02:54:02PM +0100, Ard Biesheuvel wrote:
> On Mon, 13 Feb 2023 at 08:45, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Fri, Feb 10, 2023 at 09:48:15AM -0500, James Bottomley wrote:
> > > On Wed, 2023-02-08 at 04:49 +0200, Jarkko Sakkinen wrote:
> > > > On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> > > > > Hi James,
> > > > >
> > > > > On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > > > > > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > > > > > Hi James,
> > > > > > >
> > > > > > > I love your patch! Perhaps something to improve:
> > > > > > >
> > > > > > > [auto build test WARNING on char-misc/char-misc-testing]
> > > > > > > [also build test WARNING on char-misc/char-misc-next char-
> > > > > > > misc/char-
> > > > > > > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5
> > > > > > > next-
> > > > > > > 20230124]
> > > > > > > [If your patch is applied to the wrong git tree, kindly drop us
> > > > > > > a
> > > > > > > note.
> > > > > > > And when submitting patch, we suggest to use '--base' as
> > > > > > > documented
> > > > > > > in
> > > > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information
> > > > > > > ]
> > > > > > >
> > > > > > > url:
> > > > > > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > > > > > patch link:
> > > > > > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > > > > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > > > > > encrypt/decrypt session handling code
> > > > > > > config: arc-allyesconfig
> > > > > > > (
> > > > > > > https://download.01.org/0day-ci/archive/20230125/202301250706.de
> > > > > > > Gvd0
> > > > > > > yq-lkp@intel.com/config)
> > > > > > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > > > > > reproduce (this is a W=1 build):
> > > > > > >         wget
> > > > > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > > > > >  -O ~/bin/make.cross
> > > > > > >         chmod +x ~/bin/make.cross
> > > > > > >         #
> > > > > > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > > >         git remote add linux-review
> > > > > > > https://github.com/intel-lab-lkp/linux
> > > > > > >         git fetch --no-tags linux-review James-Bottomley/tpm-
> > > > > > > move-
> > > > > > > buffer-handling-from-static-inlines-to-real-functions/20230125-
> > > > > > > 020146
> > > > > > >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > > > > > >         # save the config file
> > > > > > >         mkdir build_dir && cp config build_dir/.config
> > > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > > > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > > > > > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > > > > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> > > > > > > drivers/char/tpm/
> > > > > > >
> > > > > > > If you fix the issue, kindly add following tag where applicable
> > > > > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > > >
> > > > > > > All warnings (new ones prefixed by >>):
> > > > > > >
> > > > > > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no
> > > > > > > previous
> > > > > > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > > > > > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip)
> > > > > > > {
> > > > > > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > > 'tpm_buf_check_hmac_response':
> > > > > > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame
> > > > > > > > > size
> > > > > > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-
> > > > > > > > > than=]
> > > > > > >      831 | }
> > > > > > >          | ^
> > > > > > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > > > > > 'tpm_buf_fill_hmac_session':
> > > > > > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame
> > > > > > > size of
> > > > > > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > > > > >      579 | }
> > > > > > >          | ^
> > > > > >
> > > > > > Is this a test problem?  I can't see why the code would only blow
> > > > > > the
> > > > > > stack on the arc architecture and not on any other ... does it
> > > > > > have
> > > > > > something funny with on stack crypto structures?
> > > > >
> > > > > This warning is controlled by the value of CONFIG_FRAME_WARN.
> > > > >
> > > > > For "make ARCH=arc allyesconfig", the default value is 1024, so
> > > > > this frame warning shows up during the build.
> > > > >
> > > > > For other arch such as "make ARCH=x86_64 allyesconfig", the default
> > > > > value would be 2048 and won't have this warning.
> > > > >
> > > > > Not sure if this is a real problem that need to be fixed, here just
> > > > > providing above information for your reference.
> > > > >
> > > > > --
> > > > > Best Regards,
> > > > > Yujie
> > > >
> > > > *Must* be fixed given that it is how the default value is set now.
> > > > This is wrong place to reconsider.
> > > >
> > > >
> > > > And we do not want to add functions that bloat the stack this way.
> > > >
> > > > Shash just needs to be allocated from heap instead of stack.
> > >
> > > On x86_64 the stack usage is measured at 984 bytes, so rather than
> > > jumping to conclusions let's root cause why this is a problem only on
> > > the arc architecture.  I suspect it's something to do with the
> > > alignment constraints of shash.  I've also noted it shouldn't actually
> > > warn on arc because the default stack warning size there should be 2048
> > > (like x86_64).
> >
> > Would it such a big deal to allocate shash from heap? That would
> > be IMHO more robust in the end.
> >
> 
> Can we avoid shashes and sync skciphers at all? We have sha256 and AES
> library routines these days, and AES in CFB mode seems like a good
> candidate for a library implementation as well - it uses AES
> encryption only, and is quite straight forward to implement. [0]
> 
> The crypto API is far too clunky for synchronous operations of
> algorithms that are known at compile time, and the requirement to use
> scatterlists for skciphers is especially horrid.

I'm cool with any solution not polluting the stack to its limits...

BR, Jarkko

Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[kernel test robot]

:::::: 
:::::: Manual check reason: "low confidence static check warning: drivers/char/tpm/tpm2-sessions.c:395:3: warning: Null pointer passed as 2nd argument to memory copy function [clang-analyzer-unix.cstring.NullArg]"
:::::: 

BCC: lkp@intel.com
CC: llvm@lists.linux.dev
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230124175516.5984-7-James.Bottomley@HansenPartnership.com>
References: <20230124175516.5984-7-James.Bottomley@HansenPartnership.com>
TO: James Bottomley <James.Bottomley@HansenPartnership.com>
TO: linux-integrity@vger.kernel.org
CC: Jarkko Sakkinen <jarkko@kernel.org>
CC: keyrings@vger.kernel.org
CC: Ard Biesheuvel <ardb@kernel.org>

Hi James,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-20230127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
patch link:    https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code
:::::: branch date: 5 days ago
:::::: commit date: 5 days ago
config: s390-randconfig-c005-20230123 (https://download.01.org/0day-ci/archive/20230129/202301291449.E9ZLao2b-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
        git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
        # save the config file
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 clang-analyzer  olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 clang-analyzer 

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

clang_analyzer warnings: (new ones prefixed by >>)
                   ^
   include/linux/wait.h:1208:27: note: expanded from macro 'DEFINE_WAIT'
   #define DEFINE_WAIT(name) DEFINE_WAIT_FUNC(name, autoremove_wake_function)
                             ^
   include/linux/wait.h:1203:14: note: expanded from macro 'DEFINE_WAIT_FUNC'
                   .private        = current,                                      \
                                     ^
   arch/s390/include/asm/current.h:17:45: note: expanded from macro 'current'
   #define current ((struct task_struct *const)S390_lowcore.current_task)
                                               ^
   arch/s390/include/asm/lowcore.h:215:22: note: expanded from macro 'S390_lowcore'
   #define S390_lowcore (*((struct lowcore *) 0))
                        ^
   fs/btrfs/compression.c:1657:30: note: Calling 'get_workspace'
           struct list_head *ws_list = get_workspace(0, 0);
                                       ^~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1097:2: note: Control jumps to 'case BTRFS_COMPRESS_NONE:'  at line 1098
           switch (type) {
           ^
   fs/btrfs/compression.c:1098:35: note: Calling 'btrfs_get_workspace'
           case BTRFS_COMPRESS_NONE: return btrfs_get_workspace(type, level);
                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1037:6: note: Assuming the condition is false
           if (!list_empty(idle_ws)) {
               ^~~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1037:2: note: Taking false branch
           if (!list_empty(idle_ws)) {
           ^
   fs/btrfs/compression.c:1045:6: note: Assuming the condition is true
           if (atomic_read(total_ws) > cpus) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1045:2: note: Taking true branch
           if (atomic_read(total_ws) > cpus) {
           ^
   fs/btrfs/compression.c:1046:3: note: Dereference of null pointer
                   DEFINE_WAIT(wait);
                   ^
   include/linux/wait.h:1208:27: note: expanded from macro 'DEFINE_WAIT'
   #define DEFINE_WAIT(name) DEFINE_WAIT_FUNC(name, autoremove_wake_function)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/wait.h:1203:14: note: expanded from macro 'DEFINE_WAIT_FUNC'
                   .private        = current,                                      \
                                     ^~~~~~~
   arch/s390/include/asm/current.h:17:45: note: expanded from macro 'current'
   #define current ((struct task_struct *const)S390_lowcore.current_task)
                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/s390/include/asm/lowcore.h:215:22: note: expanded from macro 'S390_lowcore'
   #define S390_lowcore (*((struct lowcore *) 0))
                        ^
   include/linux/sched/mm.h:321:23: warning: Dereference of null pointer [clang-analyzer-core.NullDereference]
           unsigned int flags = current->flags & PF_MEMALLOC_NOFS;
                                ^
   arch/s390/include/asm/current.h:17:45: note: expanded from macro 'current'
   #define current ((struct task_struct *const)S390_lowcore.current_task)
                                               ^
   arch/s390/include/asm/lowcore.h:215:22: note: expanded from macro 'S390_lowcore'
   #define S390_lowcore (*((struct lowcore *) 0))
                        ^
   fs/btrfs/compression.c:1657:30: note: Calling 'get_workspace'
           struct list_head *ws_list = get_workspace(0, 0);
                                       ^~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1097:2: note: Control jumps to 'case BTRFS_COMPRESS_NONE:'  at line 1098
           switch (type) {
           ^
   fs/btrfs/compression.c:1098:35: note: Calling 'btrfs_get_workspace'
           case BTRFS_COMPRESS_NONE: return btrfs_get_workspace(type, level);
                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1037:6: note: Assuming the condition is false
           if (!list_empty(idle_ws)) {
               ^~~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1037:2: note: Taking false branch
           if (!list_empty(idle_ws)) {
           ^
   fs/btrfs/compression.c:1045:6: note: Assuming the condition is false
           if (atomic_read(total_ws) > cpus) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/compression.c:1045:2: note: Taking false branch
           if (atomic_read(total_ws) > cpus) {
           ^
   fs/btrfs/compression.c:1063:14: note: Calling 'memalloc_nofs_save'
           nofs_flag = memalloc_nofs_save();
                       ^~~~~~~~~~~~~~~~~~~~
   include/linux/sched/mm.h:321:23: note: Dereference of null pointer
           unsigned int flags = current->flags & PF_MEMALLOC_NOFS;
                                ^
   arch/s390/include/asm/current.h:17:45: note: expanded from macro 'current'
   #define current ((struct task_struct *const)S390_lowcore.current_task)
                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/s390/include/asm/lowcore.h:215:22: note: expanded from macro 'S390_lowcore'
   #define S390_lowcore (*((struct lowcore *) 0))
                        ^
   Suppressed 11 warnings (11 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   23 warnings generated.
   Suppressed 23 warnings (10 in non-user code, 13 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   10 warnings generated.
   Suppressed 10 warnings (9 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   26 warnings generated.
>> drivers/char/tpm/tpm2-sessions.c:395:3: warning: Null pointer passed as 2nd argument to memory copy function [clang-analyzer-unix.cstring.NullArg]
                   memcpy(auth->passphrase, passphrase, passphraselen);
                   ^
   include/linux/fortify-string.h:623:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^                       ~
   include/linux/fortify-string.h:578:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^                    ~
   note: expanded from here
   include/linux/fortify-string.h:57:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^
   drivers/char/tpm/tpm2-sessions.c:388:9: note: Assuming 'passphrase' is null
           while (passphrase && passphraselen > 0
                  ^~~~~~~~~~
   drivers/char/tpm/tpm2-sessions.c:388:20: note: Left side of '&&' is false
           while (passphrase && passphraselen > 0
                             ^
   drivers/char/tpm/tpm2-sessions.c:394:6: note: Assuming 'passphraselen' is not equal to 0
           if (passphraselen)
               ^~~~~~~~~~~~~
   drivers/char/tpm/tpm2-sessions.c:394:2: note: Taking true branch
           if (passphraselen)
           ^
   drivers/char/tpm/tpm2-sessions.c:395:3: note: '__ret_cond' is false
                   memcpy(auth->passphrase, passphrase, passphraselen);
                   ^
   include/linux/fortify-string.h:623:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:571:2: note: expanded from macro '__fortify_memcpy_chk'
           WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size,          \
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/bug.h:151:2: note: expanded from macro 'WARN_ONCE'
           DO_ONCE_LITE_IF(condition, WARN, 1, format)
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/once_lite.h:30:7: note: expanded from macro 'DO_ONCE_LITE_IF'
                   if (__ONCE_LITE_IF(__ret_do_once))                      \
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/once_lite.h:19:16: note: expanded from macro '__ONCE_LITE_IF'
                   if (unlikely(__ret_cond && !__already_done)) {          \
                                ^~~~~~~~~~
   include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   drivers/char/tpm/tpm2-sessions.c:395:3: note: Left side of '&&' is false
                   memcpy(auth->passphrase, passphrase, passphraselen);
                   ^
   include/linux/fortify-string.h:623:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^
   include/linux/fortify-string.h:571:2: note: expanded from macro '__fortify_memcpy_chk'
           WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size,          \
           ^
   include/asm-generic/bug.h:151:2: note: expanded from macro 'WARN_ONCE'
           DO_ONCE_LITE_IF(condition, WARN, 1, format)
           ^
   include/linux/once_lite.h:30:7: note: expanded from macro 'DO_ONCE_LITE_IF'
                   if (__ONCE_LITE_IF(__ret_do_once))                      \
                       ^
   include/linux/once_lite.h:19:27: note: expanded from macro '__ONCE_LITE_IF'
                   if (unlikely(__ret_cond && !__already_done)) {          \
                                           ^
   drivers/char/tpm/tpm2-sessions.c:395:3: note: Taking false branch
                   memcpy(auth->passphrase, passphrase, passphraselen);
                   ^
   include/linux/fortify-string.h:623:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^
   include/linux/fortify-string.h:571:2: note: expanded from macro '__fortify_memcpy_chk'
           WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size,          \
           ^
   include/asm-generic/bug.h:151:2: note: expanded from macro 'WARN_ONCE'
           DO_ONCE_LITE_IF(condition, WARN, 1, format)
           ^
   include/linux/once_lite.h:30:7: note: expanded from macro 'DO_ONCE_LITE_IF'
                   if (__ONCE_LITE_IF(__ret_do_once))                      \
                       ^
   include/linux/once_lite.h:19:3: note: expanded from macro '__ONCE_LITE_IF'
                   if (unlikely(__ret_cond && !__already_done)) {          \
                   ^
   drivers/char/tpm/tpm2-sessions.c:395:3: note: Taking false branch
                   memcpy(auth->passphrase, passphrase, passphraselen);
                   ^
   include/linux/fortify-string.h:623:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^
   include/linux/fortify-string.h:571:2: note: expanded from macro '__fortify_memcpy_chk'
           WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size,          \
           ^
   include/asm-generic/bug.h:151:2: note: expanded from macro 'WARN_ONCE'
           DO_ONCE_LITE_IF(condition, WARN, 1, format)
           ^
   include/linux/once_lite.h:30:3: note: expanded from macro 'DO_ONCE_LITE_IF'
                   if (__ONCE_LITE_IF(__ret_do_once))                      \
                   ^
   drivers/char/tpm/tpm2-sessions.c:395:3: note: Null pointer passed as 2nd argument to memory copy function
                   memcpy(auth->passphrase, passphrase, passphraselen);
                   ^
   include/linux/fortify-string.h:623:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^                       ~
   include/linux/fortify-string.h:578:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^                    ~
   note: expanded from here
   include/linux/fortify-string.h:57:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^
>> drivers/char/tpm/tpm2-sessions.c:758:2: warning: Value stored to 'attrs' is never read [clang-analyzer-deadcode.DeadStores]
           attrs = *s++;
           ^       ~~~~
   drivers/char/tpm/tpm2-sessions.c:758:2: note: Value stored to 'attrs' is never read
           attrs = *s++;
           ^       ~~~~
   Suppressed 24 warnings (10 in non-user code, 14 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   21 warnings generated.
   Suppressed 21 warnings (9 in non-user code, 12 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   27 warnings generated.
   drivers/pci/endpoint/functions/pci-epf-vntb.c:907:15: warning: Access to field 'num_mws' results in a dereference of a null pointer (loaded from variable 'ntb') [clang-analyzer-core.NullDereference]
           ntb->num_mws = val;
           ~~~          ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:896:2: note: 'ntb' initialized to a null pointer value
           struct epf_ntb *ntb = to_epf_ntb(group);
           ^~~~~~~~~~~~~~~~~~~
   drivers/pci/endpoint/functions/pci-epf-vntb.c:901:6: note: Assuming 'ret' is 0
           if (ret)
               ^~~
   drivers/pci/endpoint/functions/pci-epf-vntb.c:901:2: note: Taking false branch
           if (ret)
           ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:904:6: note: Assuming 'val' is <= MAX_MW
           if (val > MAX_MW)
               ^~~~~~~~~~~~
   drivers/pci/endpoint/functions/pci-epf-vntb.c:904:2: note: Taking false branch
           if (val > MAX_MW)
           ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:907:15: note: Access to field 'num_mws' results in a dereference of a null pointer (loaded from variable 'ntb')
           ntb->num_mws = val;
           ~~~          ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:913:1: warning: Access to field 'spad_count' results in a dereference of a null pointer (loaded from variable 'ntb') [clang-analyzer-core.NullDereference]
   EPF_NTB_W(spad_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:839:13: note: expanded from macro 'EPF_NTB_W'
           ntb->_name = val;                                               \
           ~~~        ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:913:1: note: 'ntb' initialized to a null pointer value
   EPF_NTB_W(spad_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:831:2: note: expanded from macro 'EPF_NTB_W'
           struct epf_ntb *ntb = to_epf_ntb(group);                        \
           ^~~~~~~~~~~~~~~~~~~
   drivers/pci/endpoint/functions/pci-epf-vntb.c:913:1: note: Assuming 'ret' is 0
   EPF_NTB_W(spad_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:836:6: note: expanded from macro 'EPF_NTB_W'
           if (ret)                                                        \
               ^~~
   drivers/pci/endpoint/functions/pci-epf-vntb.c:913:1: note: Taking false branch
   EPF_NTB_W(spad_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:836:2: note: expanded from macro 'EPF_NTB_W'
           if (ret)                                                        \
           ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:913:1: note: Access to field 'spad_count' results in a dereference of a null pointer (loaded from variable 'ntb')
   EPF_NTB_W(spad_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:839:13: note: expanded from macro 'EPF_NTB_W'
           ntb->_name = val;                                               \
           ~~~        ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:915:1: warning: Access to field 'db_count' results in a dereference of a null pointer (loaded from variable 'ntb') [clang-analyzer-core.NullDereference]
   EPF_NTB_W(db_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:839:13: note: expanded from macro 'EPF_NTB_W'
           ntb->_name = val;                                               \
           ~~~        ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:915:1: note: 'ntb' initialized to a null pointer value
   EPF_NTB_W(db_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:831:2: note: expanded from macro 'EPF_NTB_W'
           struct epf_ntb *ntb = to_epf_ntb(group);                        \
           ^~~~~~~~~~~~~~~~~~~
   drivers/pci/endpoint/functions/pci-epf-vntb.c:915:1: note: Assuming 'ret' is 0
   EPF_NTB_W(db_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:836:6: note: expanded from macro 'EPF_NTB_W'
           if (ret)                                                        \
               ^~~
   drivers/pci/endpoint/functions/pci-epf-vntb.c:915:1: note: Taking false branch
   EPF_NTB_W(db_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:836:2: note: expanded from macro 'EPF_NTB_W'
           if (ret)                                                        \
           ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:915:1: note: Access to field 'db_count' results in a dereference of a null pointer (loaded from variable 'ntb')
   EPF_NTB_W(db_count)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:839:13: note: expanded from macro 'EPF_NTB_W'
           ntb->_name = val;                                               \
           ~~~        ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:918:1: warning: Access to field 'vbus_number' results in a dereference of a null pointer (loaded from variable 'ntb') [clang-analyzer-core.NullDereference]
   EPF_NTB_W(vbus_number)
   ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:839:13: note: expanded from macro 'EPF_NTB_W'
           ntb->_name = val;                                               \
           ~~~        ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:918:1: note: 'ntb' initialized to a null pointer value
   EPF_NTB_W(vbus_number)

vim +395 drivers/char/tpm/tpm2-sessions.c

dc0fc74718b4a7 James Bottomley 2023-01-24  350  
dc0fc74718b4a7 James Bottomley 2023-01-24  351  /**
dc0fc74718b4a7 James Bottomley 2023-01-24  352   * tpm_buf_append_hmac_session() append a TPM session element
dc0fc74718b4a7 James Bottomley 2023-01-24  353   * @buf: The buffer to be appended
dc0fc74718b4a7 James Bottomley 2023-01-24  354   * @auth: the auth structure allocated by tpm2_start_auth_session()
dc0fc74718b4a7 James Bottomley 2023-01-24  355   * @attributes: The session attributes
dc0fc74718b4a7 James Bottomley 2023-01-24  356   * @passphrase: The session authority (NULL if none)
dc0fc74718b4a7 James Bottomley 2023-01-24  357   * @passphraselen: The length of the session authority (0 if none)
dc0fc74718b4a7 James Bottomley 2023-01-24  358   *
dc0fc74718b4a7 James Bottomley 2023-01-24  359   * This fills in a session structure in the TPM command buffer, except
dc0fc74718b4a7 James Bottomley 2023-01-24  360   * for the HMAC which cannot be computed until the command buffer is
dc0fc74718b4a7 James Bottomley 2023-01-24  361   * complete.  The type of session is controlled by the @attributes,
dc0fc74718b4a7 James Bottomley 2023-01-24  362   * the main ones of which are TPM2_SA_CONTINUE_SESSION which means the
dc0fc74718b4a7 James Bottomley 2023-01-24  363   * session won't terminate after tpm_buf_check_hmac_response(),
dc0fc74718b4a7 James Bottomley 2023-01-24  364   * TPM2_SA_DECRYPT which means this buffers first parameter should be
dc0fc74718b4a7 James Bottomley 2023-01-24  365   * encrypted with a session key and TPM2_SA_ENCRYPT, which means the
dc0fc74718b4a7 James Bottomley 2023-01-24  366   * response buffer's first parameter needs to be decrypted (confusing,
dc0fc74718b4a7 James Bottomley 2023-01-24  367   * but the defines are written from the point of view of the TPM).
dc0fc74718b4a7 James Bottomley 2023-01-24  368   *
dc0fc74718b4a7 James Bottomley 2023-01-24  369   * Any session appended by this command must be finalized by calling
dc0fc74718b4a7 James Bottomley 2023-01-24  370   * tpm_buf_fill_hmac_session() otherwise the HMAC will be incorrect
dc0fc74718b4a7 James Bottomley 2023-01-24  371   * and the TPM will reject the command.
dc0fc74718b4a7 James Bottomley 2023-01-24  372   *
dc0fc74718b4a7 James Bottomley 2023-01-24  373   * As with most tpm_buf operations, success is assumed because failure
dc0fc74718b4a7 James Bottomley 2023-01-24  374   * will be caused by an incorrect programming model and indicated by a
dc0fc74718b4a7 James Bottomley 2023-01-24  375   * kernel message.
dc0fc74718b4a7 James Bottomley 2023-01-24  376   */
dc0fc74718b4a7 James Bottomley 2023-01-24  377  void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth,
dc0fc74718b4a7 James Bottomley 2023-01-24  378  				 u8 attributes, u8 *passphrase,
dc0fc74718b4a7 James Bottomley 2023-01-24  379  				 int passphraselen)
dc0fc74718b4a7 James Bottomley 2023-01-24  380  {
dc0fc74718b4a7 James Bottomley 2023-01-24  381  	u8 nonce[SHA256_DIGEST_SIZE];
dc0fc74718b4a7 James Bottomley 2023-01-24  382  	u32 len;
dc0fc74718b4a7 James Bottomley 2023-01-24  383  
dc0fc74718b4a7 James Bottomley 2023-01-24  384  	/*
dc0fc74718b4a7 James Bottomley 2023-01-24  385  	 * The Architecture Guide requires us to strip trailing zeros
dc0fc74718b4a7 James Bottomley 2023-01-24  386  	 * before computing the HMAC
dc0fc74718b4a7 James Bottomley 2023-01-24  387  	 */
dc0fc74718b4a7 James Bottomley 2023-01-24  388  	while (passphrase && passphraselen > 0
dc0fc74718b4a7 James Bottomley 2023-01-24  389  	       && passphrase[passphraselen - 1] == '\0')
dc0fc74718b4a7 James Bottomley 2023-01-24  390  		passphraselen--;
dc0fc74718b4a7 James Bottomley 2023-01-24  391  
dc0fc74718b4a7 James Bottomley 2023-01-24  392  	auth->attrs = attributes;
dc0fc74718b4a7 James Bottomley 2023-01-24  393  	auth->passphraselen = passphraselen;
dc0fc74718b4a7 James Bottomley 2023-01-24  394  	if (passphraselen)
dc0fc74718b4a7 James Bottomley 2023-01-24 @395  		memcpy(auth->passphrase, passphrase, passphraselen);
dc0fc74718b4a7 James Bottomley 2023-01-24  396  
dc0fc74718b4a7 James Bottomley 2023-01-24  397  	if (auth->session != tpm_buf_length(buf)) {
dc0fc74718b4a7 James Bottomley 2023-01-24  398  		/* we're not the first session */
dc0fc74718b4a7 James Bottomley 2023-01-24  399  		len = get_unaligned_be32(&buf->data[auth->session]);
dc0fc74718b4a7 James Bottomley 2023-01-24  400  		if (4 + len + auth->session != tpm_buf_length(buf)) {
dc0fc74718b4a7 James Bottomley 2023-01-24  401  			WARN(1, "session length mismatch, cannot append");
dc0fc74718b4a7 James Bottomley 2023-01-24  402  			return;
dc0fc74718b4a7 James Bottomley 2023-01-24  403  		}
dc0fc74718b4a7 James Bottomley 2023-01-24  404  
dc0fc74718b4a7 James Bottomley 2023-01-24  405  		/* add our new session */
dc0fc74718b4a7 James Bottomley 2023-01-24  406  		len += 9 + 2 * SHA256_DIGEST_SIZE;
dc0fc74718b4a7 James Bottomley 2023-01-24  407  		put_unaligned_be32(len, &buf->data[auth->session]);
dc0fc74718b4a7 James Bottomley 2023-01-24  408  	} else {
dc0fc74718b4a7 James Bottomley 2023-01-24  409  		tpm_buf_append_u32(buf, 9 + 2 * SHA256_DIGEST_SIZE);
dc0fc74718b4a7 James Bottomley 2023-01-24  410  	}
dc0fc74718b4a7 James Bottomley 2023-01-24  411  
dc0fc74718b4a7 James Bottomley 2023-01-24  412  	/* random number for our nonce */
dc0fc74718b4a7 James Bottomley 2023-01-24  413  	get_random_bytes(nonce, sizeof(nonce));
dc0fc74718b4a7 James Bottomley 2023-01-24  414  	memcpy(auth->our_nonce, nonce, sizeof(nonce));
dc0fc74718b4a7 James Bottomley 2023-01-24  415  	tpm_buf_append_u32(buf, auth->handle);
dc0fc74718b4a7 James Bottomley 2023-01-24  416  	/* our new nonce */
dc0fc74718b4a7 James Bottomley 2023-01-24  417  	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
dc0fc74718b4a7 James Bottomley 2023-01-24  418  	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
dc0fc74718b4a7 James Bottomley 2023-01-24  419  	tpm_buf_append_u8(buf, auth->attrs);
dc0fc74718b4a7 James Bottomley 2023-01-24  420  	/* and put a placeholder for the hmac */
dc0fc74718b4a7 James Bottomley 2023-01-24  421  	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
dc0fc74718b4a7 James Bottomley 2023-01-24  422  	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
dc0fc74718b4a7 James Bottomley 2023-01-24  423  }
dc0fc74718b4a7 James Bottomley 2023-01-24  424  EXPORT_SYMBOL(tpm_buf_append_hmac_session);
dc0fc74718b4a7 James Bottomley 2023-01-24  425  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests