Re: [PATCH v3] tpm: Relocate buf->handles to appropriate place

On Tue, Jul 16, 2024 at 09:52:24PM +0300, Jarkko Sakkinen wrote:

tpm_buf_append_name() has the following snippet in the beginning:

if (!tpm2_chip_auth(chip)) { tpm_buf_append_u32(buf, handle); /* count the number of handles in the upper bits of flags */ buf->handles++; return; }

The claim in the comment is wrong, and the comment is in the wrong place as alignment in this case should not anyway be a concern of the call site. In essence the comment is lying about the code, and thus needs to be adressed.

Further, 'handles' was incorrectly place to struct tpm_buf, as tpm-buf.c does manage its state. It is easy to grep that only piece of code that actually uses the field is tpm2-sessions.c.

Address the issues by moving the variable to struct tpm_chip.

Cc: stable@vger.kernel.org # v6.10+ Fixes: 699e3efd6c64 ("tpm: Add HMAC session start and end functions") Signed-off-by: Jarkko Sakkinen jarkko@kernel.org

v3:

• Reset chip->handles in the beginning of tpm2_start_auth_session() so that it shows correct value, when TCG_TPM2_HMAC is enabled but tpm2_sessions_init() has never been called.

v2:

• Was a bit more broken than I first thought, as 'handles' is only useful for tpm2-sessions.c and has zero relation to tpm-buf.c.

---

drivers/char/tpm/tpm-buf.c | 1 - drivers/char/tpm/tpm2-cmd.c | 2 +- drivers/char/tpm/tpm2-sessions.c | 7 ++++--- include/linux/tpm.h | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c index cad0048bcc3c..d06e8e063151 100644 --- a/drivers/char/tpm/tpm-buf.c +++ b/drivers/char/tpm/tpm-buf.c @@ -44,7 +44,6 @@ void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) head->tag = cpu_to_be16(tag); head->length = cpu_to_be32(sizeof(*head)); head->ordinal = cpu_to_be32(ordinal);

• buf->handles = 0;

} EXPORT_SYMBOL_GPL(tpm_buf_reset); diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index 1e856259219e..b781e4406fc2 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -776,7 +776,7 @@ int tpm2_auto_startup(struct tpm_chip *chip) if (rc) goto out;

• rc = tpm2_sessions_init(chip);

• /* rc = tpm2_sessions_init(chip); */

Left over from testing? Or should be removed entirely?

out: /* diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c index d3521aadd43e..5e7c12d64ba8 100644 --- a/drivers/char/tpm/tpm2-sessions.c +++ b/drivers/char/tpm/tpm2-sessions.c @@ -238,8 +238,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf, if (!tpm2_chip_auth(chip)) { tpm_buf_append_u32(buf, handle);

• /* count the number of handles in the upper bits of flags */
  

• buf->handles++;
  

• chip->handles++;
  

  return; }

@@ -310,7 +309,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, if (!tpm2_chip_auth(chip)) { /* offset tells us where the sessions area begins */

• int offset = buf->handles * 4 + TPM_HEADER_SIZE;
  

• int offset = chip->handles * 4 + TPM_HEADER_SIZE;
  

  u32 len = 9 + passphrase_len;

if (tpm_buf_length(buf) != offset) { @@ -963,6 +962,8 @@ int tpm2_start_auth_session(struct tpm_chip *chip) int rc; u32 null_key;

• chip->handles = 0;
• if (!auth) { dev_warn_once(&chip->dev, "auth session is not active\n"); return 0;

diff --git a/include/linux/tpm.h b/include/linux/tpm.h index e93ee8d936a9..b664f7556494 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -202,9 +202,9 @@ struct tpm_chip { /* active locality */ int locality;

• /* handle count for session: */
• u8 handles;

#ifdef CONFIG_TCG_TPM2_HMAC

• /* details for communication security via sessions */
• /* saved context for NULL seed */ u8 null_key_context[TPM2_MAX_CONTEXT_SIZE]; /* name of NULL seed */

@@ -377,7 +377,6 @@ struct tpm_buf { u32 flags; u32 length; u8 *data;

• u8 handles;

}; enum tpm2_object_attributes { @@ -517,7 +516,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip, if (tpm2_chip_auth(chip)) { tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen); } else {

• offset = buf->handles * 4 + TPM_HEADER_SIZE;
  

• offset = chip->handles * 4 + TPM_HEADER_SIZE;
  

  head = (struct tpm_header *)buf->data;

/* @@ -541,6 +540,7 @@ void tpm2_end_auth_session(struct tpm_chip *chip); static inline int tpm2_start_auth_session(struct tpm_chip *chip) {

• chip->handles = 0; return 0;

} static inline void tpm2_end_auth_session(struct tpm_chip *chip) -- 2.45.2

J.