[PATCH v2] tpm: Opt-in in disable PCR integrity protection

[PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Jarkko Sakkinen]

The initial HMAC session feature added TPM bus encryption and/or integrity
protection to various in-kernel TPM operations. This can cause performance
bottlenecks with IMA, as it heavily utilizes PCR extend operations.

In order to mitigate this performance issue, introduce a kernel
command-line parameter to the TPM driver for disabling the integrity
protection for PCR extension.

Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Link: https://lore.kernel.org/linux-integrity/20241015193916.59964-1-zohar@linux.ibm.com/
Fixes: 6519fea6fd37 ("tpm: add hmac checks to tpm2_pcr_extend()")
Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
- Move tpm_buf_append_handle() to the correct file, remove spurious
  parameter (name), include error on TPM2B and add documentation.
  Keep the declaration in linux/tpm.h despite not exported as it
  is easiest to maintain tpm_buf_* in a single header.
- Rename kernel command-line option as "disable_pcr_integrity_protection",
  as Mimi pointed out it does not carry SA_ENCRYPT flag.
v1:
- Derived from the earlier RFC patch with a different parameter scope,
  cleaner commit message and some other tweaks. I decided to create
  something because I did not noticed any progress. Note only compile
  tested as I wanted to get something quickly out.
---
 .../admin-guide/kernel-parameters.txt         | 10 ++++
 drivers/char/tpm/tpm-buf.c                    | 20 ++++++++
 drivers/char/tpm/tpm2-cmd.c                   | 30 ++++++++---
 drivers/char/tpm/tpm2-sessions.c              | 51 ++++++++++---------
 include/linux/tpm.h                           |  3 ++
 5 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1518343bbe22..9fc406b20a74 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6727,6 +6727,16 @@
 	torture.verbose_sleep_duration= [KNL]
 			Duration of each verbose-printk() sleep in jiffies.
 
+	tpm.disable_pcr_integrity_protection= [HW,TPM]
+			Do not protect PCR registers from unintended physical
+			access, or interposers in the bus by the means of
+			having an encrypted and integrity protected session
+			wrapped around TPM2_PCR_Extend command. Consider this
+			in a situation where TPM is heavily utilized by
+			IMA, thus protection causing a major performance hit,
+			and the space where machines are deployed is by other
+			means guarded.
+
 	tpm_suspend_pcr=[HW,TPM]
 			Format: integer pcr id
 			Specify that at suspend time, the tpm driver
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index cad0048bcc3c..e49a19fea3bd 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -146,6 +146,26 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 }
 EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
 
+/**
+ * tpm_buf_append_handle() - Add a handle
+ * @chip:	&tpm_chip instance
+ * @buf:	&tpm_buf instance
+ * @handle:	a TPM object handle
+ *
+ * Add a handle to the buffer, and increase the count tracking the number of
+ * handles in the command buffer. Works only for command buffers.
+ */
+void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
+{
+	if (buf->flags & TPM_BUF_TPM2B) {
+		dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
+		return;
+	}
+
+	tpm_buf_append_u32(buf, handle);
+	buf->handles++;
+}
+
 /**
  * tpm_buf_read() - Read from a TPM buffer
  * @buf:	&tpm_buf instance
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 1e856259219e..cc443bcf15e8 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -14,6 +14,10 @@
 #include "tpm.h"
 #include <crypto/hash_info.h>
 
+static bool disable_pcr_integrity_protection;
+module_param(disable_pcr_integrity_protection, bool, 0444);
+MODULE_PARM_DESC(disable_pcr_integrity_protection, "Disable TPM2_PCR_Extend encryption");
+
 static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
@@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	int rc;
 	int i;
 
-	rc = tpm2_start_auth_session(chip);
-	if (rc)
-		return rc;
+	if (!disable_pcr_integrity_protection) {
+		rc = tpm2_start_auth_session(chip);
+		if (rc)
+			return rc;
+	}
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
 	if (rc) {
-		tpm2_end_auth_session(chip);
+		if (!disable_pcr_integrity_protection)
+			tpm2_end_auth_session(chip);
 		return rc;
 	}
 
-	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
-	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
+	if (!disable_pcr_integrity_protection) {
+		tpm_buf_append_name(chip, &buf, pcr_idx);
+		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
+	} else {
+		tpm_buf_append_handle(chip, &buf, pcr_idx);
+		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
+	}
 
 	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
 
@@ -253,9 +265,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 			       chip->allocated_banks[i].digest_size);
 	}
 
-	tpm_buf_fill_hmac_session(chip, &buf);
+	if (!disable_pcr_integrity_protection)
+		tpm_buf_fill_hmac_session(chip, &buf);
 	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
-	rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+	if (!disable_pcr_integrity_protection)
+		rc = tpm_buf_check_hmac_response(chip, &buf, rc);
 
 	tpm_buf_destroy(&buf);
 
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 42df980168b6..a7c1b162251b 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -237,9 +237,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 #endif
 
 	if (!tpm2_chip_auth(chip)) {
-		tpm_buf_append_u32(buf, handle);
-		/* count the number of handles in the upper bits of flags */
-		buf->handles++;
+		tpm_buf_append_handle(chip, buf, handle);
 		return;
 	}
 
@@ -272,6 +270,31 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 }
 EXPORT_SYMBOL_GPL(tpm_buf_append_name);
 
+void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
+			 u8 attributes, u8 *passphrase, int passphrase_len)
+{
+	/* offset tells us where the sessions area begins */
+	int offset = buf->handles * 4 + TPM_HEADER_SIZE;
+	u32 len = 9 + passphrase_len;
+
+	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, passphrase_len);
+	tpm_buf_append(buf, passphrase, passphrase_len);
+}
+
 /**
  * tpm_buf_append_hmac_session() - Append a TPM session element
  * @chip: the TPM chip structure
@@ -309,26 +332,8 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
 #endif
 
 	if (!tpm2_chip_auth(chip)) {
-		/* offset tells us where the sessions area begins */
-		int offset = buf->handles * 4 + TPM_HEADER_SIZE;
-		u32 len = 9 + passphrase_len;
-
-		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, passphrase_len);
-		tpm_buf_append(buf, passphrase, passphrase_len);
+		tpm_buf_append_auth(chip, buf, attributes, passphrase,
+				    passphrase_len);
 		return;
 	}
 
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 587b96b4418e..20a40ade8030 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -421,6 +421,7 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value);
 u8 tpm_buf_read_u8(struct tpm_buf *buf, off_t *offset);
 u16 tpm_buf_read_u16(struct tpm_buf *buf, off_t *offset);
 u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset);
+void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle);
 
 /*
  * Check if TPM device is in the firmware upgrade mode.
@@ -505,6 +506,8 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
 				 u8 attributes, u8 *passphrase,
 				 int passphraselen);
+void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
+			 u8 attributes, u8 *passphrase, int passphraselen);
 static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
 						   struct tpm_buf *buf,
 						   u8 attributes,
-- 
2.47.0

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[James Bottomley]

On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
[...]
> +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
> +                        u8 attributes, u8 *passphrase, int
> passphrase_len)
> +{
> +       /* offset tells us where the sessions area begins */
> +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> +       u32 len = 9 + passphrase_len;
> +
> +       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, passphrase_len);
> +       tpm_buf_append(buf, passphrase, passphrase_len);
> +}
> +

The rest of the code looks fine, but if you're going to extract this as
a separate function instead of doing the open coded struct
tpm2_null_auth that was there originally, you should probably extract
and use the tpm2_buf_append_auth() function in trusted_tpm2.c

James

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Mimi Zohar]

On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> The initial HMAC session feature added TPM bus encryption and/or integrity
> protection to various in-kernel TPM operations. This can cause performance
> bottlenecks with IMA, as it heavily utilizes PCR extend operations.
> 
> In order to mitigate this performance issue, introduce a kernel
> command-line parameter to the TPM driver for disabling the integrity
> protection for PCR extension.
> 
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Link: https://lore.kernel.org/linux-integrity/20241015193916.59964-1-zohar@linux.ibm.com/
> Fixes: 6519fea6fd37 ("tpm: add hmac checks to tpm2_pcr_extend()")
> Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Co-developed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v2:
> - Move tpm_buf_append_handle() to the correct file, remove spurious
>   parameter (name), include error on TPM2B and add documentation.
>   Keep the declaration in linux/tpm.h despite not exported as it
>   is easiest to maintain tpm_buf_* in a single header.
> - Rename kernel command-line option as "disable_pcr_integrity_protection",
>   as Mimi pointed out it does not carry SA_ENCRYPT flag.
> v1:
> - Derived from the earlier RFC patch with a different parameter scope,
>   cleaner commit message and some other tweaks. I decided to create
>   something because I did not noticed any progress. Note only compile
>   tested as I wanted to get something quickly out.
> ---
>  .../admin-guide/kernel-parameters.txt         | 10 ++++
>  drivers/char/tpm/tpm-buf.c                    | 20 ++++++++
>  drivers/char/tpm/tpm2-cmd.c                   | 30 ++++++++---
>  drivers/char/tpm/tpm2-sessions.c              | 51 ++++++++++---------
>  include/linux/tpm.h                           |  3 ++
>  5 files changed, 83 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1518343bbe22..9fc406b20a74 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6727,6 +6727,16 @@
>  	torture.verbose_sleep_duration= [KNL]
>  			Duration of each verbose-printk() sleep in jiffies.
>  
> +	tpm.disable_pcr_integrity_protection= [HW,TPM]
> +			Do not protect PCR registers from unintended physical
> +			access, or interposers in the bus by the means of
> +			having an encrypted and integrity protected session

"encrypted" isn't needed here.

> +			wrapped around TPM2_PCR_Extend command. Consider this
> +			in a situation where TPM is heavily utilized by
> +			IMA, thus protection causing a major performance hit,
> +			and the space where machines are deployed is by other
> +			means guarded.
> +
>  	tpm_suspend_pcr=[HW,TPM]
>  			Format: integer pcr id
>  			Specify that at suspend time, the tpm driver
> diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> index cad0048bcc3c..e49a19fea3bd 100644
> --- a/drivers/char/tpm/tpm-buf.c
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -146,6 +146,26 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
>  }
>  EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
>  
> +/**
> + * tpm_buf_append_handle() - Add a handle
> + * @chip:	&tpm_chip instance
> + * @buf:	&tpm_buf instance
> + * @handle:	a TPM object handle
> + *
> + * Add a handle to the buffer, and increase the count tracking the number of
> + * handles in the command buffer. Works only for command buffers.
> + */
> +void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
> +{
> +	if (buf->flags & TPM_BUF_TPM2B) {
> +		dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
> +		return;
> +	}
> +
> +	tpm_buf_append_u32(buf, handle);
> +	buf->handles++;
> +}
> +
>  /**
>   * tpm_buf_read() - Read from a TPM buffer
>   * @buf:	&tpm_buf instance
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 1e856259219e..cc443bcf15e8 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -14,6 +14,10 @@
>  #include "tpm.h"
>  #include <crypto/hash_info.h>
>  
> +static bool disable_pcr_integrity_protection;
> +module_param(disable_pcr_integrity_protection, bool, 0444);
> +MODULE_PARM_DESC(disable_pcr_integrity_protection, "Disable TPM2_PCR_Extend encryption");

I like the name 'disable_pcr_integrity_protection.  However, the name and
description doesn't match.  Replace 'encryption' with 'integrity protection'.

> +
>  static struct tpm2_hash tpm2_hash_map[] = {
>  	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
>  	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  	int rc;
>  	int i;
>  
> -	rc = tpm2_start_auth_session(chip);
> -	if (rc)
> -		return rc;
> +	if (!disable_pcr_integrity_protection) {
> +		rc = tpm2_start_auth_session(chip);
> +		if (rc)
> +			return rc;
> +	}
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>  	if (rc) {
> -		tpm2_end_auth_session(chip);
> +		if (!disable_pcr_integrity_protection)
> +			tpm2_end_auth_session(chip);
>  		return rc;
>  	}
>  
> -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> +	if (!disable_pcr_integrity_protection) {
> +		tpm_buf_append_name(chip, &buf, pcr_idx);

tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
here.


> +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> +	} else {
> +		tpm_buf_append_handle(chip, &buf, pcr_idx);

Or here.

> +		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
> +	}
>  
>  	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
>  
> 

Mimi

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[kernel test robot]

Hi Jarkko,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc6 next-20241106]
[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/Jarkko-Sakkinen/tpm-Opt-in-in-disable-PCR-integrity-protection/20241107-175515
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241107095138.78209-1-jarkko%40kernel.org
patch subject: [PATCH v2] tpm: Opt-in in disable PCR integrity protection
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241107/202411072138.bgOIL36O-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241107/202411072138.bgOIL36O-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411072138.bgOIL36O-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/char/tpm/tpm2-cmd.c: In function 'tpm2_pcr_extend':
>> drivers/char/tpm/tpm2-cmd.c:253:17: error: too few arguments to function 'tpm_buf_append_name'
     253 |                 tpm_buf_append_name(chip, &buf, pcr_idx);
         |                 ^~~~~~~~~~~~~~~~~~~
   In file included from drivers/char/tpm/tpm.h:27,
                    from drivers/char/tpm/tpm2-cmd.c:14:
   include/linux/tpm.h:504:6: note: declared here
     504 | void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
         |      ^~~~~~~~~~~~~~~~~~~


vim +/tpm_buf_append_name +253 drivers/char/tpm/tpm2-cmd.c

   222	
   223	/**
   224	 * tpm2_pcr_extend() - extend a PCR value
   225	 *
   226	 * @chip:	TPM chip to use.
   227	 * @pcr_idx:	index of the PCR.
   228	 * @digests:	list of pcr banks and corresponding digest values to extend.
   229	 *
   230	 * Return: Same as with tpm_transmit_cmd.
   231	 */
   232	int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
   233			    struct tpm_digest *digests)
   234	{
   235		struct tpm_buf buf;
   236		int rc;
   237		int i;
   238	
   239		if (!disable_pcr_integrity_protection) {
   240			rc = tpm2_start_auth_session(chip);
   241			if (rc)
   242				return rc;
   243		}
   244	
   245		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
   246		if (rc) {
   247			if (!disable_pcr_integrity_protection)
   248				tpm2_end_auth_session(chip);
   249			return rc;
   250		}
   251	
   252		if (!disable_pcr_integrity_protection) {
 > 253			tpm_buf_append_name(chip, &buf, pcr_idx);
   254			tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
   255		} else {
   256			tpm_buf_append_handle(chip, &buf, pcr_idx);
   257			tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
   258		}
   259	
   260		tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
   261	
   262		for (i = 0; i < chip->nr_allocated_banks; i++) {
   263			tpm_buf_append_u16(&buf, digests[i].alg_id);
   264			tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
   265				       chip->allocated_banks[i].digest_size);
   266		}
   267	
   268		if (!disable_pcr_integrity_protection)
   269			tpm_buf_fill_hmac_session(chip, &buf);
   270		rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
   271		if (!disable_pcr_integrity_protection)
   272			rc = tpm_buf_check_hmac_response(chip, &buf, rc);
   273	
   274		tpm_buf_destroy(&buf);
   275	
   276		return rc;
   277	}
   278	

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

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Jarkko Sakkinen]

On Thu Nov 7, 2024 at 3:44 PM EET, Mimi Zohar wrote:
> > +	tpm.disable_pcr_integrity_protection= [HW,TPM]
> > +			Do not protect PCR registers from unintended physical
> > +			access, or interposers in the bus by the means of
> > +			having an encrypted and integrity protected session
>
> "encrypted" isn't needed here.

fixing

>
> > +			wrapped around TPM2_PCR_Extend command. Consider this
> > +			in a situation where TPM is heavily utilized by
> > +			IMA, thus protection causing a major performance hit,
> > +			and the space where machines are deployed is by other
> > +			means guarded.
> > +
> >  	tpm_suspend_pcr=[HW,TPM]
> >  			Format: integer pcr id
> >  			Specify that at suspend time, the tpm driver
> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> > index cad0048bcc3c..e49a19fea3bd 100644
> > --- a/drivers/char/tpm/tpm-buf.c
> > +++ b/drivers/char/tpm/tpm-buf.c
> > @@ -146,6 +146,26 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
> >  }
> >  EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
> >  
> > +/**
> > + * tpm_buf_append_handle() - Add a handle
> > + * @chip:	&tpm_chip instance
> > + * @buf:	&tpm_buf instance
> > + * @handle:	a TPM object handle
> > + *
> > + * Add a handle to the buffer, and increase the count tracking the number of
> > + * handles in the command buffer. Works only for command buffers.
> > + */
> > +void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
> > +{
> > +	if (buf->flags & TPM_BUF_TPM2B) {
> > +		dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
> > +		return;
> > +	}
> > +
> > +	tpm_buf_append_u32(buf, handle);
> > +	buf->handles++;
> > +}
> > +
> >  /**
> >   * tpm_buf_read() - Read from a TPM buffer
> >   * @buf:	&tpm_buf instance
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 1e856259219e..cc443bcf15e8 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -14,6 +14,10 @@
> >  #include "tpm.h"
> >  #include <crypto/hash_info.h>
> >  
> > +static bool disable_pcr_integrity_protection;
> > +module_param(disable_pcr_integrity_protection, bool, 0444);
> > +MODULE_PARM_DESC(disable_pcr_integrity_protection, "Disable TPM2_PCR_Extend encryption");
>
> I like the name 'disable_pcr_integrity_protection.  However, the name and
> description doesn't match.  Replace 'encryption' with 'integrity protection'.

Weird, I changed that. I don't know  how it ended up being like that.

>
> > +
> >  static struct tpm2_hash tpm2_hash_map[] = {
> >  	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
> >  	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> > @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> >  	int rc;
> >  	int i;
> >  
> > -	rc = tpm2_start_auth_session(chip);
> > -	if (rc)
> > -		return rc;
> > +	if (!disable_pcr_integrity_protection) {
> > +		rc = tpm2_start_auth_session(chip);
> > +		if (rc)
> > +			return rc;
> > +	}
> >  
> >  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> >  	if (rc) {
> > -		tpm2_end_auth_session(chip);
> > +		if (!disable_pcr_integrity_protection)
> > +			tpm2_end_auth_session(chip);
> >  		return rc;
> >  	}
> >  
> > -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > +	if (!disable_pcr_integrity_protection) {
> > +		tpm_buf_append_name(chip, &buf, pcr_idx);
>
> tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
> here.

Hmm... weird I'll check this. Maybe I had something left to staging...

>
>
> > +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > +	} else {
> > +		tpm_buf_append_handle(chip, &buf, pcr_idx);
>

> Or here.

Here I think it is appropriate

>
> > +		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
> > +	}
> >  
> >  	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
> >  
> > 
>
> Mimi


BR, Jarkko

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Jarkko Sakkinen]

On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> [...]
> > +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
> > +                        u8 attributes, u8 *passphrase, int
> > passphrase_len)
> > +{
> > +       /* offset tells us where the sessions area begins */
> > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > +       u32 len = 9 + passphrase_len;
> > +
> > +       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, passphrase_len);
> > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > +}
> > +
>
> The rest of the code looks fine, but if you're going to extract this as
> a separate function instead of doing the open coded struct
> tpm2_null_auth that was there originally, you should probably extract
> and use the tpm2_buf_append_auth() function in trusted_tpm2.c

So this was straight up from Mimi's original patch :-)

Hmm... was there duplicate use for this in the patch? I'll check this.

>
> James

BR, Jarkko

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[James Bottomley]

On Thu, 2024-11-07 at 15:49 +0200, Jarkko Sakkinen wrote:
> On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> > On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> > [...]
> > > +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf
> > > *buf,
> > > +                        u8 attributes, u8 *passphrase, int
> > > passphrase_len)
> > > +{
> > > +       /* offset tells us where the sessions area begins */
> > > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > > +       u32 len = 9 + passphrase_len;
> > > +
> > > +       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, passphrase_len);
> > > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > > +}
> > > +
> > 
> > The rest of the code looks fine, but if you're going to extract
> > this as a separate function instead of doing the open coded struct
> > tpm2_null_auth that was there originally, you should probably
> > extract and use the tpm2_buf_append_auth() function in
> > trusted_tpm2.c
> 
> So this was straight up from Mimi's original patch :-)

Yes, I had the same comment prepped for that too.

> Hmm... was there duplicate use for this in the patch? I'll check
> this.

The original open coded the empty auth append with struct
tpm2_null_auth since it's the only user.  However, since we do have
another user in trusted keys, it might make sense to consolidate.

James

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Mimi Zohar]

On Thu, 2024-11-07 at 15:47 +0200, Jarkko Sakkinen wrote:
> On Thu Nov 7, 2024 at 3:44 PM EET, Mimi Zohar wrote:
> > > 
> > > @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > >  	int rc;
> > >  	int i;
> > >  
> > > -	rc = tpm2_start_auth_session(chip);
> > > -	if (rc)
> > > -		return rc;
> > > +	if (!disable_pcr_integrity_protection) {
> > > +		rc = tpm2_start_auth_session(chip);
> > > +		if (rc)
> > > +			return rc;
> > > +	}
> > >  
> > >  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> > >  	if (rc) {
> > > -		tpm2_end_auth_session(chip);
> > > +		if (!disable_pcr_integrity_protection)
> > > +			tpm2_end_auth_session(chip);
> > >  		return rc;
> > >  	}
> > >  
> > > -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > +	if (!disable_pcr_integrity_protection) {
> > > +		tpm_buf_append_name(chip, &buf, pcr_idx);
> > 
> > tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
> > here.
> 
> Hmm... weird I'll check this. Maybe I had something left to staging...
> 
> > 
> > 
> > > +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > +	} else {
> > > +		tpm_buf_append_handle(chip, &buf, pcr_idx);
> > 
> 
> > Or here.
> 
> Here I think it is appropriate

Agreed

> 
> > 
> > > +		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
> > > +	}
> > >  
> > >  	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
> > > 

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Jarkko Sakkinen]

On Thu Nov 7, 2024 at 4:00 PM EET, Mimi Zohar wrote:
> On Thu, 2024-11-07 at 15:47 +0200, Jarkko Sakkinen wrote:
> > On Thu Nov 7, 2024 at 3:44 PM EET, Mimi Zohar wrote:
> > > > 
> > > > @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > > >  	int rc;
> > > >  	int i;
> > > >  
> > > > -	rc = tpm2_start_auth_session(chip);
> > > > -	if (rc)
> > > > -		return rc;
> > > > +	if (!disable_pcr_integrity_protection) {
> > > > +		rc = tpm2_start_auth_session(chip);
> > > > +		if (rc)
> > > > +			return rc;
> > > > +	}
> > > >  
> > > >  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> > > >  	if (rc) {
> > > > -		tpm2_end_auth_session(chip);
> > > > +		if (!disable_pcr_integrity_protection)
> > > > +			tpm2_end_auth_session(chip);
> > > >  		return rc;
> > > >  	}
> > > >  
> > > > -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > > -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > > +	if (!disable_pcr_integrity_protection) {
> > > > +		tpm_buf_append_name(chip, &buf, pcr_idx);
> > > 
> > > tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
> > > here.
> > 
> > Hmm... weird I'll check this. Maybe I had something left to staging...

Yes! This was correct in my clone but not in the patch.

Clearly a sign that I wait until next week before sending a new version
:-)


> > 
> > > 
> > > 
> > > > +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > > +	} else {
> > > > +		tpm_buf_append_handle(chip, &buf, pcr_idx);
> > > 
> > 
> > > Or here.
> > 
> > Here I think it is appropriate
>
> Agreed

Great

BR, Jarkko

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[kernel test robot]

Hi Jarkko,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc6 next-20241107]
[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/Jarkko-Sakkinen/tpm-Opt-in-in-disable-PCR-integrity-protection/20241107-175515
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241107095138.78209-1-jarkko%40kernel.org
patch subject: [PATCH v2] tpm: Opt-in in disable PCR integrity protection
config: arm-randconfig-003-20241108 (https://download.01.org/0day-ci/archive/20241108/202411080436.fb2O1apj-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/202411080436.fb2O1apj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411080436.fb2O1apj-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/char/tpm/tpm2-cmd.c:14:
   In file included from drivers/char/tpm/tpm.h:28:
   include/linux/tpm_eventlog.h:167:6: warning: variable 'mapping_size' set but not used [-Wunused-but-set-variable]
           int mapping_size;
               ^
>> drivers/char/tpm/tpm2-cmd.c:253:42: error: too few arguments to function call, expected 4, have 3
                   tpm_buf_append_name(chip, &buf, pcr_idx);
                   ~~~~~~~~~~~~~~~~~~~                    ^
   include/linux/tpm.h:504:6: note: 'tpm_buf_append_name' declared here
   void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
        ^
   1 warning and 1 error generated.


vim +253 drivers/char/tpm/tpm2-cmd.c

   222	
   223	/**
   224	 * tpm2_pcr_extend() - extend a PCR value
   225	 *
   226	 * @chip:	TPM chip to use.
   227	 * @pcr_idx:	index of the PCR.
   228	 * @digests:	list of pcr banks and corresponding digest values to extend.
   229	 *
   230	 * Return: Same as with tpm_transmit_cmd.
   231	 */
   232	int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
   233			    struct tpm_digest *digests)
   234	{
   235		struct tpm_buf buf;
   236		int rc;
   237		int i;
   238	
   239		if (!disable_pcr_integrity_protection) {
   240			rc = tpm2_start_auth_session(chip);
   241			if (rc)
   242				return rc;
   243		}
   244	
   245		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
   246		if (rc) {
   247			if (!disable_pcr_integrity_protection)
   248				tpm2_end_auth_session(chip);
   249			return rc;
   250		}
   251	
   252		if (!disable_pcr_integrity_protection) {
 > 253			tpm_buf_append_name(chip, &buf, pcr_idx);
   254			tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
   255		} else {
   256			tpm_buf_append_handle(chip, &buf, pcr_idx);
   257			tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
   258		}
   259	
   260		tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
   261	
   262		for (i = 0; i < chip->nr_allocated_banks; i++) {
   263			tpm_buf_append_u16(&buf, digests[i].alg_id);
   264			tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
   265				       chip->allocated_banks[i].digest_size);
   266		}
   267	
   268		if (!disable_pcr_integrity_protection)
   269			tpm_buf_fill_hmac_session(chip, &buf);
   270		rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
   271		if (!disable_pcr_integrity_protection)
   272			rc = tpm_buf_check_hmac_response(chip, &buf, rc);
   273	
   274		tpm_buf_destroy(&buf);
   275	
   276		return rc;
   277	}
   278	

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

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Mimi Zohar]

On Thu, 2024-11-07 at 08:52 -0500, James Bottomley wrote:
> On Thu, 2024-11-07 at 15:49 +0200, Jarkko Sakkinen wrote:
> > On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> > > On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> > > [...]
> > > > +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf
> > > > *buf,
> > > > +                        u8 attributes, u8 *passphrase, int
> > > > passphrase_len)
> > > > +{
> > > > +       /* offset tells us where the sessions area begins */
> > > > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > > > +       u32 len = 9 + passphrase_len;
> > > > +
> > > > +       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, passphrase_len);
> > > > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > > > +}
> > > > +
> > > 
> > > The rest of the code looks fine, but if you're going to extract
> > > this as a separate function instead of doing the open coded struct
> > > tpm2_null_auth that was there originally, you should probably
> > > extract and use the tpm2_buf_append_auth() function in
> > > trusted_tpm2.c
> > 
> > So this was straight up from Mimi's original patch :-)
> 
> Yes, I had the same comment prepped for that too.

But it wasn't sent ...
> 
> > Hmm... was there duplicate use for this in the patch? I'll check
> > this.
> 
> The original open coded the empty auth append with struct
> tpm2_null_auth since it's the only user.  However, since we do have
> another user in trusted keys, it might make sense to consolidate.

Instead of delaying the current patch from being upstreamed, perhaps this change
can be deferred?

thanks,

Mimi

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[James Bottomley]

On Mon, 2024-11-11 at 14:53 -0500, Mimi Zohar wrote:
> On Thu, 2024-11-07 at 08:52 -0500, James Bottomley wrote:
> > On Thu, 2024-11-07 at 15:49 +0200, Jarkko Sakkinen wrote:
> > > On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> > > > On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> > > > [...]
> > > > > +void tpm_buf_append_auth(struct tpm_chip *chip, struct
> > > > > tpm_buf
> > > > > *buf,
> > > > > +                        u8 attributes, u8 *passphrase, int
> > > > > passphrase_len)
> > > > > +{
> > > > > +       /* offset tells us where the sessions area begins */
> > > > > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > > > > +       u32 len = 9 + passphrase_len;
> > > > > +
> > > > > +       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, passphrase_len);
> > > > > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > > > > +}
> > > > > +
> > > > 
> > > > The rest of the code looks fine, but if you're going to extract
> > > > this as a separate function instead of doing the open coded
> > > > struct
> > > > tpm2_null_auth that was there originally, you should probably
> > > > extract and use the tpm2_buf_append_auth() function in
> > > > trusted_tpm2.c
> > > 
> > > So this was straight up from Mimi's original patch :-)
> > 
> > Yes, I had the same comment prepped for that too.
> 
> But it wasn't sent ...

no.

> > 
> > > Hmm... was there duplicate use for this in the patch? I'll check
> > > this.
> > 
> > The original open coded the empty auth append with struct
> > tpm2_null_auth since it's the only user.  However, since we do have
> > another user in trusted keys, it might make sense to consolidate.
> 
> Instead of delaying the current patch from being upstreamed, perhaps
> this change can be deferred?

I don't see why not; it was the introduction of the new function rather
than going back to the struct tpm2_null_auth_area of the original that
caught my attention.

James

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Jarkko Sakkinen]

On Mon Nov 11, 2024 at 9:53 PM EET, Mimi Zohar wrote:
> > The original open coded the empty auth append with struct
> > tpm2_null_auth since it's the only user.  However, since we do have
> > another user in trusted keys, it might make sense to consolidate.
>
> Instead of delaying the current patch from being upstreamed, perhaps this change
> can be deferred?

Yes.

BR, Jarkko

Re: [PATCH v2] tpm: Opt-in in disable PCR integrity protection

[Mimi Zohar]

On Tue, 2024-11-12 at 19:57 +0200, Jarkko Sakkinen wrote:
> On Mon Nov 11, 2024 at 9:53 PM EET, Mimi Zohar wrote:
> > > The original open coded the empty auth append with struct
> > > tpm2_null_auth since it's the only user.  However, since we do have
> > > another user in trusted keys, it might make sense to consolidate.
> > 
> > Instead of delaying the current patch from being upstreamed, perhaps this change
> > can be deferred?
> 
> Yes.

Thanks.  As soon as you repost the patch, I'll re-test.

Mimi