[PATCH 0/4] tpm2-session: correct disperancies

[PATCH 0/4] tpm2-session: correct disperancies

[Jarkko Sakkinen]

Correct various disperancies in tpm2-session implementation that set knots
on improving the feature.

Jarkko Sakkinen (4):
  tpm: Use -EPERM as fallback error code in tpm_ret_to_err
  tpm2-sessions: Remove unused parameter from tpm_buf_append_auth
  tpm2-sessions: Remove unnecessary wrapper
  keys, trusted: Remove redundant helper

 drivers/char/tpm/tpm2-cmd.c               | 16 +++-
 drivers/char/tpm/tpm2-sessions.c          |  5 +-
 include/linux/tpm.h                       | 34 ++-------
 security/keys/trusted-keys/trusted_tpm2.c | 89 +++++++----------------
 4 files changed, 47 insertions(+), 97 deletions(-)

-- 
2.39.5

[PATCH 1/4] tpm: Use -EPERM as fallback error code in tpm_ret_to_err

[Jarkko Sakkinen]

From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>

Using -EFAULT here was not the best idea for tpm_ret_to_err as the fallback
error code as it is no concise with trusted keys.

Change the fallback as -EPERM, process TPM_RC_HASH also in tpm_ret_to_err,
and by these changes make the helper applicable for trusted keys.

Fixes: 539fbab37881 ("tpm: Mask TPM RC in tpm2_start_auth_session()")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
 include/linux/tpm.h                       |  9 +++++---
 security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index dc0338a783f3..667d290789ca 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -449,13 +449,16 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret)
 	if (ret < 0)
 		return ret;
 
-	switch (tpm2_rc_value(ret)) {
-	case TPM2_RC_SUCCESS:
+	if (!ret)
 		return 0;
+
+	switch (tpm2_rc_value(ret)) {
 	case TPM2_RC_SESSION_MEMORY:
 		return -ENOMEM;
+	case TPM2_RC_HASH:
+		return -EINVAL;
 	default:
-		return -EFAULT;
+		return -EPERM;
 	}
 }
 
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 024be262702f..e165b117bbca 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	}
 
 	blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+	if (blob_len < 0)
+		rc = blob_len;
 
 out:
 	tpm_buf_destroy(&sized);
 	tpm_buf_destroy(&buf);
 
-	if (rc > 0) {
-		if (tpm2_rc_value(rc) == TPM2_RC_HASH)
-			rc = -EINVAL;
-		else
-			rc = -EPERM;
-	}
-	if (blob_len < 0)
-		rc = blob_len;
-	else
+	if (!rc)
 		payload->blob_len = blob_len;
 
 out_put:
 	tpm_put_ops(chip);
-	return rc;
+	return tpm_ret_to_err(rc);
 }
 
 /**
@@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		kfree(blob);
 	tpm_buf_destroy(&buf);
 
-	if (rc > 0)
-		rc = -EPERM;
-
-	return rc;
+	return tpm_ret_to_err(rc);
 }
 
 /**
@@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	tpm_buf_fill_hmac_session(chip, &buf);
 	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
 	rc = tpm_buf_check_hmac_response(chip, &buf, rc);
-	if (rc > 0)
-		rc = -EPERM;
 
 	if (!rc) {
 		data_len = be16_to_cpup(
@@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 
 out:
 	tpm_buf_destroy(&buf);
-	return rc;
+	return tpm_ret_to_err(rc);
 }
 
 /**
@@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 
 out:
 	tpm_put_ops(chip);
-
-	return rc;
+	return tpm_ret_to_err(rc);
 }
-- 
2.39.5

Re: [PATCH 1/4] tpm: Use -EPERM as fallback error code in tpm_ret_to_err

[Jonathan McDowell]

On Mon, Sep 22, 2025 at 07:43:14PM +0300, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
>Using -EFAULT here was not the best idea for tpm_ret_to_err as the fallback
>error code as it is no concise with trusted keys.
>
>Change the fallback as -EPERM, process TPM_RC_HASH also in tpm_ret_to_err,
>and by these changes make the helper applicable for trusted keys.
>
>Fixes: 539fbab37881 ("tpm: Mask TPM RC in tpm2_start_auth_session()")
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>---
> include/linux/tpm.h                       |  9 +++++---
> security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
> 2 files changed, 13 insertions(+), 22 deletions(-)
>
>diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>index dc0338a783f3..667d290789ca 100644
>--- a/include/linux/tpm.h
>+++ b/include/linux/tpm.h
>@@ -449,13 +449,16 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret)
> 	if (ret < 0)
> 		return ret;
>
>-	switch (tpm2_rc_value(ret)) {
>-	case TPM2_RC_SUCCESS:
>+	if (!ret)
> 		return 0;

Fold this into the check above to get:

	if (ret <= 0)

?

>+
>+	switch (tpm2_rc_value(ret)) {
> 	case TPM2_RC_SESSION_MEMORY:
> 		return -ENOMEM;
>+	case TPM2_RC_HASH:
>+		return -EINVAL;
> 	default:
>-		return -EFAULT;
>+		return -EPERM;
> 	}
> }
>
>diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>index 024be262702f..e165b117bbca 100644
>--- a/security/keys/trusted-keys/trusted_tpm2.c
>+++ b/security/keys/trusted-keys/trusted_tpm2.c
>@@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> 	}
>
> 	blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
>+	if (blob_len < 0)
>+		rc = blob_len;
>
> out:
> 	tpm_buf_destroy(&sized);
> 	tpm_buf_destroy(&buf);
>
>-	if (rc > 0) {
>-		if (tpm2_rc_value(rc) == TPM2_RC_HASH)
>-			rc = -EINVAL;
>-		else
>-			rc = -EPERM;
>-	}
>-	if (blob_len < 0)
>-		rc = blob_len;
>-	else
>+	if (!rc)
> 		payload->blob_len = blob_len;
>
> out_put:
> 	tpm_put_ops(chip);
>-	return rc;
>+	return tpm_ret_to_err(rc);
> }
>
> /**
>@@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> 		kfree(blob);
> 	tpm_buf_destroy(&buf);
>
>-	if (rc > 0)
>-		rc = -EPERM;
>-
>-	return rc;
>+	return tpm_ret_to_err(rc);
> }
>
> /**
>@@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> 	tpm_buf_fill_hmac_session(chip, &buf);
> 	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
> 	rc = tpm_buf_check_hmac_response(chip, &buf, rc);
>-	if (rc > 0)
>-		rc = -EPERM;
>
> 	if (!rc) {
> 		data_len = be16_to_cpup(
>@@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>
> out:
> 	tpm_buf_destroy(&buf);
>-	return rc;
>+	return tpm_ret_to_err(rc);
> }
>
> /**
>@@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
>
> out:
> 	tpm_put_ops(chip);
>-
>-	return rc;
>+	return tpm_ret_to_err(rc);
> }
>-- 
>2.39.5
>
>

J.

-- 
Web [  Sleep?  Isn't that some inferior replacement for caffeine?  ]
site: https:// [                                          ]      Made by
www.earth.li/~noodles/  [                      ]         HuggieTag 0.0.24

Re: [PATCH 1/4] tpm: Use -EPERM as fallback error code in tpm_ret_to_err

[Jarkko Sakkinen]

On Wed, Sep 24, 2025 at 09:32:33AM +0100, Jonathan McDowell wrote:
> On Mon, Sep 22, 2025 at 07:43:14PM +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > 
> > Using -EFAULT here was not the best idea for tpm_ret_to_err as the fallback
> > error code as it is no concise with trusted keys.
> > 
> > Change the fallback as -EPERM, process TPM_RC_HASH also in tpm_ret_to_err,
> > and by these changes make the helper applicable for trusted keys.
> > 
> > Fixes: 539fbab37881 ("tpm: Mask TPM RC in tpm2_start_auth_session()")
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > ---
> > include/linux/tpm.h                       |  9 +++++---
> > security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
> > 2 files changed, 13 insertions(+), 22 deletions(-)
> > 
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index dc0338a783f3..667d290789ca 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -449,13 +449,16 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret)
> > 	if (ret < 0)
> > 		return ret;
> > 
> > -	switch (tpm2_rc_value(ret)) {
> > -	case TPM2_RC_SUCCESS:
> > +	if (!ret)
> > 		return 0;
> 
> Fold this into the check above to get:
> 
> 	if (ret <= 0)
> 
> ?

This is really a glitch in this patch, and I think following what
Stefano suggested is the right call:

https://lore.kernel.org/linux-integrity/tnxfamnvxoanaihka3em7ktmzkervoea43zn2l3mqxvnuivb6n@p5nn34vns3zf/

I.e., a random change to something that was not broken in the first
place :-) Never a good idea (not even in microscale), except something
super cosmetic like maybe grouping constants and stylistic stuff like
that.

BR, Jarkko

[PATCH 2/4] tpm2-sessions: Remove unused parameter from tpm_buf_append_auth

[Jarkko Sakkinen]

From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>

In earlier bug fix, an extra parameter was by mistake to the function.

Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
 drivers/char/tpm/tpm2-cmd.c      |  2 +-
 drivers/char/tpm/tpm2-sessions.c |  5 ++---
 include/linux/tpm.h              | 25 +------------------------
 3 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7d77f6fbc152..61a4daaef292 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -191,7 +191,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 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_auth(chip, &buf, NULL, 0);
 	}
 
 	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 6d03c224e6b2..13f019d1312a 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -266,7 +266,7 @@ 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)
+			 u8 *passphrase, int passphrase_len)
 {
 	/* offset tells us where the sessions area begins */
 	int offset = buf->handles * 4 + TPM_HEADER_SIZE;
@@ -327,8 +327,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
 #endif
 
 	if (!tpm2_chip_auth(chip)) {
-		tpm_buf_append_auth(chip, buf, attributes, passphrase,
-				    passphrase_len);
+		tpm_buf_append_auth(chip, buf, passphrase, passphrase_len);
 		return;
 	}
 
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 667d290789ca..a8984d273c28 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -533,30 +533,7 @@ 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,
-						   u8 *passphrase,
-						   int passphraselen)
-{
-	struct tpm_header *head;
-	int offset;
-
-	if (tpm2_chip_auth(chip)) {
-		tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
-	} else  {
-		offset = buf->handles * 4 + TPM_HEADER_SIZE;
-		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);
-	}
-}
+			 u8 *passphrase, int passphraselen);
 
 #ifdef CONFIG_TCG_TPM2_HMAC
 
-- 
2.39.5

Re: [PATCH 2/4] tpm2-sessions: Remove unused parameter from tpm_buf_append_auth

[Jonathan McDowell]

On Mon, Sep 22, 2025 at 07:43:15PM +0300, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
>In earlier bug fix, an extra parameter was by mistake to the function.
>
>Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection")
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>---
> drivers/char/tpm/tpm2-cmd.c      |  2 +-
> drivers/char/tpm/tpm2-sessions.c |  5 ++---
> include/linux/tpm.h              | 25 +------------------------
> 3 files changed, 4 insertions(+), 28 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>index 7d77f6fbc152..61a4daaef292 100644
>--- a/drivers/char/tpm/tpm2-cmd.c
>+++ b/drivers/char/tpm/tpm2-cmd.c
>@@ -191,7 +191,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 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_auth(chip, &buf, NULL, 0);
> 	}
>
> 	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
>diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
>index 6d03c224e6b2..13f019d1312a 100644
>--- a/drivers/char/tpm/tpm2-sessions.c
>+++ b/drivers/char/tpm/tpm2-sessions.c
>@@ -266,7 +266,7 @@ 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)
>+			 u8 *passphrase, int passphrase_len)
> {
> 	/* offset tells us where the sessions area begins */
> 	int offset = buf->handles * 4 + TPM_HEADER_SIZE;
>@@ -327,8 +327,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
> #endif
>
> 	if (!tpm2_chip_auth(chip)) {
>-		tpm_buf_append_auth(chip, buf, attributes, passphrase,
>-				    passphrase_len);
>+		tpm_buf_append_auth(chip, buf, passphrase, passphrase_len);
> 		return;
> 	}

This chunk below should be in patch 3/4 where you open code 
tpm_buf_append_hmac_session_opt, rather than here:

>diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>index 667d290789ca..a8984d273c28 100644
>--- a/include/linux/tpm.h
>+++ b/include/linux/tpm.h
>@@ -533,30 +533,7 @@ 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,
>-						   u8 *passphrase,
>-						   int passphraselen)
>-{
>-	struct tpm_header *head;
>-	int offset;
>-
>-	if (tpm2_chip_auth(chip)) {
>-		tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
>-	} else  {
>-		offset = buf->handles * 4 + TPM_HEADER_SIZE;
>-		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);
>-	}
>-}
>+			 u8 *passphrase, int passphraselen);
>
> #ifdef CONFIG_TCG_TPM2_HMAC
>
>-- 
>2.39.5
>
>

J.

-- 
101 things you can't have too much of : 19 - A Good Thing.
This .sig brought to you by the letter V and the number 13
Product of the Republic of HuggieTag

Re: [PATCH 2/4] tpm2-sessions: Remove unused parameter from tpm_buf_append_auth

[Jarkko Sakkinen]

On Wed, Sep 24, 2025 at 09:47:59AM +0100, Jonathan McDowell wrote:
> On Mon, Sep 22, 2025 at 07:43:15PM +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > 
> > In earlier bug fix, an extra parameter was by mistake to the function.
> > 
> > Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection")
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > ---
> > drivers/char/tpm/tpm2-cmd.c      |  2 +-
> > drivers/char/tpm/tpm2-sessions.c |  5 ++---
> > include/linux/tpm.h              | 25 +------------------------
> > 3 files changed, 4 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 7d77f6fbc152..61a4daaef292 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -191,7 +191,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 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_auth(chip, &buf, NULL, 0);
> > 	}
> > 
> > 	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
> > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> > index 6d03c224e6b2..13f019d1312a 100644
> > --- a/drivers/char/tpm/tpm2-sessions.c
> > +++ b/drivers/char/tpm/tpm2-sessions.c
> > @@ -266,7 +266,7 @@ 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)
> > +			 u8 *passphrase, int passphrase_len)
> > {
> > 	/* offset tells us where the sessions area begins */
> > 	int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > @@ -327,8 +327,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
> > #endif
> > 
> > 	if (!tpm2_chip_auth(chip)) {
> > -		tpm_buf_append_auth(chip, buf, attributes, passphrase,
> > -				    passphrase_len);
> > +		tpm_buf_append_auth(chip, buf, passphrase, passphrase_len);
> > 		return;
> > 	}
> 
> This chunk below should be in patch 3/4 where you open code
> tpm_buf_append_hmac_session_opt, rather than here:

True, thanks for catching this.

> 
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index 667d290789ca..a8984d273c28 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -533,30 +533,7 @@ 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,
> > -						   u8 *passphrase,
> > -						   int passphraselen)
> > -{
> > -	struct tpm_header *head;
> > -	int offset;
> > -
> > -	if (tpm2_chip_auth(chip)) {
> > -		tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
> > -	} else  {
> > -		offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > -		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);
> > -	}
> > -}
> > +			 u8 *passphrase, int passphraselen);
> > 
> > #ifdef CONFIG_TCG_TPM2_HMAC
> > 
> > -- 
> > 2.39.5
> > 
> > 
> 
> J.
> 
> -- 
> 101 things you can't have too much of : 19 - A Good Thing.
> This .sig brought to you by the letter V and the number 13
> Product of the Republic of HuggieTag

BR, Jarkko

[PATCH 3/4] tpm2-sessions: Remove unnecessary wrapper

[Jarkko Sakkinen]

From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>

Open code tpm_buf_append_hmac_session_opt() because it adds unnecessary
disperancy to the call sites (and reduces the amount of code).

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
 drivers/char/tpm/tpm2-cmd.c               | 14 +++++++++++---
 security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 61a4daaef292..0a795adbdc11 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -257,9 +257,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 
 	do {
 		tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
-		tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
-						| TPM2_SA_CONTINUE_SESSION,
-						NULL, 0);
+		if (tpm2_chip_auth(chip)) {
+			tpm_buf_append_hmac_session(chip, &buf,
+						    TPM2_SA_ENCRYPT |
+						    TPM2_SA_CONTINUE_SESSION,
+						    NULL, 0);
+		} else  {
+			offset = buf.handles * 4 + TPM_HEADER_SIZE;
+			head = (struct tpm_header *)buf.data;
+			if (tpm_buf_length(&buf) == offset)
+				head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+		}
 		tpm_buf_append_u16(&buf, num_bytes);
 		tpm_buf_fill_hmac_session(chip, &buf);
 		err = tpm_transmit_cmd(chip, &buf,
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index e165b117bbca..c414a7006d78 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -482,8 +482,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   struct trusted_key_options *options,
 			   u32 blob_handle)
 {
+	struct tpm_header *head;
 	struct tpm_buf buf;
 	u16 data_len;
+	int offset;
 	u8 *data;
 	int rc;
 
@@ -518,8 +520,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 		tpm2_buf_append_auth(&buf, options->policyhandle,
 				     NULL /* nonce */, 0, 0,
 				     options->blobauth, options->blobauth_len);
-		tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
-						NULL, 0);
+		if (tpm2_chip_auth(chip)) {
+			tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+		} else  {
+			offset = buf.handles * 4 + TPM_HEADER_SIZE;
+			head = (struct tpm_header *)buf.data;
+			if (tpm_buf_length(&buf) == offset)
+				head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+		}
 	}
 
 	tpm_buf_fill_hmac_session(chip, &buf);
-- 
2.39.5

Re: [PATCH 3/4] tpm2-sessions: Remove unnecessary wrapper

[Ben Boeckel]

On Mon, Sep 22, 2025 at 19:43:16 +0300, Jarkko Sakkinen wrote:
> From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> 
> Open code tpm_buf_append_hmac_session_opt() because it adds unnecessary
> disperancy to the call sites (and reduces the amount of code).
  ^^^^^^^^^^

"discrepancy" as in "difference"? But that doesn't feel like the right
usage either. Perhaps "unnecessary abstraction"? Also, open coding it
reduces the amount of code, so some clarification to not read as
something else that "it" (`tpm_buf_append_hmac_session_opt`) does would
be clearer.

Thanks,

--Ben

Re: [PATCH 3/4] tpm2-sessions: Remove unnecessary wrapper

[Jarkko Sakkinen]

On Mon, Sep 22, 2025 at 01:22:13PM -0400, Ben Boeckel wrote:
> On Mon, Sep 22, 2025 at 19:43:16 +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > 
> > Open code tpm_buf_append_hmac_session_opt() because it adds unnecessary
> > disperancy to the call sites (and reduces the amount of code).
>   ^^^^^^^^^^
> 
> "discrepancy" as in "difference"? But that doesn't feel like the right
> usage either. Perhaps "unnecessary abstraction"? Also, open coding it
> reduces the amount of code, so some clarification to not read as
> something else that "it" (`tpm_buf_append_hmac_session_opt`) does would
> be clearer.

Fair points. I'll re-edit the commit message and try to address the
issues you reported.

Intend of these changes is to essentially uncover the code paths so that
we know how to wrap it up better than it is wrapped up right now. Also,
they help to reveal possible regression paths. So while not functional 
per se, they do serve a purpose.

Once these fixes have been applied I'll start to look up the call
patterns and try to find a model where essentially we can transform
a TPM command to HMAC wrapped TPM command i.e., from tpm_buf to tpm_buf
operation where both sides of the function are TPM commands.

That way we can better selectively use the feature and it is easier
to fixup up e.g., a persistent parent key because key generation is
a huge bottleneck.

> 
> Thanks,
> 
> --Ben

[PATCH 4/4] keys, trusted: Remove redundant helper

[Jarkko Sakkinen]

From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>

tpm2_buf_append_auth has only single call site and most of its parameters
are redundant. Open code it to the call site. Remove illegit FIXME comment
as there is no categorized bug and replace it with more sane comment about
implementation (i.e. "non-opionated inline comment").

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
 security/keys/trusted-keys/trusted_tpm2.c | 51 ++++-------------------
 1 file changed, 9 insertions(+), 42 deletions(-)

diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index c414a7006d78..8e3b283a59b2 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -198,36 +198,6 @@ int tpm2_key_priv(void *context, size_t hdrlen,
 	return 0;
 }
 
-/**
- * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
- *
- * @buf: an allocated tpm_buf instance
- * @session_handle: session handle
- * @nonce: the session nonce, may be NULL if not used
- * @nonce_len: the session nonce length, may be 0 if not used
- * @attributes: the session attributes
- * @hmac: the session HMAC or password, may be NULL if not used
- * @hmac_len: the session HMAC or password length, maybe 0 if not used
- */
-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
-				 const u8 *nonce, u16 nonce_len,
-				 u8 attributes,
-				 const u8 *hmac, u16 hmac_len)
-{
-	tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
-	tpm_buf_append_u32(buf, session_handle);
-	tpm_buf_append_u16(buf, nonce_len);
-
-	if (nonce && nonce_len)
-		tpm_buf_append(buf, nonce, nonce_len);
-
-	tpm_buf_append_u8(buf, attributes);
-	tpm_buf_append_u16(buf, hmac_len);
-
-	if (hmac && hmac_len)
-		tpm_buf_append(buf, hmac, hmac_len);
-}
-
 /**
  * tpm2_seal_trusted() - seal the payload of a trusted key
  *
@@ -507,19 +477,16 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 					    options->blobauth_len);
 	} 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.
+		 * The policy session is generated outside the kernel, and thus
+		 * the password will end up being unencrypted on the bus, as
+		 * HMAC nonce cannot be calculated for it.
 		 */
-		tpm2_buf_append_auth(&buf, options->policyhandle,
-				     NULL /* nonce */, 0, 0,
-				     options->blobauth, options->blobauth_len);
+		tpm_buf_append_u32(&buf, 9 + options->blobauth_len);
+		tpm_buf_append_u32(&buf, options->policyhandle);
+		tpm_buf_append_u16(&buf, 0);
+		tpm_buf_append_u8(&buf, 0);
+		tpm_buf_append_u16(&buf, options->blobauth_len);
+		tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
 		if (tpm2_chip_auth(chip)) {
 			tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
 		} else  {
-- 
2.39.5

Re: [PATCH 4/4] keys, trusted: Remove redundant helper

[Jonathan McDowell]

On Mon, Sep 22, 2025 at 07:43:17PM +0300, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
>tpm2_buf_append_auth has only single call site and most of its parameters
>are redundant. Open code it to the call site. Remove illegit FIXME comment
>as there is no categorized bug and replace it with more sane comment about
>implementation (i.e. "non-opionated inline comment").
>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>

Seems like a reasonable cleanup.

Reviewed-by: Jonathan McDowell <noodles@earth.li>

>---
> security/keys/trusted-keys/trusted_tpm2.c | 51 ++++-------------------
> 1 file changed, 9 insertions(+), 42 deletions(-)
>
>diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>index c414a7006d78..8e3b283a59b2 100644
>--- a/security/keys/trusted-keys/trusted_tpm2.c
>+++ b/security/keys/trusted-keys/trusted_tpm2.c
>@@ -198,36 +198,6 @@ int tpm2_key_priv(void *context, size_t hdrlen,
> 	return 0;
> }
>
>-/**
>- * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
>- *
>- * @buf: an allocated tpm_buf instance
>- * @session_handle: session handle
>- * @nonce: the session nonce, may be NULL if not used
>- * @nonce_len: the session nonce length, may be 0 if not used
>- * @attributes: the session attributes
>- * @hmac: the session HMAC or password, may be NULL if not used
>- * @hmac_len: the session HMAC or password length, maybe 0 if not used
>- */
>-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
>-				 const u8 *nonce, u16 nonce_len,
>-				 u8 attributes,
>-				 const u8 *hmac, u16 hmac_len)
>-{
>-	tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
>-	tpm_buf_append_u32(buf, session_handle);
>-	tpm_buf_append_u16(buf, nonce_len);
>-
>-	if (nonce && nonce_len)
>-		tpm_buf_append(buf, nonce, nonce_len);
>-
>-	tpm_buf_append_u8(buf, attributes);
>-	tpm_buf_append_u16(buf, hmac_len);
>-
>-	if (hmac && hmac_len)
>-		tpm_buf_append(buf, hmac, hmac_len);
>-}
>-
> /**
>  * tpm2_seal_trusted() - seal the payload of a trusted key
>  *
>@@ -507,19 +477,16 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> 					    options->blobauth_len);
> 	} 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.
>+		 * The policy session is generated outside the kernel, and thus
>+		 * the password will end up being unencrypted on the bus, as
>+		 * HMAC nonce cannot be calculated for it.
> 		 */
>-		tpm2_buf_append_auth(&buf, options->policyhandle,
>-				     NULL /* nonce */, 0, 0,
>-				     options->blobauth, options->blobauth_len);
>+		tpm_buf_append_u32(&buf, 9 + options->blobauth_len);
>+		tpm_buf_append_u32(&buf, options->policyhandle);
>+		tpm_buf_append_u16(&buf, 0);
>+		tpm_buf_append_u8(&buf, 0);
>+		tpm_buf_append_u16(&buf, options->blobauth_len);
>+		tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
> 		if (tpm2_chip_auth(chip)) {
> 			tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
> 		} else  {
>-- 
>2.39.5
>
>

J.

-- 
If a program is useful, it must be changed.

Re: [PATCH 4/4] keys, trusted: Remove redundant helper

[Jarkko Sakkinen]

On Wed, Sep 24, 2025 at 09:29:23AM +0100, Jonathan McDowell wrote:
> On Mon, Sep 22, 2025 at 07:43:17PM +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > 
> > tpm2_buf_append_auth has only single call site and most of its parameters
> > are redundant. Open code it to the call site. Remove illegit FIXME comment
> > as there is no categorized bug and replace it with more sane comment about
> > implementation (i.e. "non-opionated inline comment").
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> 
> Seems like a reasonable cleanup.
> 
> Reviewed-by: Jonathan McDowell <noodles@earth.li>

Thank you!

BTW, I applied your patches to master:

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/

I did not yet bother to test them other than compilation and checkpatch
but I'm testing this and tpm_buf-series so they will get a lot of stress
testing. I'll rebase 'next' once they are actually tested (before PR).

BR, Jarkko