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

Jarkko Sakkinen posted 4 patches 1 week, 2 days ago
[PATCH 3/4] tpm2-sessions: Remove unnecessary wrapper
Posted by Jarkko Sakkinen 1 week, 2 days ago
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
Posted by Ben Boeckel 1 week, 2 days ago
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
Posted by Jarkko Sakkinen 1 week, 1 day ago
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