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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240716185225.873090-1-jarkko@kernel.org>
Date: Tue, 16 Jul 2024 21:52:24 +0300
From: Jarkko Sakkinen <jarkko@...nel.org>
To: linux-integrity@...r.kernel.org
Cc: Jarkko Sakkinen <jarkko@...nel.org>,
	stable@...r.kernel.org,
	James Bottomley <James.Bottomley@...senPartnership.com>,
	Mimi Zohar <zohar@...ux.ibm.com>,
	David Howells <dhowells@...hat.com>,
	Paul Moore <paul@...l-moore.com>,
	James Morris <jmorris@...ei.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	keyrings@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH v3] tpm: Relocate buf->handles to appropriate place

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@...r.kernel.org # v6.10+
Fixes: 699e3efd6c64 ("tpm: Add HMAC session start and end functions")
Signed-off-by: Jarkko Sakkinen <jarkko@...nel.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); */
 
 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