All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: linux-integrity@vger.kernel.org
Cc: Jarkko Sakkinen <jarkko@kernel.org>,
	stable@vger.kernel.org,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	Mimi Zohar <zohar@linux.ibm.com>,
	David Howells <dhowells@redhat.com>,
	Paul Moore <paul@paul-moore.com>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v3] tpm: Relocate buf->handles to appropriate place
Date: Tue, 16 Jul 2024 21:52:24 +0300	[thread overview]
Message-ID: <20240716185225.873090-1-jarkko@kernel.org> (raw)

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); */
 
 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


             reply	other threads:[~2024-07-16 18:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-16 18:52 Jarkko Sakkinen [this message]
2024-07-16 18:54 ` [PATCH v3] tpm: Relocate buf->handles to appropriate place Jarkko Sakkinen
2024-07-16 19:32 ` James Bottomley
2024-07-17  9:27   ` Jarkko Sakkinen
2024-07-17  9:31     ` Jarkko Sakkinen
2024-07-17  9:34       ` Jarkko Sakkinen
2024-07-17  9:55 ` Jonathan McDowell
2024-07-17 11:23   ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240716185225.873090-1-jarkko@kernel.org \
    --to=jarkko@kernel.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=dhowells@redhat.com \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=serge@hallyn.com \
    --cc=stable@vger.kernel.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.