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

[Jonathan McDowell <noodles@earth.li>]

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.