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

Jonathan McDowell
2025-09-24T08:29:37Z

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.