[PATCH] tpm2-sessions: Remove unnecessary wrapper

[PATCH] 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 +++++++++++---
 include/linux/tpm.h                       | 23 -----------------------
 security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
 3 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 7d77f6fbc152..89e6169add88 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/include/linux/tpm.h b/include/linux/tpm.h
index 667d290789ca..aaa407ddef21 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -534,29 +534,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
 				 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);
-	}
-}
 
 #ifdef CONFIG_TCG_TPM2_HMAC
 
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] tpm2-sessions: Remove unnecessary wrapper

[Jonathan McDowell]

On Mon, Sep 22, 2025 at 02:50:09PM +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).
>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>

I don't have a strong opinion on whether this is significantly better, 
but for 2 call sites it's not unreasonable, so:

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

>---
> drivers/char/tpm/tpm2-cmd.c               | 14 +++++++++++---
> include/linux/tpm.h                       | 23 -----------------------
> security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
> 3 files changed, 21 insertions(+), 28 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>index 7d77f6fbc152..89e6169add88 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/include/linux/tpm.h b/include/linux/tpm.h
>index 667d290789ca..aaa407ddef21 100644
>--- a/include/linux/tpm.h
>+++ b/include/linux/tpm.h
>@@ -534,29 +534,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
> 				 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);
>-	}
>-}
>
> #ifdef CONFIG_TCG_TPM2_HMAC
>
>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
>
>

J.

-- 
"Where else in computing can a random government that isn't even in
your country force you to make a change to your servers on three day's
notice?" -- Russ Allbery on DST

Re: [PATCH] tpm2-sessions: Remove unnecessary wrapper

[Jarkko Sakkinen]

On Wed, Sep 24, 2025 at 09:35:24AM +0100, Jonathan McDowell wrote:
> On Mon, Sep 22, 2025 at 02:50:09PM +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).
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> 
> I don't have a strong opinion on whether this is significantly better, but
> for 2 call sites it's not unreasonable, so:
> 
> Reviewed-By: Jonathan McDowell <noodles@earth.li>

I'll explain a bit of context.

I'm opening tpm2-session knot by knot and once it is like out and naked
my grand plan is to create a single function tpm2_authenticate or similar
function that wraps a TPM command into an authenticated TPM command.

The main priority with each patch is to do at least microscopic amount
of more than just pure clean up. The tpm_buf series and this series
sort of converge towards the same common goal. I think this is a good
way to move forward and refactor complex functionality without causing
major risks in production.

They key topic on this all maximizing the decoupling and it goes beyond
just "fixing tpm2-sessions". By doing this process we can more easily
re-use parts of the driver code in early boot code and sort of create
enablers for e.g., Trenchboot.

BR, Jarkko