* [PATCH v3 3/4] tpm2-sessions: Remove 'attributes' parameter from tpm_buf_append_auth
From: Jarkko Sakkinen @ 2025-12-03 22:12 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, Jarkko Sakkinen, Jonathan McDowell, Roberto Sassu,
Mimi Zohar
In-Reply-To: <20251203221215.536031-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Remove 'attributes' parameter from 'tpm_buf_append_auth', as it is not used
by the function.
Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
---
drivers/char/tpm/tpm2-cmd.c | 2 +-
drivers/char/tpm/tpm2-sessions.c | 5 ++---
include/linux/tpm.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 34e3599f094f..ce0a1c6b0596 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -210,7 +210,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
} else {
tpm_buf_append_handle(chip, &buf, pcr_idx);
- tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
+ tpm_buf_append_auth(chip, &buf, NULL, 0);
}
tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index e9f439be3916..f0eb8a7d8f72 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -311,7 +311,7 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
- u8 attributes, u8 *passphrase, int passphrase_len)
+ u8 *passphrase, int passphrase_len)
{
/* offset tells us where the sessions area begins */
int offset = buf->handles * 4 + TPM_HEADER_SIZE;
@@ -372,8 +372,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
#endif
if (!tpm2_chip_auth(chip)) {
- tpm_buf_append_auth(chip, buf, attributes, passphrase,
- passphrase_len);
+ tpm_buf_append_auth(chip, buf, passphrase, passphrase_len);
return;
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index aa816b144ab3..afa51723296a 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -535,7 +535,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
- u8 attributes, u8 *passphrase, int passphraselen);
+ u8 *passphrase, int passphraselen);
static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
struct tpm_buf *buf,
u8 attributes,
--
2.52.0
^ permalink raw reply related
* [PATCH v3 2/4] tpm2-sessions: Fix tpm2_read_public range checks
From: Jarkko Sakkinen @ 2025-12-03 22:12 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, stable, James Bottomley, Ard Biesheuvel
In-Reply-To: <20251203221215.536031-1-jarkko@kernel.org>
'tpm2_read_public' has some rudimentary range checks but the function
does not ensure that the response buffer has enough bytes for the full
TPMT_HA payload.
Re-implement the function with necessary checks and validation.
Cc: stable@vger.kernel.org # v6.10+
Fixes: d0a25bb961e6 ("tpm: Add HMAC session name/handle append")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
v2:
- Made the fix localized instead of spread all over the place.
---
drivers/char/tpm/tpm2-cmd.c | 3 ++
drivers/char/tpm/tpm2-sessions.c | 77 +++++++++++++++++---------------
2 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index be4a9c7f2e1a..34e3599f094f 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -11,8 +11,11 @@
* used by the kernel internally.
*/
+#include "linux/dev_printk.h"
+#include "linux/tpm.h"
#include "tpm.h"
#include <crypto/hash_info.h>
+#include <linux/unaligned.h>
static bool disable_pcr_integrity;
module_param(disable_pcr_integrity, bool, 0444);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index a265e9752a5e..e9f439be3916 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -163,54 +163,59 @@ static int name_size(const u8 *name)
}
}
-static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
+static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
{
- struct tpm_header *head = (struct tpm_header *)buf->data;
+ u32 mso = tpm2_handle_mso(handle);
off_t offset = TPM_HEADER_SIZE;
- u32 tot_len = be32_to_cpu(head->length);
- int ret;
- u32 val;
-
- /* we're starting after the header so adjust the length */
- tot_len -= TPM_HEADER_SIZE;
-
- /* skip public */
- val = tpm_buf_read_u16(buf, &offset);
- if (val > tot_len)
- return -EINVAL;
- offset += val;
- /* name */
-
- val = tpm_buf_read_u16(buf, &offset);
- ret = name_size(&buf->data[offset]);
- if (ret < 0)
- return ret;
+ struct tpm_buf buf;
+ int rc, rc2;
- if (val != ret)
+ if (mso != TPM2_MSO_PERSISTENT && mso != TPM2_MSO_VOLATILE &&
+ mso != TPM2_MSO_NVRAM)
return -EINVAL;
- memcpy(name, &buf->data[offset], val);
- /* forget the rest */
- return 0;
-}
-
-static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
-{
- struct tpm_buf buf;
- int rc;
-
rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
if (rc)
return rc;
tpm_buf_append_u32(&buf, handle);
- rc = tpm_transmit_cmd(chip, &buf, 0, "read public");
- if (rc == TPM2_RC_SUCCESS)
- rc = tpm2_parse_read_public(name, &buf);
- tpm_buf_destroy(&buf);
+ rc = tpm_transmit_cmd(chip, &buf, 0, "TPM2_ReadPublic");
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return tpm_ret_to_err(rc);
+ }
- return rc;
+ /* Skip TPMT_PUBLIC: */
+ offset += tpm_buf_read_u16(&buf, &offset);
+
+ /*
+ * Ensure space for the length field of TPM2B_NAME and hashAlg field of
+ * TPMT_HA (the extra four bytes).
+ */
+ if (offset + 4 > tpm_buf_length(&buf)) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ rc = tpm_buf_read_u16(&buf, &offset);
+ rc2 = name_size(&buf.data[offset]);
+
+ if (rc2 < 0)
+ return rc2;
+
+ if (rc != rc2) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ if (offset + rc > tpm_buf_length(&buf)) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ memcpy(name, &buf.data[offset], rc);
+ return 0;
}
#endif /* CONFIG_TCG_TPM2_HMAC */
--
2.52.0
^ permalink raw reply related
* [PATCH v3 1/4] tpm2-sessions: fix out of range indexing in name_size
From: Jarkko Sakkinen @ 2025-12-03 22:12 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, stable, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ard Biesheuvel,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251203221215.536031-1-jarkko@kernel.org>
'name_size' does not have any range checks, and it just directly indexes
with TPM_ALG_ID, which could lead into memory corruption at worst.
Address the issue by only processing known values and returning -EINVAL for
unrecognized values.
Make also 'tpm_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible so
that errors are detected before causing any spurious TPM traffic.
End also the authorization session on failure in both of the functions, as
the session state would be then by definition corrupted.
Cc: stable@vger.kernel.org # v6.10+
Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
v3:
- Fix pr_warn() format string in name_size().
- Fix !CONFIG_TPM2_HMAC compilation.
v2:
- Wrote a better short summary.
- Addressed remarks in https://lore.kernel.org/linux-integrity/aS8TIeviaippVAha@earth.li/
- Use -EIO consistently in tpm2_fill_hmac_session. These are not input value
errors. They could only spun from malformed (kernel) state.
- name_size did not have a proper default-case. Reorganize the
fallback into that.
---
drivers/char/tpm/tpm2-cmd.c | 23 +++-
drivers/char/tpm/tpm2-sessions.c | 133 +++++++++++++++-------
include/linux/tpm.h | 13 ++-
security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
4 files changed, 143 insertions(+), 55 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index dd502322f499..be4a9c7f2e1a 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -199,7 +199,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
}
if (!disable_pcr_integrity) {
- tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+ rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return rc;
+ }
tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
} else {
tpm_buf_append_handle(chip, &buf, pcr_idx);
@@ -214,8 +218,14 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
chip->allocated_banks[i].digest_size);
}
- if (!disable_pcr_integrity)
- tpm_buf_fill_hmac_session(chip, &buf);
+ if (!disable_pcr_integrity) {
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return rc;
+ }
+ }
+
rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
if (!disable_pcr_integrity)
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
@@ -273,7 +283,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
| TPM2_SA_CONTINUE_SESSION,
NULL, 0);
tpm_buf_append_u16(&buf, num_bytes);
- tpm_buf_fill_hmac_session(chip, &buf);
+ err = tpm_buf_fill_hmac_session(chip, &buf);
+ if (err) {
+ tpm_buf_destroy(&buf);
+ return err;
+ }
+
err = tpm_transmit_cmd(chip, &buf,
offsetof(struct tpm2_get_random_out,
buffer),
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 6d03c224e6b2..a265e9752a5e 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -144,16 +144,23 @@ struct tpm2_auth {
/*
* Name Size based on TPM algorithm (assumes no hash bigger than 255)
*/
-static u8 name_size(const u8 *name)
+static int name_size(const u8 *name)
{
- static u8 size_map[] = {
- [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
- [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
- [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
- [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
- };
- u16 alg = get_unaligned_be16(name);
- return size_map[alg] + 2;
+ u16 hash_alg = get_unaligned_be16(name);
+
+ switch (hash_alg) {
+ case TPM_ALG_SHA1:
+ return SHA1_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA256:
+ return SHA256_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA384:
+ return SHA384_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA512:
+ return SHA512_DIGEST_SIZE + 2;
+ default:
+ pr_warn("tpm: unsupported name algorithm: 0x%04x\n", hash_alg);
+ return -EINVAL;
+ }
}
static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
@@ -161,6 +168,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
struct tpm_header *head = (struct tpm_header *)buf->data;
off_t offset = TPM_HEADER_SIZE;
u32 tot_len = be32_to_cpu(head->length);
+ int ret;
u32 val;
/* we're starting after the header so adjust the length */
@@ -172,9 +180,15 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
return -EINVAL;
offset += val;
/* name */
+
val = tpm_buf_read_u16(buf, &offset);
- if (val != name_size(&buf->data[offset]))
+ ret = name_size(&buf->data[offset]);
+ if (ret < 0)
+ return ret;
+
+ if (val != ret)
return -EINVAL;
+
memcpy(name, &buf->data[offset], val);
/* forget the rest */
return 0;
@@ -221,46 +235,72 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
* As with most tpm_buf operations, success is assumed because failure
* will be caused by an incorrect programming model and indicated by a
* kernel message.
+ *
+ * Ends the authorization session on failure.
*/
-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name)
+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
+ u32 handle, u8 *name)
{
#ifdef CONFIG_TCG_TPM2_HMAC
enum tpm2_mso_type mso = tpm2_handle_mso(handle);
struct tpm2_auth *auth;
int slot;
+ int ret;
#endif
if (!tpm2_chip_auth(chip)) {
tpm_buf_append_handle(chip, buf, handle);
- return;
+ return 0;
}
#ifdef CONFIG_TCG_TPM2_HMAC
slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
if (slot >= AUTH_MAX_NAMES) {
- dev_err(&chip->dev, "TPM: too many handles\n");
- return;
+ dev_err(&chip->dev, "too many handles\n");
+ ret = -EIO;
+ goto err;
}
auth = chip->auth;
- WARN(auth->session != tpm_buf_length(buf),
- "name added in wrong place\n");
+ if (auth->session != tpm_buf_length(buf)) {
+ dev_err(&chip->dev, "session state malformed");
+ ret = -EIO;
+ goto err;
+ }
tpm_buf_append_u32(buf, handle);
auth->session += 4;
if (mso == TPM2_MSO_PERSISTENT ||
mso == TPM2_MSO_VOLATILE ||
mso == TPM2_MSO_NVRAM) {
- if (!name)
- tpm2_read_public(chip, handle, auth->name[slot]);
+ if (!name) {
+ ret = tpm2_read_public(chip, handle, auth->name[slot]);
+ if (ret)
+ goto err;
+ }
} else {
- if (name)
- dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
+ if (name) {
+ dev_err(&chip->dev, "handle 0x%08x does not use a name\n",
+ handle);
+ ret = -EIO;
+ goto err;
+ }
}
auth->name_h[slot] = handle;
- if (name)
- memcpy(auth->name[slot], name, name_size(name));
+ if (name) {
+ ret = name_size(name);
+ if (ret < 0)
+ goto err;
+
+ memcpy(auth->name[slot], name, ret);
+ }
+#endif
+ return 0;
+
+#ifdef CONFIG_TCG_TPM2_HMAC
+err:
+ tpm2_end_auth_session(chip);
+ return tpm_ret_to_err(ret);
#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -533,11 +573,9 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
* encryption key and encrypts the first parameter of the command
* buffer with it.
*
- * As with most tpm_buf operations, success is assumed because failure
- * will be caused by an incorrect programming model and indicated by a
- * kernel message.
+ * Ends the authorization session on failure.
*/
-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
{
u32 cc, handles, val;
struct tpm2_auth *auth = chip->auth;
@@ -549,9 +587,12 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
u8 cphash[SHA256_DIGEST_SIZE];
struct sha256_ctx sctx;
struct hmac_sha256_ctx hctx;
+ int ret;
- if (!auth)
- return;
+ if (!auth) {
+ ret = -EIO;
+ goto err;
+ }
/* save the command code in BE format */
auth->ordinal = head->ordinal;
@@ -560,9 +601,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
i = tpm2_find_cc(chip, cc);
if (i < 0) {
- dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
- return;
+ dev_err(&chip->dev, "command 0x%08x not found\n", cc);
+ ret = -EIO;
+ goto err;
}
+
attrs = chip->cc_attrs_tbl[i];
handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
@@ -576,9 +619,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
u32 handle = tpm_buf_read_u32(buf, &offset_s);
if (auth->name_h[i] != handle) {
- dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
- i);
- return;
+ dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
+ ret = -EIO;
+ goto err;
}
}
/* point offset_s to the start of the sessions */
@@ -609,12 +652,14 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
offset_s += len;
}
if (offset_s != offset_p) {
- dev_err(&chip->dev, "TPM session length is incorrect\n");
- return;
+ dev_err(&chip->dev, "session length is incorrect\n");
+ ret = -EIO;
+ goto err;
}
if (!hmac) {
- dev_err(&chip->dev, "TPM could not find HMAC session\n");
- return;
+ dev_err(&chip->dev, "could not find HMAC session\n");
+ ret = -EIO;
+ goto err;
}
/* encrypt before HMAC */
@@ -646,8 +691,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
if (mso == TPM2_MSO_PERSISTENT ||
mso == TPM2_MSO_VOLATILE ||
mso == TPM2_MSO_NVRAM) {
- sha256_update(&sctx, auth->name[i],
- name_size(auth->name[i]));
+ ret = name_size(auth->name[i]);
+ if (ret < 0)
+ goto err;
+
+ sha256_update(&sctx, auth->name[i], ret);
} else {
__be32 h = cpu_to_be32(auth->name_h[i]);
@@ -668,6 +716,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
hmac_sha256_update(&hctx, &auth->attrs, 1);
hmac_sha256_final(&hctx, hmac);
+ return 0;
+
+err:
+ tpm2_end_auth_session(chip);
+ return ret;
}
EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 3d8f7d1ce2b8..aa816b144ab3 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -529,8 +529,8 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
#endif
}
-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name);
+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
+ u32 handle, u8 *name);
void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
@@ -563,7 +563,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
#ifdef CONFIG_TCG_TPM2_HMAC
int tpm2_start_auth_session(struct tpm_chip *chip);
-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
int rc);
void tpm2_end_auth_session(struct tpm_chip *chip);
@@ -577,10 +577,13 @@ static inline int tpm2_start_auth_session(struct tpm_chip *chip)
static inline void tpm2_end_auth_session(struct tpm_chip *chip)
{
}
-static inline void tpm_buf_fill_hmac_session(struct tpm_chip *chip,
- struct tpm_buf *buf)
+
+static inline int tpm_buf_fill_hmac_session(struct tpm_chip *chip,
+ struct tpm_buf *buf)
{
+ return 0;
}
+
static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
struct tpm_buf *buf,
int rc)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 8bc6efa8accb..5b205279584b 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -268,7 +268,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out_put;
}
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
+
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
options->keyauth, TPM_DIGEST_SIZE);
@@ -316,7 +319,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (rc)
@@ -427,7 +433,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
return rc;
}
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
+
tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
TPM_DIGEST_SIZE);
@@ -439,7 +448,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
goto out;
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (!rc)
@@ -484,7 +496,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
return rc;
}
- tpm_buf_append_name(chip, &buf, blob_handle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
if (!options->policyhandle) {
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
@@ -509,7 +523,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
NULL, 0);
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
--
2.52.0
^ permalink raw reply related
* [PATCH v3 0/4] tpm2-sessions: Fixes for v6.19-rc2
From: Jarkko Sakkinen @ 2025-12-03 22:12 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list
I collected the accumulated fixed for tpm2-sessions, geared towards
v6.19-rc2.
v3:
- Fixed two issues in 1/4.
v2:
- Addressed issues reported by Jonathan McDowell:
https://lore.kernel.org/linux-integrity/aS8TIeviaippVAha@earth.li/
Cc: Jonathan McDowell <noodles@earth.li>
Jarkko Sakkinen (4):
tpm2-sessions: fix out of range indexing in name_size
tpm2-sessions: Fix tpm2_read_public range checks
tpm2-sessions: Remove 'attributes' parameter from tpm_buf_append_auth
tpm2-sessions: Open code tpm_buf_append_hmac_session()
drivers/char/tpm/tpm2-cmd.c | 42 ++++-
drivers/char/tpm/tpm2-sessions.c | 199 ++++++++++++++--------
include/linux/tpm.h | 38 +----
security/keys/trusted-keys/trusted_tpm2.c | 41 ++++-
4 files changed, 204 insertions(+), 116 deletions(-)
--
2.52.0
^ permalink raw reply
* [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-6.19-rc1-v4
From: Jarkko Sakkinen @ 2025-12-03 21:18 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Huewe, Jason Gunthorpe, David Howells, keyrings,
linux-integrity, linux-kernel
The following changes since commit a619fe35ab41fded440d3762d4fbad84ff86a4d4:
Merge tag 'v6.19-p1' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6 (2025-12-03 11:28:38 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/tpmdd-next-6.19-rc1-v4
for you to fetch changes up to 09b71a58ee355ae09f302bd0875a29ffbcbbe3ac:
KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 (2025-12-03 22:55:28 +0200)
----------------------------------------------------------------
Hi,
This pull request for TPM driver contains changes to unify TPM return
code translation between trusted_tpm2 and TPM driver itself. Other than
that the changes are either bug fixes or minor imrovements.
Change log that should explain the previous iterations:
1. "Documentation: tpm-security.rst: change title to section"
https://lore.kernel.org/all/86514a6ab364e01f163470a91cacef120e1b8b47.camel@HansenPartnership.com/
2. "drivers/char/tpm: use min() instead of min_t()"
https://lore.kernel.org/all/20251201161228.3c09d88a@pumpkin/
3. Removed spurious kfree(): https://lore.kernel.org/linux-integrity/aS+K5nO2MP7N+kxQ@ly-workstation/
BR, Jarkko
----------------------------------------------------------------
Chu Guangqing (1):
tpm_crb: Fix a spelling mistake
Jarkko Sakkinen (3):
tpm: Cap the number of PCR banks
tpm: Use -EPERM as fallback error code in tpm_ret_to_err
KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2
Jonathan McDowell (1):
tpm: Remove tpm_find_get_ops
Marco Crivellari (1):
tpm: add WQ_PERCPU to alloc_workqueue users
Maurice Hieronymus (1):
selftests: tpm2: Fix ill defined assertions
Stuart Yoder (1):
tpm_crb: add missing loc parameter to kerneldoc
drivers/char/tpm/tpm-chip.c | 37 -------------------------------
drivers/char/tpm/tpm-dev-common.c | 3 ++-
drivers/char/tpm/tpm-interface.c | 20 +++++++++++++----
drivers/char/tpm/tpm.h | 1 -
drivers/char/tpm/tpm1-cmd.c | 5 -----
drivers/char/tpm/tpm2-cmd.c | 8 +++----
drivers/char/tpm/tpm_crb.c | 4 +++-
drivers/char/tpm/tpm_tis_core.c | 3 +--
include/linux/tpm.h | 12 ++++++----
security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++----------------
tools/testing/selftests/tpm2/tpm2.py | 4 ++--
11 files changed, 42 insertions(+), 81 deletions(-)
^ permalink raw reply
* Re: [GIT PULL] integrity: subsystem updates for v6.19
From: pr-tracker-bot @ 2025-12-03 19:32 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Linus Torvalds, linux-integrity, linux-kernel, Roberto Sassu
In-Reply-To: <58c716aa7e18d107590f98705c29e5a0434cbcbf.camel@linux.ibm.com>
The pull request you sent on Tue, 02 Dec 2025 16:55:12 -0500:
> https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/ tags/integrity-v6.19
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/777f8171602d5954cac024b66afa1b5b030641a4
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [GIT PULL] KEYS: trusted: keys-trusted-next-rc1
From: pr-tracker-bot @ 2025-12-03 19:32 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Linus Torvalds, David Howells, James Bottomley, Mimi Zohar,
keyrings, linux-integrity, linux-security-module
In-Reply-To: <aSthHCovbsDZANsa@kernel.org>
The pull request you sent on Sat, 29 Nov 2025 23:09:48 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/keys-trusted-next-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7fc2cd2e4b398c57c9cf961cfea05eadbf34c05c
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [GIT PULL] KEYS: keys-next-6.19-rc1
From: pr-tracker-bot @ 2025-12-03 19:32 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Linus Torvalds, David Howells, Herbert Xu, David S. Miller,
keyrings, linux-integrity, Thorsten Blum
In-Reply-To: <aSjSZj47-p_3i_Xn@kernel.org>
The pull request you sent on Fri, 28 Nov 2025 00:36:22 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/keys-next-6.19-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b082c4b06056139b2f3e0a8a0fa3235e145fba80
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: Are setuid shell scripts safe? (Implied by security_bprm_creds_for_exec)
From: Bernd Edlinger @ 2025-12-03 13:16 UTC (permalink / raw)
To: Eric W. Biederman, Roberto Sassu
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet, zohar, linux-integrity, Ryan Lee, apparmor
In-Reply-To: <87ms42rq3t.fsf@email.froward.int.ebiederm.org>
On 12/1/25 19:53, Eric W. Biederman wrote:
> Roberto Sassu <roberto.sassu@huaweicloud.com> writes:
>
>> On Mon, 2025-12-01 at 10:06 -0600, Eric W. Biederman wrote:
>>> Roberto Sassu <roberto.sassu@huaweicloud.com> writes:
>>>
>>>> + Mimi, linux-integrity (would be nice if we are in CC when linux-
>>>> security-module is in CC).
>>>>
>>>> Apologies for not answering earlier, it seems I don't receive the
>>>> emails from the linux-security-module mailing list (thanks Serge for
>>>> letting me know!).
>>>>
>>>> I see two main effects of this patch. First, the bprm_check_security
>>>> hook implementations will not see bprm->cred populated. That was a
>>>> problem before we made this patch:
>>>>
>>>> https://patchew.org/linux/20251008113503.2433343-1-roberto.sassu@huaweicloud.com/
>>>
>>> Thanks, that is definitely needed.
>>>
>>> Does calling process_measurement(CREDS_CHECK) on only the final file
>>> pass review? Do you know of any cases where that will break things?
>>
>> We intentionally changed the behavior of CREDS_CHECK to be invoked only
>> for the final file. We are monitoring for bug reports, if we receive
>> complains from people that the patch breaks their expectation we will
>> revisit the issue.
>>
>> Any LSM implementing bprm_check_security looking for brpm->cred would
>> be affected by recalculating the DAC credentials for the final binary.
>>
>>> As it stands I don't think it should be assumed that any LSM has
>>> computed it's final creds until bprm_creds_from_file. Not just the
>>> uid and gid.
>>
>> Uhm, I can be wrong, but most LSMs calculate their state change in
>> bprm_creds_for_exec (git grep bprm_creds_for_exec|grep LSM_HOOK_INIT).
>>
>>> If the patch you posted for review works that helps sort that mess out.
>>
>> Well, it works because we changed the expectation :)
>
> I just haven't seen that code land in Linus's tree yet so I am a bit
> cautious in adopting that. It is definitely needed as the behavior
> of IMA as v6.18 simply does not work in general.
>
>>>> to work around the problem of not calculating the final DAC credentials
>>>> early enough (well, we actually had to change our CREDS_CHECK hook
>>>> behavior).
>>>>
>>>> The second, I could not check. If I remember well, unlike the
>>>> capability LSM, SELinux/Apparmor/SMACK calculate the final credentials
>>>> based on the first file being executed (thus the script, not the
>>>> interpreter). Is this patch keeping the same behavior despite preparing
>>>> the credentials when the final binary is found?
>>>
>>> The patch I posted was.
>>>
>>> My brain is still reeling from the realization that our security modules
>>> have the implicit assumption that it is safe to calculate their security
>>> information from shell scripts.
>>
>> If I'm interpreting this behavior correctly (please any LSM maintainer
>> could comment on it), the intent is just to transition to a different
>> security context where a different set of rules could apply (since we
>> are executing a script).
>>
>> Imagine if for every script, the security transition is based on the
>> interpreter, it would be hard to differentiate between scripts and
>> associate to the respective processes different security labels.
>>
>>> In the first half of the 90's I remember there was lots of effort to try
>>> and make setuid shell scripts and setuid perl scripts work, and the
>>> final conclusion was it was a lost cause.
>>
>> Definitely I lack a lot of context...
>
> From the usenet comp.unix.faq that was probably updated in 1994:
> http://www.faqs.org/faqs/unix-faq/faq/part4/section-7.html
>
> I have been trying to remember enough details by looking it up, but the
> short version is that one of the big problems is there is a race between
> the kernel doing it's thing and the shell opening the shell script.
>
> Clever people have been able to take advantage of that race and insert
> arbitrary code in that window for the shell to execute. All you have to
> do is google for how to find a reproducer if the one in the link above
> is not enough.
>
>>> Now I look at security_bprm_creds_for_exec and security_bprm_check which
>>> both have the implicit assumption that it is indeed safe to compute the
>>> credentials from a shell script.
>>>
>>> When passing a file descriptor to execat we have
>>> BINPRM_FLAGS_PATH_INACCESSIBLE and use /dev/fd/NNN as the filename
>>> which reduces some of the races.
>>>
>>> However when just plain executing a shell script we pass the filename of
>>> the shell script as a command line argument, and expect the shell to
>>> open the filename again. This has been a time of check to time of use
>>> race for decades, and one of the reasons we don't have setuid shell
>>> scripts.
>>
>> Yes, it would be really nice to fix it!
>
> After 30 years I really don't expect that is even a reasonable request.
>
> I think we are solidly into "Don't do that then", and the LSM security
> hooks are definitely doing that.
>
> There is the partial solution of passing /dev/fd instead of passing the
> name of the script. I suspect that would break things. I don't
> remember why that was never adopted.
>
> I think even with the TOCTOU race fixed there were other serious issues.
>
> I really think it behooves any security module people who want to use
> the shell script as the basis of their security decisions to research
> all of the old well known issues and describe how they don't apply.
>
> All I have energy for is to point out it is broken as is and to start
> moving code down into bprm_creds_from_file to avoid the race.
>
> Right now as far as I can tell anything based upon the script itself
> is worthless junk so changing that would not be breaking anything that
> wasn't already broken.
>
>>> Yet the IMA implementation (without the above mentioned patch) assumes
>>> the final creds will be calculated before security_bprm_check is called,
>>> and security_bprm_creds_for_exec busily calculate the final creds.
>>>
>>> For some of the security modules I believe anyone can set any label they
>>> want on a file and they remain secure (At which point I don't understand
>>> the point of having labels on files). I don't believe that is the case
>>> for selinux, or in general.
>>
>> A simple example for SELinux. Suppose that the parent process has type
>> initrc_t, then the SELinux policy configures the following transitions
>> based on the label of the first file executed (sesearch -T -s initrc_t
>> -c process):
>>
>> type_transition initrc_t NetworkManager_dispatcher_exec_t:process NetworkManager_dispatcher_t;
>> type_transition initrc_t NetworkManager_exec_t:process NetworkManager_t;
>> type_transition initrc_t NetworkManager_initrc_exec_t:process initrc_t;
>> type_transition initrc_t NetworkManager_priv_helper_exec_t:process NetworkManager_priv_helper_t;
>> type_transition initrc_t abrt_dump_oops_exec_t:process abrt_dump_oops_t;
>> type_transition initrc_t abrt_exec_t:process abrt_t;
>> [...]
>>
>> (there are 747 rules in my system).
>>
>> If the transition would be based on the interpreter label, it would be
>> hard to express with rules.
>
> Which is a problem for the people making the rules engine. Because
> 30 years of experience with this problem says basing anything on the
> script is already broken.
>
> I understand the frustration, but it requires a new way of launching
> shell scripts to even begin to make it secure.
>
>> If the transition does not occur for any reason the parent process
>> policy would still apply, but maybe it would not have the necessary
>> permissions for the execution of the script.
>
> Yep.
>
>>> So just to remove the TOCTOU race the security_bprm_creds_for_exec
>>> and security_bprm_check hooks need to be removed, after moving their
>>> code into something like security_bprm_creds_from_file.
>>>
>>> Or am I missing something and even with the TOCTOU race are setuid shell
>>> scripts somehow safe now?
>>
>> Take this with a looot of salt, if there is a TOCTOU race, the script
>> will be executed with a security context that does not belong to it.
>> But the transition already happened. Not sure if it is safe.
>
> Historically it hasn't been safe.
>
>> I also don't know how the TOCTOU race could be solved, but I also would
>> like it to be fixed. I'm available to comment on any proposal!
>
> I am hoping someone who helped put these security hooks where they are
> will speak up, and tell me what I am missing.
>
> All I have the energy for right now is to point out security policies
> based upon shell scripts appear to be security policies that only
> protect you from well behaved programs.
>
Hmm, yes, that looks like an issue.
I would have expected the security engine to look at bprm->filenanme
especially in the case, when bprm->interp != bprm->filename,
and check that it is not a sym-link with write-access for the
current user and of course also that the bprm->file is not a regular file
which is writable by the current user, if that is the case I would have expected
the secuity engine to enforce non-new-privs on a SUID executable somehow.
Bernd.
^ permalink raw reply
* Re: [PATCHv2 1/2] kernel/kexec: Change the prototype of kimage_map_segment()
From: Baoquan He @ 2025-12-03 8:06 UTC (permalink / raw)
To: Pingfan Liu
Cc: kexec, linux-integrity, Andrew Morton, Mimi Zohar, Roberto Sassu,
Alexander Graf, Steven Chen, stable
In-Reply-To: <CAF+s44S2_DG92dJAGX8GZdc-OgOz1a7E+ScbyOGcG85QayBS1w@mail.gmail.com>
On 12/03/25 at 12:22pm, Pingfan Liu wrote:
> Hi Baoquan,
>
> On Wed, Nov 26, 2025 at 9:10 AM Baoquan He <bhe@redhat.com> wrote:
> >
> > Hi Pingfan,
> >
> > On 11/06/25 at 02:59pm, Pingfan Liu wrote:
> > > The kexec segment index will be required to extract the corresponding
> > > information for that segment in kimage_map_segment(). Additionally,
> > > kexec_segment already holds the kexec relocation destination address and
> > > size. Therefore, the prototype of kimage_map_segment() can be changed.
> >
> > Because no cover letter, I just reply here.
> >
> > I am testing code of (tag: next-20251125, next/master) on arm64 system.
> > I saw your two patches are already in there. When I used kexec reboot
> > as below, I still got the warning message during ima_kexec_post_load()
> > invocation.
> >
> > ====================
> > kexec -d -l /boot/vmlinuz-6.18.0-rc7-next-20251125 --initrd /boot/initramfs-6.18.0-rc7-next-20251125.img --reuse-cmdline
> > ====================
> >
>
> "I have used the Fedora 42 server and its config file to reproduce the
> issue you reported here. However, I cannot reproduce it with my patch.
> Instead, if I revert my patch, I can see the warning again.
>
> I suspect that you observed the warning thrown by the original Fedora
> 42 kernel instead of mine.
>
> You need to kexec-reboot into vmlinuz-6.18.0-rc7-next-20251125, and at
> that point, try 'kexec -d -l /boot/vmlinuz-6.18.0-rc7-next-20251125
> --initrd /boot/initramfs-6.18.0-rc7-next-20251125.img
> --reuse-cmdline'.
>
> If this is a false alarm, I will rewrite the commit log and send out v3.
Thanks for checking, Pingfan. I am not quite sure if I did wrong
operation on the machine. Since you are testing the same machine and the
same linux-next code, and it still can'e be reproduced, it should be
from my mistake. Then please go ahead posting v3.
Thanks
Baoquan
^ permalink raw reply
* Re: [PATCHv2 1/2] kernel/kexec: Change the prototype of kimage_map_segment()
From: Pingfan Liu @ 2025-12-03 4:22 UTC (permalink / raw)
To: Baoquan He
Cc: kexec, linux-integrity, Andrew Morton, Mimi Zohar, Roberto Sassu,
Alexander Graf, Steven Chen, stable
In-Reply-To: <aSZTb1X26MjSZIzF@MiWiFi-R3L-srv>
Hi Baoquan,
On Wed, Nov 26, 2025 at 9:10 AM Baoquan He <bhe@redhat.com> wrote:
>
> Hi Pingfan,
>
> On 11/06/25 at 02:59pm, Pingfan Liu wrote:
> > The kexec segment index will be required to extract the corresponding
> > information for that segment in kimage_map_segment(). Additionally,
> > kexec_segment already holds the kexec relocation destination address and
> > size. Therefore, the prototype of kimage_map_segment() can be changed.
>
> Because no cover letter, I just reply here.
>
> I am testing code of (tag: next-20251125, next/master) on arm64 system.
> I saw your two patches are already in there. When I used kexec reboot
> as below, I still got the warning message during ima_kexec_post_load()
> invocation.
>
> ====================
> kexec -d -l /boot/vmlinuz-6.18.0-rc7-next-20251125 --initrd /boot/initramfs-6.18.0-rc7-next-20251125.img --reuse-cmdline
> ====================
>
"I have used the Fedora 42 server and its config file to reproduce the
issue you reported here. However, I cannot reproduce it with my patch.
Instead, if I revert my patch, I can see the warning again.
I suspect that you observed the warning thrown by the original Fedora
42 kernel instead of mine.
You need to kexec-reboot into vmlinuz-6.18.0-rc7-next-20251125, and at
that point, try 'kexec -d -l /boot/vmlinuz-6.18.0-rc7-next-20251125
--initrd /boot/initramfs-6.18.0-rc7-next-20251125.img
--reuse-cmdline'.
If this is a false alarm, I will rewrite the commit log and send out v3.
Thanks,
Pingfan
> ====================
> [34283.657670] kexec_file: kernel: 000000006cf71829 kernel_size: 0x48b0000
> [34283.657700] PEFILE: Unsigned PE binary
> [34283.676597] ima: kexec measurement buffer for the loaded kernel at 0xff206000.
> [34283.676621] kexec_file: Loaded initrd at 0x84cb0000 bufsz=0x25ec426 memsz=0x25ed000
> [34283.684646] kexec_file: Loaded dtb at 0xff400000 bufsz=0x39e memsz=0x1000
> [34283.684653] kexec_file(Image): Loaded kernel at 0x80400000 bufsz=0x48b0000 memsz=0x48b0000
> [34283.684663] kexec_file: nr_segments = 4
> [34283.684666] kexec_file: segment[0]: buf=0x0000000000000000 bufsz=0x0 mem=0xff206000 memsz=0x1000
> [34283.684674] kexec_file: segment[1]: buf=0x000000006cf71829 bufsz=0x48b0000 mem=0x80400000 memsz=0x48b0000
> [34283.725987] kexec_file: segment[2]: buf=0x00000000c7369de6 bufsz=0x25ec426 mem=0x84cb0000 memsz=0x25ed000
> [34283.747670] kexec_file: segmen
> ** replaying previous printk message **
> [34283.747670] kexec_file: segment[3]: buf=0x00000000d83b530b bufsz=0x39e mem=0xff400000 memsz=0x1000
> [34283.747973] ------------[ cut here ]------------
> [34283.747976] WARNING: CPU: 33 PID: 16112 at kernel/kexec_core.c:1002 kimage_map_segment+0x138/0x190
> [34283.778574] Modules linked in: rfkill vfat fat ipmi_ssif igb acpi_ipmi ipmi_si ipmi_devintf mlx5_fwctl i2c_algo_bit ipmi_msghandler fwctl fuse loop nfnetlink zram lz4hc_compress lz4_compress xfs mlx5_ib macsec mlx5_core nvme nvme_core mlxfw psample tls nvme_keyring nvme_auth pci_hyperv_intf sbsa_gwdt rpcrdma sunrpc rdma_ucm ib_uverbs ib_srpt ib_isert iscsi_target_mod target_core_mod ib_iser i2c_dev ib_umad rdma_cm ib_ipoib iw_cm ib_cm libiscsi ib_core scsi_transport_iscsi aes_neon_bs
> [34283.824233] CPU: 33 UID: 0 PID: 16112 Comm: kexec Tainted: G W 6.17.8-200.fc42.aarch64 #1 PREEMPT(voluntary)
> [34283.836355] Tainted: [W]=WARN
> [34283.839684] Hardware name: CRAY CS500/CMUD , BIOS 1.4.0 Jun 17 2020
> [34283.846903] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [34283.854243] pc : kimage_map_segment+0x138/0x190
> [34283.859120] lr : kimage_map_segment+0x4c/0x190
> [34283.863920] sp : ffff8000a0643a90
> [34283.867394] x29: ffff8000a0643a90 x28: ffff800083d0a000 x27: 0000000000000000
> [34283.874901] x26: 0000aaaad722d4b0 x25: 000000000000008f x24: ffff800083d0a000
> [34283.882608] x23: 0000000000000001 x22: 00000000ff206000 x21: 00000000ff207000
> [34283.890305] x20: ffff008fbd306980 x19: ffff008f895d6400 x18: 00000000fffffff9
> [34283.897815] x17: 303d6d656d206539 x16: 3378303d7a736675 x15: 646565732d676e72
> [34283.905516] x14: 00646565732d726c x13: 616d692c78756e69 x12: 6c00636578656b2d
> [34283.912999] x11: 007265666675622d x10: 636578656b2d616d x9 : ffff80008050b73c
> [34283.920691] x8 : 0001000000000000 x7 : 0000000000000000 x6 : 0000000080000000
> [34283.928197] x5 : 0000000084cb0000 x4 : ffff008fbd2306b0 x3 : ffff008fbd305000
> [34283.935898] x2 : fffffff7ff000000 x1 : 0000000000000004 x0 : ffff800082046000
> [34283.943603] Call trace:
> [34283.946039] kimage_map_segment+0x138/0x190 (P)
> [34283.950935] ima_kexec_post_load+0x58/0xc0
> [34283.955225] __do_sys_kexec_file_load+0x2b8/0x398
> [34283.960279] __arm64_sys_kexec_file_load+0x28/0x40
> [34283.965965] invoke_syscall.constprop.0+0x64/0xe8
> [34283.971025] el0_svc_common.constprop.0+0x40/0xe8
> [34283.975883] do_el0_svc+0x24/0x38
> [34283.979361] el0_svc+0x3c/0x168
> [34283.982833] el0t_64_sync_handler+0xa0/0xf0
> [34283.987176] el0t_64_sync+0x1b0/0x1b8
> [34283.991000] ---[ end trace 0000000000000000 ]---
> [34283.996060] ------------[ cut here ]------------
> [34283.996064] WARNING: CPU: 33 PID: 16112 at mm/vmalloc.c:538 vmap_pages_pte_range+0x2bc/0x3c0
> [34284.010006] Modules linked in: rfkill vfat fat ipmi_ssif igb acpi_ipmi ipmi_si ipmi_devintf mlx5_fwctl i2c_algo_bit ipmi_msghandler fwctl fuse loop nfnetlink zram lz4hc_compress lz4_compress xfs mlx5_ib macsec mlx5_core nvme nvme_core mlxfw psample tls nvme_keyring nvme_auth pci_hyperv_intf sbsa_gwdt rpcrdma sunrpc rdma_ucm ib_uverbs ib_srpt ib_isert iscsi_target_mod target_core_mod ib_iser i2c_dev ib_umad rdma_cm ib_ipoib iw_cm ib_cm libiscsi ib_core scsi_transport_iscsi aes_neon_bs
> [34284.055630] CPU: 33 UID: 0 PID: 16112 Comm: kexec Tainted: G W 6.17.8-200.fc42.aarch64 #1 PREEMPT(voluntary)
> [34284.067701] Tainted: [W]=WARN
> [34284.070833] Hardware name: CRAY CS500/CMUD , BIOS 1.4.0 Jun 17 2020
> [34284.078238] pstate: 40400009 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [34284.085546] pc : vmap_pages_pte_range+0x2bc/0x3c0
> [34284.090607] lr : vmap_small_pages_range_noflush+0x16c/0x298
> [34284.096528] sp : ffff8000a0643940
> [34284.100001] x29: ffff8000a0643940 x28: 0000000000000000 x27: ffff800084f76000
> [34284.107699] x26: fffffdffc0000000 x25: ffff8000a06439d0 x24: ffff800082046000
> [34284.115174] x23: ffff800084f75000 x22: ffff007f80337ba8 x21: 03ffffffffffffc0
> [34284.122821] x20: ffff008fbd306980 x19: ffff8000a06439d4 x18: 00000000fffffff9
> [34284.130331] x17: 303d6d656d206539 x16: 3378303d7a736675 x15: 646565732d676e72
> [34284.138032] x14: 0000000000004000 x13: ffff009781307130 x12: 0000000000002000
> [34284.145733] x11: 0000000000000000 x10: 0000000000000001 x9 : ffff8000804e197c
> [34284.153248] x8 : 0000000000000027 x7 : ffff800085175000 x6 : ffff8000a06439d4
> [34284.160944] x5 : ffff8000a06439d0 x4 : ffff008fbd306980 x3 : 0068000000000f03
> [34284.168449] x2 : ffff007f80337ba8 x1 : 0000000000000000 x0 : 0000000000000000
> [34284.176150] Call trace:
> [34284.178768] vmap_pages_pte_range+0x2bc/0x3c0 (P)
> [34284.183665] vmap_small_pages_range_noflush+0x16c/0x298
> [34284.189264] vmap+0xb4/0x138
> [34284.192312] kimage_map_segment+0xdc/0x190
> [34284.196794] ima_kexec_post_load+0x58/0xc0
> [34284.201044] __do_sys_kexec_file_load+0x2b8/0x398
> [34284.206107] __arm64_sys_kexec_file_load+0x28/0x40
> [34284.211254] invoke_syscall.constprop.0+0x64/0xe8
> [34284.216139] el0_svc_common.constprop.0+0x40/0xe8
> [34284.221196] do_el0_svc+0x24/0x38
> [34284.224678] el0_svc+0x3c/0x168
> [34284.227983] el0t_64_sync_handler+0xa0/0xf0
> [34284.232526] el0t_64_sync+0x1b0/0x1b8
> [34284.236376] ---[ end trace 0000000000000000 ]---
> [34284.241412] kexec_core: Could not map ima buffer.
> [34284.241421] ima: Could not map measurements buffer.
> [34284.551336] machine_kexec_post_load:155:
> [34284.551354] kexec kimage info:
> [34284.551366] type: 0
> [34284.551373] head: 90363f9002
> [34284.551377] kern_reloc: 0x00000090363f7000
> [34284.551381] el2_vectors: 0x0000000000000000
> [34284.551384] kexec_file: kexec_file_load: type:0, start:0x80400000 head:0x90363f9002 flags:0x8
> ====================
>
> >
> > Fixes: 07d24902977e ("kexec: enable CMA based contiguous allocation")
> > Signed-off-by: Pingfan Liu <piliu@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Baoquan He <bhe@redhat.com>
> > Cc: Mimi Zohar <zohar@linux.ibm.com>
> > Cc: Roberto Sassu <roberto.sassu@huawei.com>
> > Cc: Alexander Graf <graf@amazon.com>
> > Cc: Steven Chen <chenste@linux.microsoft.com>
> > Cc: <stable@vger.kernel.org>
> > To: kexec@lists.infradead.org
> > To: linux-integrity@vger.kernel.org
> > ---
> > include/linux/kexec.h | 4 ++--
> > kernel/kexec_core.c | 9 ++++++---
> > security/integrity/ima/ima_kexec.c | 4 +---
> > 3 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index ff7e231b0485..8a22bc9b8c6c 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -530,7 +530,7 @@ extern bool kexec_file_dbg_print;
> > #define kexec_dprintk(fmt, arg...) \
> > do { if (kexec_file_dbg_print) pr_info(fmt, ##arg); } while (0)
> >
> > -extern void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size);
> > +extern void *kimage_map_segment(struct kimage *image, int idx);
> > extern void kimage_unmap_segment(void *buffer);
> > #else /* !CONFIG_KEXEC_CORE */
> > struct pt_regs;
> > @@ -540,7 +540,7 @@ static inline void __crash_kexec(struct pt_regs *regs) { }
> > static inline void crash_kexec(struct pt_regs *regs) { }
> > static inline int kexec_should_crash(struct task_struct *p) { return 0; }
> > static inline int kexec_crash_loaded(void) { return 0; }
> > -static inline void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size)
> > +static inline void *kimage_map_segment(struct kimage *image, int idx)
> > { return NULL; }
> > static inline void kimage_unmap_segment(void *buffer) { }
> > #define kexec_in_progress false
> > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > index fa00b239c5d9..9a1966207041 100644
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -960,17 +960,20 @@ int kimage_load_segment(struct kimage *image, int idx)
> > return result;
> > }
> >
> > -void *kimage_map_segment(struct kimage *image,
> > - unsigned long addr, unsigned long size)
> > +void *kimage_map_segment(struct kimage *image, int idx)
> > {
> > + unsigned long addr, size, eaddr;
> > unsigned long src_page_addr, dest_page_addr = 0;
> > - unsigned long eaddr = addr + size;
> > kimage_entry_t *ptr, entry;
> > struct page **src_pages;
> > unsigned int npages;
> > void *vaddr = NULL;
> > int i;
> >
> > + addr = image->segment[idx].mem;
> > + size = image->segment[idx].memsz;
> > + eaddr = addr + size;
> > +
> > /*
> > * Collect the source pages and map them in a contiguous VA range.
> > */
> > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > index 7362f68f2d8b..5beb69edd12f 100644
> > --- a/security/integrity/ima/ima_kexec.c
> > +++ b/security/integrity/ima/ima_kexec.c
> > @@ -250,9 +250,7 @@ void ima_kexec_post_load(struct kimage *image)
> > if (!image->ima_buffer_addr)
> > return;
> >
> > - ima_kexec_buffer = kimage_map_segment(image,
> > - image->ima_buffer_addr,
> > - image->ima_buffer_size);
> > + ima_kexec_buffer = kimage_map_segment(image, image->ima_segment_index);
> > if (!ima_kexec_buffer) {
> > pr_err("Could not map measurements buffer.\n");
> > return;
> > --
> > 2.49.0
> >
>
^ permalink raw reply
* Re: [PATCH v7 01/11] tpm: Cap the number of PCR banks
From: Jarkko Sakkinen @ 2025-12-03 2:03 UTC (permalink / raw)
To: Lai, Yi
Cc: linux-integrity, ross.philipson, Jonathan McDowell,
Stefano Garzarella, Jarkko Sakkinen, Roberto Sassu,
Jonathan McDowell, Peter Huewe, Jason Gunthorpe, linux-kernel,
yi1.lai, syzkaller-bugs
In-Reply-To: <aS+Rze1SqZFkEtwa@ly-workstation>
On Wed, Dec 03, 2025 at 09:26:37AM +0800, Lai, Yi wrote:
> On Wed, Dec 03, 2025 at 03:11:54AM +0200, Jarkko Sakkinen wrote:
> > On Wed, Dec 03, 2025 at 08:57:10AM +0800, Lai, Yi wrote:
> > > On Thu, Nov 27, 2025 at 03:54:33PM +0200, Jarkko Sakkinen wrote:
> > > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > > >
> > > > tpm2_get_pcr_allocation() does not cap any upper limit for the number of
> > > > banks. Cap the limit to eight banks so that out of bounds values coming
> > > > from external I/O cause on only limited harm.
> > > >
> > > > Cc: Roberto Sassu <roberto.sassu@huawei.com>
> > > > Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array")
> > > > Reviewed-By: Jonathan McDowell <noodles@meta.com>
> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > > > ---
> > > > v7:
> > > > - In Ryzen desktop there is total three banks so yep, eight is probably
> > > > much safer bet than four banks. Fixed the commit message as per remark
> > > > from Jonathan:
> > > >
> > > > https://lore.kernel.org/linux-integrity/aPYg1N0TvrkG6AJI@earth.li/#t
> > > >
> > > > And with that added also reviewed-by.
> > > > v6
> > > > - No changes.
> > > > v5:
> > > > - No changes.
> > > > v4:
> > > > - Revert spurious changes from include/linux/tpm.h.
> > > > - Increase TPM2_MAX_BANKS to 8.
> > > > - Rename TPM2_MAX_BANKS as TPM2_MAX_PCR_BANKS for the sake of clarity.
> > > > v3:
> > > > - Wrote a more clear commit message.
> > > > - Fixed pr_err() message.
> > > > v2:
> > > > - A new patch.
> > > > ---
> > > > drivers/char/tpm/tpm-chip.c | 13 +++++++++----
> > > > drivers/char/tpm/tpm.h | 1 -
> > > > drivers/char/tpm/tpm1-cmd.c | 25 -------------------------
> > > > drivers/char/tpm/tpm2-cmd.c | 8 +++-----
> > > > include/linux/tpm.h | 8 +++++---
> > > > 5 files changed, 17 insertions(+), 38 deletions(-)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > > index e25daf2396d3..6cb25862688f 100644
> > > > --- a/drivers/char/tpm/tpm-chip.c
> > > > +++ b/drivers/char/tpm/tpm-chip.c
> > > > @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
> > > >
> > > > static int tpm_get_pcr_allocation(struct tpm_chip *chip)
> > > > {
> > > > - int rc;
> > > > + int rc = 0;
> > > >
> > > > if (tpm_is_firmware_upgrade(chip))
> > > > return 0;
> > > >
> > > > - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
> > > > - tpm2_get_pcr_allocation(chip) :
> > > > - tpm1_get_pcr_allocation(chip);
> > > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > > > + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > > + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > > + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > > + chip->nr_allocated_banks = 1;
> > > > + } else {
> > > > + rc = tpm2_get_pcr_allocation(chip);
> > > > + }
> > > >
> > > > if (rc > 0)
> > > > return -ENODEV;
> > > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > > index 2726bd38e5ac..a37712c02e44 100644
> > > > --- a/drivers/char/tpm/tpm.h
> > > > +++ b/drivers/char/tpm/tpm.h
> > > > @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> > > > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> > > > const char *desc, size_t min_cap_length);
> > > > int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> > > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip);
> > > > unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > > > int tpm_pm_suspend(struct device *dev);
> > > > int tpm_pm_resume(struct device *dev);
> > > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > > > index 11088bda4e68..708bc553437b 100644
> > > > --- a/drivers/char/tpm/tpm1-cmd.c
> > > > +++ b/drivers/char/tpm/tpm1-cmd.c
> > > > @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
> > > >
> > > > return rc;
> > > > }
> > > > -
> > > > -/**
> > > > - * tpm1_get_pcr_allocation() - initialize the allocated bank
> > > > - * @chip: TPM chip to use.
> > > > - *
> > > > - * The function initializes the SHA1 allocated bank to extend PCR
> > > > - *
> > > > - * Return:
> > > > - * * 0 on success,
> > > > - * * < 0 on error.
> > > > - */
> > > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip)
> > > > -{
> > > > - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> > > > - GFP_KERNEL);
> > > > - if (!chip->allocated_banks)
> > > > - return -ENOMEM;
> > > > -
> > > > - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > > - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > > - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > > - chip->nr_allocated_banks = 1;
> > > > -
> > > > - return 0;
> > > > -}
> > > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > > index 7d77f6fbc152..97501c567c34 100644
> > > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > > @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> > > >
> > > > nr_possible_banks = be32_to_cpup(
> > > > (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> > > > -
> > > > - chip->allocated_banks = kcalloc(nr_possible_banks,
> > > > - sizeof(*chip->allocated_banks),
> > > > - GFP_KERNEL);
> > > > - if (!chip->allocated_banks) {
> > > > + if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
> > > > + pr_err("tpm: unexpected number of banks: %u > %u",
> > > > + nr_possible_banks, TPM2_MAX_PCR_BANKS);
> > > > rc = -ENOMEM;
> > > > goto out;
> > > > }
> > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > > index dc0338a783f3..eb0ff071bcae 100644
> > > > --- a/include/linux/tpm.h
> > > > +++ b/include/linux/tpm.h
> > > > @@ -26,7 +26,9 @@
> > > > #include <crypto/aes.h>
> > > >
> > > > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> > > > -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > > +
> > > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > > +#define TPM2_MAX_PCR_BANKS 8
> > > >
> > > > struct tpm_chip;
> > > > struct trusted_key_payload;
> > > > @@ -68,7 +70,7 @@ enum tpm2_curves {
> > > >
> > > > struct tpm_digest {
> > > > u16 alg_id;
> > > > - u8 digest[TPM_MAX_DIGEST_SIZE];
> > > > + u8 digest[TPM2_MAX_DIGEST_SIZE];
> > > > } __packed;
> > > >
> > > > struct tpm_bank_info {
> > > > @@ -189,7 +191,7 @@ struct tpm_chip {
> > > > unsigned int groups_cnt;
> > > >
> > > > u32 nr_allocated_banks;
> > > > - struct tpm_bank_info *allocated_banks;
> > > > + struct tpm_bank_info allocated_banks[TPM2_MAX_PCR_BANKS];
> > > > #ifdef CONFIG_ACPI
> > > > acpi_handle acpi_dev_handle;
> > > > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > > > --
> > > > 2.52.0
> > > >
> > >
> > > Hi Jarkko Sakkinen,
> > >
> > > Greetings!
> > >
> > > I used Syzkaller and found that there is KASAN: invalid-free in tpm_dev_release in linux-tpmdd branch tpmdd-next-6.19-rc1-v3.
> > >
> > > After bisection and the first bad commit is:
> > > "
> > > 83f6ace27d21 tpm: Cap the number of PCR banks
> >
> > Thank you. I updated the patch in my branch:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=bb731e948843a947b2b7e51552c1387e2c377e03
> >
>
> Re-test using the latest branch. Issue cannot be reproduced using the
> same binary.
>
> Regards,
> Yi Lai
Thank you!
BR, Jarkko
^ permalink raw reply
* Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-6.19-rc1-v3
From: Jarkko Sakkinen @ 2025-12-03 2:02 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Huewe, Jason Gunthorpe, David Howells, keyrings,
linux-integrity, linux-kernel
In-Reply-To: <aS4m3QWXzE4nSG1O@kernel.org>
On Tue, Dec 02, 2025 at 01:38:05AM +0200, Jarkko Sakkinen wrote:
> The following changes since commit 4664fb427c8fd0080f40109f5e2b2090a6fb0c84:
>
> Merge tag 'vfs-6.19-rc1.minix' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs (2025-12-01 15:22:40 -0800)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/tpmdd-next-6.19-rc1-v3
>
> for you to fetch changes up to b8b93e380604ce33e41e78522406df011f07aefe:
>
> KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 (2025-12-02 01:30:23 +0200)
>
> ----------------------------------------------------------------
> Hi,
>
> This pull request for TPM driver contains changes to unify TPM return
> code translation between trusted_tpm2 and TPM driver itself. Other than
> that the changes are either bug fixes or minor imrovements.
>
> Removed commits from earlier revisions and associated lore links:
>
> 1. "drivers/char/tpm: use min() instead of min_t()"
> https://lore.kernel.org/all/20251201161228.3c09d88a@pumpkin/
> 2. "Documentation: tpm-security.rst: change title to section"
> https://lore.kernel.org/all/86514a6ab364e01f163470a91cacef120e1b8b47.camel@HansenPartnership.com/
>
> BR, Jarkko
>
> ----------------------------------------------------------------
> Chu Guangqing (1):
> tpm_crb: Fix a spelling mistake
>
> Jarkko Sakkinen (3):
> tpm: Cap the number of PCR banks
> tpm: Use -EPERM as fallback error code in tpm_ret_to_err
> KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2
>
> Jonathan McDowell (1):
> tpm: Remove tpm_find_get_ops
>
> Marco Crivellari (1):
> tpm: add WQ_PERCPU to alloc_workqueue users
>
> Maurice Hieronymus (1):
> selftests: tpm2: Fix ill defined assertions
>
> Stuart Yoder (1):
> tpm_crb: add missing loc parameter to kerneldoc
>
> drivers/char/tpm/tpm-chip.c | 36 -------------------------------
> drivers/char/tpm/tpm-dev-common.c | 3 ++-
> drivers/char/tpm/tpm-interface.c | 20 +++++++++++++----
> drivers/char/tpm/tpm.h | 1 -
> drivers/char/tpm/tpm1-cmd.c | 5 -----
> drivers/char/tpm/tpm2-cmd.c | 8 +++----
> drivers/char/tpm/tpm_crb.c | 4 +++-
> drivers/char/tpm/tpm_tis_core.c | 3 +--
> include/linux/tpm.h | 12 +++++++----
> security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++----------------
> tools/testing/selftests/tpm2/tpm2.py | 4 ++--
> 11 files changed, 42 insertions(+), 80 deletions(-)
This is getting silly but I'll update this one more time.
BR, Jarkkok
^ permalink raw reply
* Re: [PATCH v7 01/11] tpm: Cap the number of PCR banks
From: Jarkko Sakkinen @ 2025-12-03 1:54 UTC (permalink / raw)
To: Lai, Yi
Cc: linux-integrity, ross.philipson, Jonathan McDowell,
Stefano Garzarella, Jarkko Sakkinen, Roberto Sassu,
Jonathan McDowell, Peter Huewe, Jason Gunthorpe, linux-kernel,
yi1.lai, syzkaller-bugs
In-Reply-To: <aS-YQjztmfbJCKNd@kernel.org>
On Wed, Dec 03, 2025 at 03:54:15AM +0200, Jarkko Sakkinen wrote:
> On Wed, Dec 03, 2025 at 03:11:59AM +0200, Jarkko Sakkinen wrote:
> > On Wed, Dec 03, 2025 at 08:57:10AM +0800, Lai, Yi wrote:
> > > On Thu, Nov 27, 2025 at 03:54:33PM +0200, Jarkko Sakkinen wrote:
> > > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > > >
> > > > tpm2_get_pcr_allocation() does not cap any upper limit for the number of
> > > > banks. Cap the limit to eight banks so that out of bounds values coming
> > > > from external I/O cause on only limited harm.
> > > >
> > > > Cc: Roberto Sassu <roberto.sassu@huawei.com>
> > > > Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array")
> > > > Reviewed-By: Jonathan McDowell <noodles@meta.com>
> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > > > ---
> > > > v7:
> > > > - In Ryzen desktop there is total three banks so yep, eight is probably
> > > > much safer bet than four banks. Fixed the commit message as per remark
> > > > from Jonathan:
> > > >
> > > > https://lore.kernel.org/linux-integrity/aPYg1N0TvrkG6AJI@earth.li/#t
> > > >
> > > > And with that added also reviewed-by.
> > > > v6
> > > > - No changes.
> > > > v5:
> > > > - No changes.
> > > > v4:
> > > > - Revert spurious changes from include/linux/tpm.h.
> > > > - Increase TPM2_MAX_BANKS to 8.
> > > > - Rename TPM2_MAX_BANKS as TPM2_MAX_PCR_BANKS for the sake of clarity.
> > > > v3:
> > > > - Wrote a more clear commit message.
> > > > - Fixed pr_err() message.
> > > > v2:
> > > > - A new patch.
> > > > ---
> > > > drivers/char/tpm/tpm-chip.c | 13 +++++++++----
> > > > drivers/char/tpm/tpm.h | 1 -
> > > > drivers/char/tpm/tpm1-cmd.c | 25 -------------------------
> > > > drivers/char/tpm/tpm2-cmd.c | 8 +++-----
> > > > include/linux/tpm.h | 8 +++++---
> > > > 5 files changed, 17 insertions(+), 38 deletions(-)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > > index e25daf2396d3..6cb25862688f 100644
> > > > --- a/drivers/char/tpm/tpm-chip.c
> > > > +++ b/drivers/char/tpm/tpm-chip.c
> > > > @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
> > > >
> > > > static int tpm_get_pcr_allocation(struct tpm_chip *chip)
> > > > {
> > > > - int rc;
> > > > + int rc = 0;
> > > >
> > > > if (tpm_is_firmware_upgrade(chip))
> > > > return 0;
> > > >
> > > > - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
> > > > - tpm2_get_pcr_allocation(chip) :
> > > > - tpm1_get_pcr_allocation(chip);
> > > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > > > + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > > + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > > + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > > + chip->nr_allocated_banks = 1;
> > > > + } else {
> > > > + rc = tpm2_get_pcr_allocation(chip);
> > > > + }
> > > >
> > > > if (rc > 0)
> > > > return -ENODEV;
> > > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > > index 2726bd38e5ac..a37712c02e44 100644
> > > > --- a/drivers/char/tpm/tpm.h
> > > > +++ b/drivers/char/tpm/tpm.h
> > > > @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> > > > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> > > > const char *desc, size_t min_cap_length);
> > > > int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> > > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip);
> > > > unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > > > int tpm_pm_suspend(struct device *dev);
> > > > int tpm_pm_resume(struct device *dev);
> > > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > > > index 11088bda4e68..708bc553437b 100644
> > > > --- a/drivers/char/tpm/tpm1-cmd.c
> > > > +++ b/drivers/char/tpm/tpm1-cmd.c
> > > > @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
> > > >
> > > > return rc;
> > > > }
> > > > -
> > > > -/**
> > > > - * tpm1_get_pcr_allocation() - initialize the allocated bank
> > > > - * @chip: TPM chip to use.
> > > > - *
> > > > - * The function initializes the SHA1 allocated bank to extend PCR
> > > > - *
> > > > - * Return:
> > > > - * * 0 on success,
> > > > - * * < 0 on error.
> > > > - */
> > > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip)
> > > > -{
> > > > - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> > > > - GFP_KERNEL);
> > > > - if (!chip->allocated_banks)
> > > > - return -ENOMEM;
> > > > -
> > > > - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > > - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > > - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > > - chip->nr_allocated_banks = 1;
> > > > -
> > > > - return 0;
> > > > -}
> > > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > > index 7d77f6fbc152..97501c567c34 100644
> > > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > > @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> > > >
> > > > nr_possible_banks = be32_to_cpup(
> > > > (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> > > > -
> > > > - chip->allocated_banks = kcalloc(nr_possible_banks,
> > > > - sizeof(*chip->allocated_banks),
> > > > - GFP_KERNEL);
> > > > - if (!chip->allocated_banks) {
> > > > + if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
> > > > + pr_err("tpm: unexpected number of banks: %u > %u",
> > > > + nr_possible_banks, TPM2_MAX_PCR_BANKS);
> > > > rc = -ENOMEM;
> > > > goto out;
> > > > }
> > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > > index dc0338a783f3..eb0ff071bcae 100644
> > > > --- a/include/linux/tpm.h
> > > > +++ b/include/linux/tpm.h
> > > > @@ -26,7 +26,9 @@
> > > > #include <crypto/aes.h>
> > > >
> > > > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> > > > -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > > +
> > > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > > +#define TPM2_MAX_PCR_BANKS 8
> > > >
> > > > struct tpm_chip;
> > > > struct trusted_key_payload;
> > > > @@ -68,7 +70,7 @@ enum tpm2_curves {
> > > >
> > > > struct tpm_digest {
> > > > u16 alg_id;
> > > > - u8 digest[TPM_MAX_DIGEST_SIZE];
> > > > + u8 digest[TPM2_MAX_DIGEST_SIZE];
> > > > } __packed;
> > > >
> > > > struct tpm_bank_info {
> > > > @@ -189,7 +191,7 @@ struct tpm_chip {
> > > > unsigned int groups_cnt;
> > > >
> > > > u32 nr_allocated_banks;
> > > > - struct tpm_bank_info *allocated_banks;
> > > > + struct tpm_bank_info allocated_banks[TPM2_MAX_PCR_BANKS];
> > > > #ifdef CONFIG_ACPI
> > > > acpi_handle acpi_dev_handle;
> > > > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > > > --
> > > > 2.52.0
> > > >
> > >
> > > Hi Jarkko Sakkinen,
> > >
> > > Greetings!
> > >
> > > I used Syzkaller and found that there is KASAN: invalid-free in tpm_dev_release in linux-tpmdd branch tpmdd-next-6.19-rc1-v3.
> > >
> > > After bisection and the first bad commit is:
> > > "
> > > 83f6ace27d21 tpm: Cap the number of PCR banks
> >
> > Thank you. I updated the patch in my branch:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=bb731e948843a947b2b7e51552c1387e2c377e03
> >
> > Possible to re-test (I can add your tested-by afterwards)?
>
> I mean tested-by AND reported-by.
Duh. Only tested-by since the patch is not yet in mainline (sorry) :-)
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v7 01/11] tpm: Cap the number of PCR banks
From: Jarkko Sakkinen @ 2025-12-03 1:54 UTC (permalink / raw)
To: Lai, Yi
Cc: linux-integrity, ross.philipson, Jonathan McDowell,
Stefano Garzarella, Jarkko Sakkinen, Roberto Sassu,
Jonathan McDowell, Peter Huewe, Jason Gunthorpe, linux-kernel,
yi1.lai, syzkaller-bugs
In-Reply-To: <aS-OWjUbhERbh3Za@kernel.org>
On Wed, Dec 03, 2025 at 03:11:59AM +0200, Jarkko Sakkinen wrote:
> On Wed, Dec 03, 2025 at 08:57:10AM +0800, Lai, Yi wrote:
> > On Thu, Nov 27, 2025 at 03:54:33PM +0200, Jarkko Sakkinen wrote:
> > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > >
> > > tpm2_get_pcr_allocation() does not cap any upper limit for the number of
> > > banks. Cap the limit to eight banks so that out of bounds values coming
> > > from external I/O cause on only limited harm.
> > >
> > > Cc: Roberto Sassu <roberto.sassu@huawei.com>
> > > Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array")
> > > Reviewed-By: Jonathan McDowell <noodles@meta.com>
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > > ---
> > > v7:
> > > - In Ryzen desktop there is total three banks so yep, eight is probably
> > > much safer bet than four banks. Fixed the commit message as per remark
> > > from Jonathan:
> > >
> > > https://lore.kernel.org/linux-integrity/aPYg1N0TvrkG6AJI@earth.li/#t
> > >
> > > And with that added also reviewed-by.
> > > v6
> > > - No changes.
> > > v5:
> > > - No changes.
> > > v4:
> > > - Revert spurious changes from include/linux/tpm.h.
> > > - Increase TPM2_MAX_BANKS to 8.
> > > - Rename TPM2_MAX_BANKS as TPM2_MAX_PCR_BANKS for the sake of clarity.
> > > v3:
> > > - Wrote a more clear commit message.
> > > - Fixed pr_err() message.
> > > v2:
> > > - A new patch.
> > > ---
> > > drivers/char/tpm/tpm-chip.c | 13 +++++++++----
> > > drivers/char/tpm/tpm.h | 1 -
> > > drivers/char/tpm/tpm1-cmd.c | 25 -------------------------
> > > drivers/char/tpm/tpm2-cmd.c | 8 +++-----
> > > include/linux/tpm.h | 8 +++++---
> > > 5 files changed, 17 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index e25daf2396d3..6cb25862688f 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
> > >
> > > static int tpm_get_pcr_allocation(struct tpm_chip *chip)
> > > {
> > > - int rc;
> > > + int rc = 0;
> > >
> > > if (tpm_is_firmware_upgrade(chip))
> > > return 0;
> > >
> > > - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
> > > - tpm2_get_pcr_allocation(chip) :
> > > - tpm1_get_pcr_allocation(chip);
> > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > > + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > + chip->nr_allocated_banks = 1;
> > > + } else {
> > > + rc = tpm2_get_pcr_allocation(chip);
> > > + }
> > >
> > > if (rc > 0)
> > > return -ENODEV;
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 2726bd38e5ac..a37712c02e44 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> > > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> > > const char *desc, size_t min_cap_length);
> > > int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip);
> > > unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > > int tpm_pm_suspend(struct device *dev);
> > > int tpm_pm_resume(struct device *dev);
> > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > > index 11088bda4e68..708bc553437b 100644
> > > --- a/drivers/char/tpm/tpm1-cmd.c
> > > +++ b/drivers/char/tpm/tpm1-cmd.c
> > > @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
> > >
> > > return rc;
> > > }
> > > -
> > > -/**
> > > - * tpm1_get_pcr_allocation() - initialize the allocated bank
> > > - * @chip: TPM chip to use.
> > > - *
> > > - * The function initializes the SHA1 allocated bank to extend PCR
> > > - *
> > > - * Return:
> > > - * * 0 on success,
> > > - * * < 0 on error.
> > > - */
> > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip)
> > > -{
> > > - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> > > - GFP_KERNEL);
> > > - if (!chip->allocated_banks)
> > > - return -ENOMEM;
> > > -
> > > - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > - chip->nr_allocated_banks = 1;
> > > -
> > > - return 0;
> > > -}
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 7d77f6fbc152..97501c567c34 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> > >
> > > nr_possible_banks = be32_to_cpup(
> > > (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> > > -
> > > - chip->allocated_banks = kcalloc(nr_possible_banks,
> > > - sizeof(*chip->allocated_banks),
> > > - GFP_KERNEL);
> > > - if (!chip->allocated_banks) {
> > > + if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
> > > + pr_err("tpm: unexpected number of banks: %u > %u",
> > > + nr_possible_banks, TPM2_MAX_PCR_BANKS);
> > > rc = -ENOMEM;
> > > goto out;
> > > }
> > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > index dc0338a783f3..eb0ff071bcae 100644
> > > --- a/include/linux/tpm.h
> > > +++ b/include/linux/tpm.h
> > > @@ -26,7 +26,9 @@
> > > #include <crypto/aes.h>
> > >
> > > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> > > -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > +
> > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > +#define TPM2_MAX_PCR_BANKS 8
> > >
> > > struct tpm_chip;
> > > struct trusted_key_payload;
> > > @@ -68,7 +70,7 @@ enum tpm2_curves {
> > >
> > > struct tpm_digest {
> > > u16 alg_id;
> > > - u8 digest[TPM_MAX_DIGEST_SIZE];
> > > + u8 digest[TPM2_MAX_DIGEST_SIZE];
> > > } __packed;
> > >
> > > struct tpm_bank_info {
> > > @@ -189,7 +191,7 @@ struct tpm_chip {
> > > unsigned int groups_cnt;
> > >
> > > u32 nr_allocated_banks;
> > > - struct tpm_bank_info *allocated_banks;
> > > + struct tpm_bank_info allocated_banks[TPM2_MAX_PCR_BANKS];
> > > #ifdef CONFIG_ACPI
> > > acpi_handle acpi_dev_handle;
> > > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > > --
> > > 2.52.0
> > >
> >
> > Hi Jarkko Sakkinen,
> >
> > Greetings!
> >
> > I used Syzkaller and found that there is KASAN: invalid-free in tpm_dev_release in linux-tpmdd branch tpmdd-next-6.19-rc1-v3.
> >
> > After bisection and the first bad commit is:
> > "
> > 83f6ace27d21 tpm: Cap the number of PCR banks
>
> Thank you. I updated the patch in my branch:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=bb731e948843a947b2b7e51552c1387e2c377e03
>
> Possible to re-test (I can add your tested-by afterwards)?
I mean tested-by AND reported-by.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v7 01/11] tpm: Cap the number of PCR banks
From: Lai, Yi @ 2025-12-03 1:26 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, ross.philipson, Jonathan McDowell,
Stefano Garzarella, Jarkko Sakkinen, Roberto Sassu,
Jonathan McDowell, Peter Huewe, Jason Gunthorpe, linux-kernel,
yi1.lai, syzkaller-bugs
In-Reply-To: <aS-OWjUbhERbh3Za@kernel.org>
On Wed, Dec 03, 2025 at 03:11:54AM +0200, Jarkko Sakkinen wrote:
> On Wed, Dec 03, 2025 at 08:57:10AM +0800, Lai, Yi wrote:
> > On Thu, Nov 27, 2025 at 03:54:33PM +0200, Jarkko Sakkinen wrote:
> > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > >
> > > tpm2_get_pcr_allocation() does not cap any upper limit for the number of
> > > banks. Cap the limit to eight banks so that out of bounds values coming
> > > from external I/O cause on only limited harm.
> > >
> > > Cc: Roberto Sassu <roberto.sassu@huawei.com>
> > > Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array")
> > > Reviewed-By: Jonathan McDowell <noodles@meta.com>
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > > ---
> > > v7:
> > > - In Ryzen desktop there is total three banks so yep, eight is probably
> > > much safer bet than four banks. Fixed the commit message as per remark
> > > from Jonathan:
> > >
> > > https://lore.kernel.org/linux-integrity/aPYg1N0TvrkG6AJI@earth.li/#t
> > >
> > > And with that added also reviewed-by.
> > > v6
> > > - No changes.
> > > v5:
> > > - No changes.
> > > v4:
> > > - Revert spurious changes from include/linux/tpm.h.
> > > - Increase TPM2_MAX_BANKS to 8.
> > > - Rename TPM2_MAX_BANKS as TPM2_MAX_PCR_BANKS for the sake of clarity.
> > > v3:
> > > - Wrote a more clear commit message.
> > > - Fixed pr_err() message.
> > > v2:
> > > - A new patch.
> > > ---
> > > drivers/char/tpm/tpm-chip.c | 13 +++++++++----
> > > drivers/char/tpm/tpm.h | 1 -
> > > drivers/char/tpm/tpm1-cmd.c | 25 -------------------------
> > > drivers/char/tpm/tpm2-cmd.c | 8 +++-----
> > > include/linux/tpm.h | 8 +++++---
> > > 5 files changed, 17 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index e25daf2396d3..6cb25862688f 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
> > >
> > > static int tpm_get_pcr_allocation(struct tpm_chip *chip)
> > > {
> > > - int rc;
> > > + int rc = 0;
> > >
> > > if (tpm_is_firmware_upgrade(chip))
> > > return 0;
> > >
> > > - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
> > > - tpm2_get_pcr_allocation(chip) :
> > > - tpm1_get_pcr_allocation(chip);
> > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > > + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > + chip->nr_allocated_banks = 1;
> > > + } else {
> > > + rc = tpm2_get_pcr_allocation(chip);
> > > + }
> > >
> > > if (rc > 0)
> > > return -ENODEV;
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 2726bd38e5ac..a37712c02e44 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> > > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> > > const char *desc, size_t min_cap_length);
> > > int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip);
> > > unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > > int tpm_pm_suspend(struct device *dev);
> > > int tpm_pm_resume(struct device *dev);
> > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > > index 11088bda4e68..708bc553437b 100644
> > > --- a/drivers/char/tpm/tpm1-cmd.c
> > > +++ b/drivers/char/tpm/tpm1-cmd.c
> > > @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
> > >
> > > return rc;
> > > }
> > > -
> > > -/**
> > > - * tpm1_get_pcr_allocation() - initialize the allocated bank
> > > - * @chip: TPM chip to use.
> > > - *
> > > - * The function initializes the SHA1 allocated bank to extend PCR
> > > - *
> > > - * Return:
> > > - * * 0 on success,
> > > - * * < 0 on error.
> > > - */
> > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip)
> > > -{
> > > - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> > > - GFP_KERNEL);
> > > - if (!chip->allocated_banks)
> > > - return -ENOMEM;
> > > -
> > > - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > > - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > > - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > > - chip->nr_allocated_banks = 1;
> > > -
> > > - return 0;
> > > -}
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 7d77f6fbc152..97501c567c34 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> > >
> > > nr_possible_banks = be32_to_cpup(
> > > (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> > > -
> > > - chip->allocated_banks = kcalloc(nr_possible_banks,
> > > - sizeof(*chip->allocated_banks),
> > > - GFP_KERNEL);
> > > - if (!chip->allocated_banks) {
> > > + if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
> > > + pr_err("tpm: unexpected number of banks: %u > %u",
> > > + nr_possible_banks, TPM2_MAX_PCR_BANKS);
> > > rc = -ENOMEM;
> > > goto out;
> > > }
> > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > index dc0338a783f3..eb0ff071bcae 100644
> > > --- a/include/linux/tpm.h
> > > +++ b/include/linux/tpm.h
> > > @@ -26,7 +26,9 @@
> > > #include <crypto/aes.h>
> > >
> > > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> > > -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > +
> > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > > +#define TPM2_MAX_PCR_BANKS 8
> > >
> > > struct tpm_chip;
> > > struct trusted_key_payload;
> > > @@ -68,7 +70,7 @@ enum tpm2_curves {
> > >
> > > struct tpm_digest {
> > > u16 alg_id;
> > > - u8 digest[TPM_MAX_DIGEST_SIZE];
> > > + u8 digest[TPM2_MAX_DIGEST_SIZE];
> > > } __packed;
> > >
> > > struct tpm_bank_info {
> > > @@ -189,7 +191,7 @@ struct tpm_chip {
> > > unsigned int groups_cnt;
> > >
> > > u32 nr_allocated_banks;
> > > - struct tpm_bank_info *allocated_banks;
> > > + struct tpm_bank_info allocated_banks[TPM2_MAX_PCR_BANKS];
> > > #ifdef CONFIG_ACPI
> > > acpi_handle acpi_dev_handle;
> > > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > > --
> > > 2.52.0
> > >
> >
> > Hi Jarkko Sakkinen,
> >
> > Greetings!
> >
> > I used Syzkaller and found that there is KASAN: invalid-free in tpm_dev_release in linux-tpmdd branch tpmdd-next-6.19-rc1-v3.
> >
> > After bisection and the first bad commit is:
> > "
> > 83f6ace27d21 tpm: Cap the number of PCR banks
>
> Thank you. I updated the patch in my branch:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=bb731e948843a947b2b7e51552c1387e2c377e03
>
Re-test using the latest branch. Issue cannot be reproduced using the
same binary.
Regards,
Yi Lai
> Possible to re-test (I can add your tested-by afterwards)?
>
> BR, Jarkko
^ permalink raw reply
* Re: [PATCH v7 01/11] tpm: Cap the number of PCR banks
From: Jarkko Sakkinen @ 2025-12-03 1:11 UTC (permalink / raw)
To: Lai, Yi
Cc: linux-integrity, ross.philipson, Jonathan McDowell,
Stefano Garzarella, Jarkko Sakkinen, Roberto Sassu,
Jonathan McDowell, Peter Huewe, Jason Gunthorpe, linux-kernel,
yi1.lai, syzkaller-bugs
In-Reply-To: <aS+K5nO2MP7N+kxQ@ly-workstation>
On Wed, Dec 03, 2025 at 08:57:10AM +0800, Lai, Yi wrote:
> On Thu, Nov 27, 2025 at 03:54:33PM +0200, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> >
> > tpm2_get_pcr_allocation() does not cap any upper limit for the number of
> > banks. Cap the limit to eight banks so that out of bounds values coming
> > from external I/O cause on only limited harm.
> >
> > Cc: Roberto Sassu <roberto.sassu@huawei.com>
> > Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array")
> > Reviewed-By: Jonathan McDowell <noodles@meta.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > ---
> > v7:
> > - In Ryzen desktop there is total three banks so yep, eight is probably
> > much safer bet than four banks. Fixed the commit message as per remark
> > from Jonathan:
> >
> > https://lore.kernel.org/linux-integrity/aPYg1N0TvrkG6AJI@earth.li/#t
> >
> > And with that added also reviewed-by.
> > v6
> > - No changes.
> > v5:
> > - No changes.
> > v4:
> > - Revert spurious changes from include/linux/tpm.h.
> > - Increase TPM2_MAX_BANKS to 8.
> > - Rename TPM2_MAX_BANKS as TPM2_MAX_PCR_BANKS for the sake of clarity.
> > v3:
> > - Wrote a more clear commit message.
> > - Fixed pr_err() message.
> > v2:
> > - A new patch.
> > ---
> > drivers/char/tpm/tpm-chip.c | 13 +++++++++----
> > drivers/char/tpm/tpm.h | 1 -
> > drivers/char/tpm/tpm1-cmd.c | 25 -------------------------
> > drivers/char/tpm/tpm2-cmd.c | 8 +++-----
> > include/linux/tpm.h | 8 +++++---
> > 5 files changed, 17 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index e25daf2396d3..6cb25862688f 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
> >
> > static int tpm_get_pcr_allocation(struct tpm_chip *chip)
> > {
> > - int rc;
> > + int rc = 0;
> >
> > if (tpm_is_firmware_upgrade(chip))
> > return 0;
> >
> > - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
> > - tpm2_get_pcr_allocation(chip) :
> > - tpm1_get_pcr_allocation(chip);
> > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > + chip->nr_allocated_banks = 1;
> > + } else {
> > + rc = tpm2_get_pcr_allocation(chip);
> > + }
> >
> > if (rc > 0)
> > return -ENODEV;
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 2726bd38e5ac..a37712c02e44 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> > const char *desc, size_t min_cap_length);
> > int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> > -int tpm1_get_pcr_allocation(struct tpm_chip *chip);
> > unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > int tpm_pm_suspend(struct device *dev);
> > int tpm_pm_resume(struct device *dev);
> > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > index 11088bda4e68..708bc553437b 100644
> > --- a/drivers/char/tpm/tpm1-cmd.c
> > +++ b/drivers/char/tpm/tpm1-cmd.c
> > @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
> >
> > return rc;
> > }
> > -
> > -/**
> > - * tpm1_get_pcr_allocation() - initialize the allocated bank
> > - * @chip: TPM chip to use.
> > - *
> > - * The function initializes the SHA1 allocated bank to extend PCR
> > - *
> > - * Return:
> > - * * 0 on success,
> > - * * < 0 on error.
> > - */
> > -int tpm1_get_pcr_allocation(struct tpm_chip *chip)
> > -{
> > - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> > - GFP_KERNEL);
> > - if (!chip->allocated_banks)
> > - return -ENOMEM;
> > -
> > - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> > - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> > - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> > - chip->nr_allocated_banks = 1;
> > -
> > - return 0;
> > -}
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 7d77f6fbc152..97501c567c34 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> >
> > nr_possible_banks = be32_to_cpup(
> > (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> > -
> > - chip->allocated_banks = kcalloc(nr_possible_banks,
> > - sizeof(*chip->allocated_banks),
> > - GFP_KERNEL);
> > - if (!chip->allocated_banks) {
> > + if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
> > + pr_err("tpm: unexpected number of banks: %u > %u",
> > + nr_possible_banks, TPM2_MAX_PCR_BANKS);
> > rc = -ENOMEM;
> > goto out;
> > }
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index dc0338a783f3..eb0ff071bcae 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -26,7 +26,9 @@
> > #include <crypto/aes.h>
> >
> > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> > -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > +
> > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> > +#define TPM2_MAX_PCR_BANKS 8
> >
> > struct tpm_chip;
> > struct trusted_key_payload;
> > @@ -68,7 +70,7 @@ enum tpm2_curves {
> >
> > struct tpm_digest {
> > u16 alg_id;
> > - u8 digest[TPM_MAX_DIGEST_SIZE];
> > + u8 digest[TPM2_MAX_DIGEST_SIZE];
> > } __packed;
> >
> > struct tpm_bank_info {
> > @@ -189,7 +191,7 @@ struct tpm_chip {
> > unsigned int groups_cnt;
> >
> > u32 nr_allocated_banks;
> > - struct tpm_bank_info *allocated_banks;
> > + struct tpm_bank_info allocated_banks[TPM2_MAX_PCR_BANKS];
> > #ifdef CONFIG_ACPI
> > acpi_handle acpi_dev_handle;
> > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > --
> > 2.52.0
> >
>
> Hi Jarkko Sakkinen,
>
> Greetings!
>
> I used Syzkaller and found that there is KASAN: invalid-free in tpm_dev_release in linux-tpmdd branch tpmdd-next-6.19-rc1-v3.
>
> After bisection and the first bad commit is:
> "
> 83f6ace27d21 tpm: Cap the number of PCR banks
Thank you. I updated the patch in my branch:
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=bb731e948843a947b2b7e51552c1387e2c377e03
Possible to re-test (I can add your tested-by afterwards)?
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v7 01/11] tpm: Cap the number of PCR banks
From: Lai, Yi @ 2025-12-03 0:57 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, ross.philipson, Jonathan McDowell,
Stefano Garzarella, Jarkko Sakkinen, Roberto Sassu,
Jonathan McDowell, Peter Huewe, Jason Gunthorpe, linux-kernel,
yi1.lai, syzkaller-bugs
In-Reply-To: <20251127135445.2141241-2-jarkko@kernel.org>
On Thu, Nov 27, 2025 at 03:54:33PM +0200, Jarkko Sakkinen wrote:
> From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
> tpm2_get_pcr_allocation() does not cap any upper limit for the number of
> banks. Cap the limit to eight banks so that out of bounds values coming
> from external I/O cause on only limited harm.
>
> Cc: Roberto Sassu <roberto.sassu@huawei.com>
> Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array")
> Reviewed-By: Jonathan McDowell <noodles@meta.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> ---
> v7:
> - In Ryzen desktop there is total three banks so yep, eight is probably
> much safer bet than four banks. Fixed the commit message as per remark
> from Jonathan:
>
> https://lore.kernel.org/linux-integrity/aPYg1N0TvrkG6AJI@earth.li/#t
>
> And with that added also reviewed-by.
> v6
> - No changes.
> v5:
> - No changes.
> v4:
> - Revert spurious changes from include/linux/tpm.h.
> - Increase TPM2_MAX_BANKS to 8.
> - Rename TPM2_MAX_BANKS as TPM2_MAX_PCR_BANKS for the sake of clarity.
> v3:
> - Wrote a more clear commit message.
> - Fixed pr_err() message.
> v2:
> - A new patch.
> ---
> drivers/char/tpm/tpm-chip.c | 13 +++++++++----
> drivers/char/tpm/tpm.h | 1 -
> drivers/char/tpm/tpm1-cmd.c | 25 -------------------------
> drivers/char/tpm/tpm2-cmd.c | 8 +++-----
> include/linux/tpm.h | 8 +++++---
> 5 files changed, 17 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index e25daf2396d3..6cb25862688f 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
>
> static int tpm_get_pcr_allocation(struct tpm_chip *chip)
> {
> - int rc;
> + int rc = 0;
>
> if (tpm_is_firmware_upgrade(chip))
> return 0;
>
> - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
> - tpm2_get_pcr_allocation(chip) :
> - tpm1_get_pcr_allocation(chip);
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> + chip->nr_allocated_banks = 1;
> + } else {
> + rc = tpm2_get_pcr_allocation(chip);
> + }
>
> if (rc > 0)
> return -ENODEV;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 2726bd38e5ac..a37712c02e44 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> const char *desc, size_t min_cap_length);
> int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> -int tpm1_get_pcr_allocation(struct tpm_chip *chip);
> unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> int tpm_pm_suspend(struct device *dev);
> int tpm_pm_resume(struct device *dev);
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index 11088bda4e68..708bc553437b 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
>
> return rc;
> }
> -
> -/**
> - * tpm1_get_pcr_allocation() - initialize the allocated bank
> - * @chip: TPM chip to use.
> - *
> - * The function initializes the SHA1 allocated bank to extend PCR
> - *
> - * Return:
> - * * 0 on success,
> - * * < 0 on error.
> - */
> -int tpm1_get_pcr_allocation(struct tpm_chip *chip)
> -{
> - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> - GFP_KERNEL);
> - if (!chip->allocated_banks)
> - return -ENOMEM;
> -
> - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> - chip->nr_allocated_banks = 1;
> -
> - return 0;
> -}
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 7d77f6fbc152..97501c567c34 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>
> nr_possible_banks = be32_to_cpup(
> (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> -
> - chip->allocated_banks = kcalloc(nr_possible_banks,
> - sizeof(*chip->allocated_banks),
> - GFP_KERNEL);
> - if (!chip->allocated_banks) {
> + if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
> + pr_err("tpm: unexpected number of banks: %u > %u",
> + nr_possible_banks, TPM2_MAX_PCR_BANKS);
> rc = -ENOMEM;
> goto out;
> }
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index dc0338a783f3..eb0ff071bcae 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -26,7 +26,9 @@
> #include <crypto/aes.h>
>
> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> +
> +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
> +#define TPM2_MAX_PCR_BANKS 8
>
> struct tpm_chip;
> struct trusted_key_payload;
> @@ -68,7 +70,7 @@ enum tpm2_curves {
>
> struct tpm_digest {
> u16 alg_id;
> - u8 digest[TPM_MAX_DIGEST_SIZE];
> + u8 digest[TPM2_MAX_DIGEST_SIZE];
> } __packed;
>
> struct tpm_bank_info {
> @@ -189,7 +191,7 @@ struct tpm_chip {
> unsigned int groups_cnt;
>
> u32 nr_allocated_banks;
> - struct tpm_bank_info *allocated_banks;
> + struct tpm_bank_info allocated_banks[TPM2_MAX_PCR_BANKS];
> #ifdef CONFIG_ACPI
> acpi_handle acpi_dev_handle;
> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> --
> 2.52.0
>
Hi Jarkko Sakkinen,
Greetings!
I used Syzkaller and found that there is KASAN: invalid-free in tpm_dev_release in linux-tpmdd branch tpmdd-next-6.19-rc1-v3.
After bisection and the first bad commit is:
"
83f6ace27d21 tpm: Cap the number of PCR banks
"
All detailed into can be found at:
https://github.com/laifryiee/syzkaller_logs/tree/main/251202_193851_tpm_dev_release
Syzkaller repro code:
https://github.com/laifryiee/syzkaller_logs/tree/main/251202_193851_tpm_dev_release/repro.c
Syzkaller repro syscall steps:
https://github.com/laifryiee/syzkaller_logs/tree/main/251202_193851_tpm_dev_release/repro.prog
Syzkaller report:
https://github.com/laifryiee/syzkaller_logs/tree/main/251202_193851_tpm_dev_release/repro.report
Kconfig(make olddefconfig):
https://github.com/laifryiee/syzkaller_logs/tree/main/251202_193851_tpm_dev_release/kconfig_origin
Bisect info:
https://github.com/laifryiee/syzkaller_logs/tree/main/251202_193851_tpm_dev_release/bisect_info.log
bzImage:
https://github.com/laifryiee/syzkaller_logs/raw/refs/heads/main/251202_193851_tpm_dev_release/bzImage_tpmdd-next-6.19-rc1-v3
Issue dmesg:
https://github.com/laifryiee/syzkaller_logs/blob/main/251202_193851_tpm_dev_release/tpmdd-next-6.19-rc1-v3_dmesg.log
"
[ 18.405712] ==================================================================
[ 18.406210] BUG: KASAN: invalid-free in tpm_dev_release+0xdc/0x110
[ 18.406657] Free of addr ff11000011572d38 by task repro/729
[ 18.407037]
[ 18.407156] CPU: 1 UID: 0 PID: 729 Comm: repro Not tainted 6.18.0-tpmdd-next-6+ #1 PREEMPT(voluntary)
[ 18.407167] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.q4
[ 18.407173] Call Trace:
[ 18.407177] <TASK>
[ 18.407180] dump_stack_lvl+0xea/0x150
[ 18.407197] print_report+0xce/0x610
[ 18.407210] ? kasan_complete_mode_report_info+0x40/0x200
[ 18.407220] ? tpm_dev_release+0xdc/0x110
[ 18.407232] kasan_report_invalid_free+0xa7/0xd0
[ 18.407245] ? tpm_dev_release+0xdc/0x110
[ 18.407259] ? tpm_dev_release+0xdc/0x110
[ 18.407271] check_slab_allocation+0x128/0x140
[ 18.407281] __kasan_slab_pre_free+0xd/0x20
[ 18.407291] kfree+0x119/0x620
[ 18.407300] ? tpm_dev_release+0xdc/0x110
[ 18.407311] ? do_wait_intr_irq+0x1e0/0x230
[ 18.407326] ? __pfx_tpm_dev_release+0x10/0x10
[ 18.407340] tpm_dev_release+0xdc/0x110
[ 18.407351] ? tpm_dev_release+0xdc/0x110
[ 18.407363] device_release+0xb6/0x260
[ 18.407374] kobject_put+0x22d/0x550
[ 18.407383] put_device+0x29/0x40
[ 18.407391] vtpm_proxy_fops_release+0x152/0x1e0
[ 18.407403] ? __pfx_vtpm_proxy_fops_release+0x10/0x10
[ 18.407412] __fput+0x41f/0xb70
[ 18.407435] ____fput+0x22/0x30
[ 18.407449] task_work_run+0x19e/0x2b0
[ 18.407467] ? __pfx_task_work_run+0x10/0x10
[ 18.407480] ? __sanitizer_cov_trace_const_cmp4+0x1a/0x20
[ 18.407490] ? switch_task_namespaces+0xdd/0x130
[ 18.407502] do_exit+0x893/0x28c0
[ 18.407519] ? do_group_exit+0x1d8/0x2c0
[ 18.407532] ? __pfx_do_exit+0x10/0x10
[ 18.407545] ? __this_cpu_preempt_check+0x21/0x30
[ 18.407555] ? _raw_spin_unlock_irq+0x2c/0x60
[ 18.407571] ? lockdep_hardirqs_on+0x89/0x110
[ 18.407589] do_group_exit+0xe4/0x2c0
[ 18.407602] __x64_sys_exit_group+0x4d/0x60
[ 18.407615] x64_sys_call+0x21a2/0x21b0
[ 18.407627] do_syscall_64+0x6d/0x450
[ 18.407641] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 18.407650] RIP: 0033:0x7f1331118a4d
[ 18.407658] Code: Unable to access opcode bytes at 0x7f1331118a23.
[ 18.407662] RSP: 002b:00007ffec868c668 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
[ 18.407671] RAX: ffffffffffffffda RBX: 00007f13311f69e0 RCX: 00007f1331118a4d
[ 18.407677] RDX: 00000000000000e7 RSI: ffffffffffffff80 RDI: 0000000000000000
[ 18.407683] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000020
[ 18.407688] R10: 00007ffec868c510 R11: 0000000000000246 R12: 00007f13311f69e0
[ 18.407694] R13: 00007f13311fbf00 R14: 0000000000000001 R15: 00007f13311fbee8
[ 18.407707] </TASK>
[ 18.407710]
[ 18.423610] Allocated by task 729:
[ 18.423853] kasan_save_stack+0x2c/0x60
[ 18.424122] kasan_save_track+0x18/0x40
[ 18.424392] kasan_save_alloc_info+0x3c/0x50
[ 18.424690] __kasan_kmalloc+0x88/0xa0
[ 18.424954] __kmalloc_cache_noprof+0x2b1/0x840
[ 18.425271] tpm_chip_alloc+0x56/0x4c0
[ 18.425540] vtpmx_fops_ioctl+0x2d0/0x830
[ 18.425819] __x64_sys_ioctl+0x1bf/0x220
[ 18.426100] x64_sys_call+0x1280/0x21b0
[ 18.426366] do_syscall_64+0x6d/0x450
[ 18.426630] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 18.426980]
[ 18.427098] The buggy address belongs to the object at ff11000011572000
[ 18.427098] which belongs to the cache kmalloc-4k of size 4096
[ 18.427931] The buggy address is located 3384 bytes inside of
[ 18.427931] 3528-byte region [ff11000011572000, ff11000011572dc8)
[ 18.428720]
[ 18.428836] The buggy address belongs to the physical page:
[ 18.429210] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x11570
[ 18.429744] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 18.430258] anon flags: 0xfffffc0000040(head|node=0|zone=1|lastcpupid=0x1fffff)
[ 18.430754] page_type: f5(slab)
[ 18.430982] raw: 000fffffc0000040 ff1100000ac38140 0000000000000000 dead000000000001
[ 18.431505] raw: 0000000000000000 0000000080040004 00000000f5000000 0000000000000000
[ 18.432025] head: 000fffffc0000040 ff1100000ac38140 0000000000000000 dead000000000001
[ 18.432549] head: 0000000000000000 0000000080040004 00000000f5000000 0000000000000000
[ 18.433077] head: 000fffffc0000003 ffd4000000455c01 00000000ffffffff 00000000ffffffff
[ 18.433605] head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
[ 18.434126] page dumped because: kasan: bad access detected
[ 18.434504]
[ 18.434621] Memory state around the buggy address:
[ 18.434950] ff11000011572c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 18.435438] ff11000011572c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 18.435925] >ff11000011572d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 18.436399] ^
[ 18.436744] ff11000011572d80: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
[ 18.437229] ff11000011572e00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 18.437709] ==================================================================
"
Hope this cound be insightful to you.
Regards,
Yi Lai
---
If you don't need the following environment to reproduce the problem or if you
already have one reproduced environment, please ignore the following information.
How to reproduce:
git clone https://gitlab.com/xupengfe/repro_vm_env.git
cd repro_vm_env
tar -xvf repro_vm_env.tar.gz
cd repro_vm_env; ./start3.sh // it needs qemu-system-x86_64 and I used v7.1.0
// start3.sh will load bzImage_2241ab53cbb5cdb08a6b2d4688feb13971058f65 v6.2-rc5 kernel
// You could change the bzImage_xxx as you want
// Maybe you need to remove line "-drive if=pflash,format=raw,readonly=on,file=./OVMF_CODE.fd \" for different qemu version
You could use below command to log in, there is no password for root.
ssh -p 10023 root@localhost
After login vm(virtual machine) successfully, you could transfer reproduced
binary to the vm by below way, and reproduce the problem in vm:
gcc -pthread -o repro repro.c
scp -P 10023 repro root@localhost:/root/
Get the bzImage for target kernel:
Please use target kconfig and copy it to kernel_src/.config
make olddefconfig
make -jx bzImage //x should equal or less than cpu num your pc has
Fill the bzImage file into above start3.sh to load the target kernel in vm.
Tips:
If you already have qemu-system-x86_64, please ignore below info.
If you want to install qemu v7.1.0 version:
git clone https://github.com/qemu/qemu.git
cd qemu
git checkout -f v7.1.0
mkdir build
cd build
yum install -y ninja-build.x86_64
yum -y install libslirp-devel.x86_64
../configure --target-list=x86_64-softmmu --enable-kvm --enable-vnc --enable-gtk --enable-sdl --enable-usb-redir --enable-slirp
make
make install
^ permalink raw reply
* [PATCH 1/1] IMA event log trimming
From: steven chen @ 2025-12-02 23:28 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring
In-Reply-To: <20251202232857.8211-1-chenste@linux.microsoft.com>
This patch is for trimming N entries of the IMA event logs as well as
cleaning the hash table.
It provides a userspace interface ima_trim_log that can be used to input
number N to let kernel to trim N entries of IMA event logs. When read
this interface, it returns number of entries trimmed last tim.
A mutex ima_trim_list_mutex is provided to allow one trimming request
at a time.
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_fs.c | 78 ++++++++++++++++++++++++++++++
security/integrity/ima/ima_queue.c | 42 ++++++++++++++++
3 files changed, 122 insertions(+)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e3d71d8d56e3..ab0e30ee25ea 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
#ifdef CONFIG_IMA_KEXEC
void ima_measure_kexec_event(const char *event_name);
+long ima_purge_event_log(long number_logs);
#else
static inline void ima_measure_kexec_event(const char *event_name) {}
+static inline long ima_purge_event_log(long number_logs) { return 0; }
#endif
/*
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 87045b09f120..ea93448feedd 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -38,6 +38,11 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
static int valid_policy = 1;
+#define IMA_LOG_TRIM_REQ_LENGTH 11
+static long trimcount;
+/* mutex protects atomicity of trimming measurement list requests */
+static DEFINE_MUTEX(ima_trim_list_mutex);
+
static ssize_t ima_show_htable_value(char __user *buf, size_t count,
loff_t *ppos, atomic_long_t *val)
{
@@ -289,6 +294,69 @@ static const struct file_operations ima_ascii_measurements_ops = {
.release = seq_release,
};
+static int ima_log_trim_open(struct inode *inode, struct file *filp)
+{
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ return 0;
+}
+
+static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
+{
+ char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
+ ssize_t len;
+
+ len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
+ return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
+}
+
+static ssize_t ima_log_trim_write(struct file *file,
+ const char __user *buf, size_t datalen, loff_t *ppos)
+{
+ unsigned char req[IMA_LOG_TRIM_REQ_LENGTH];
+ long count, n;
+ int ret;
+
+ mutex_lock(&ima_trim_list_mutex);
+
+ if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ n = (int)datalen;
+
+ ret = copy_from_user(req, buf, datalen);
+ if (ret < 0)
+ goto out;
+
+ count = 0;
+ for (int i = 0; i < n; ++i) {
+ if (req[i] < '0' || req[i] > '9') {
+ ret = -EINVAL;
+ goto out;
+ }
+ count = count * 10 + req[i] - '0';
+ }
+ ret = ima_purge_event_log(count);
+
+ if (ret < 0)
+ goto out;
+
+ trimcount = ret;
+ ret = datalen;
+out:
+ mutex_unlock(&ima_trim_list_mutex);
+ return ret;
+}
+
+static const struct file_operations ima_log_trim_ops = {
+ .open = ima_log_trim_open,
+ .read = ima_log_trim_read,
+ .write = ima_log_trim_write,
+ .llseek = generic_file_llseek,
+};
+
static ssize_t ima_read_policy(char *path)
{
void *data = NULL;
@@ -528,6 +596,16 @@ int __init ima_fs_init(void)
goto out;
}
+ dentry = securityfs_create_file("ima_trim_log",
+ S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
+ ima_dir, NULL, &ima_log_trim_ops);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
+ trimcount = 0;
+
dentry = securityfs_create_file("runtime_measurements_count",
S_IRUSR | S_IRGRP, ima_dir, NULL,
&ima_measurements_count_ops);
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 590637e81ad1..999cd42c517c 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -220,6 +220,48 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
return result;
}
+/* Delete the IMA event logs */
+long ima_purge_event_log(long number_logs)
+{
+ struct ima_queue_entry *qe;
+ long cur = 0;
+
+ if (number_logs <= 0)
+ return number_logs;
+
+ mutex_lock(&ima_extend_list_mutex);
+ rcu_read_lock();
+
+ /*
+ * Remove this entry from both hash table and the measurement list
+ * When removing from hash table, decrease the length counter
+ * so that the hash table re-sizing logic works correctly
+ */
+ list_for_each_entry_rcu(qe, &ima_measurements, later) {
+ int i;
+
+ /* if CONFIG_IMA_DISABLE_HTABLE is set, the hash table is not used */
+ if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE))
+ hlist_del_rcu(&qe->hnext);
+
+ for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
+ kfree(qe->entry->template_data[i].data);
+ qe->entry->template_data[i].data = NULL;
+ qe->entry->template_data[i].len = 0;
+ }
+
+ atomic_long_dec(&ima_htable.len);
+ list_del_rcu(&qe->later);
+ ++cur;
+ if (cur >= number_logs)
+ break;
+ }
+
+ rcu_read_unlock();
+ mutex_unlock(&ima_extend_list_mutex);
+ return cur;
+}
+
int ima_restore_measurement_entry(struct ima_template_entry *entry)
{
int result = 0;
--
2.43.0
^ permalink raw reply related
* [PATCH 0/1] Trim N entries of IMA event logs
From: steven chen @ 2025-12-02 23:28 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring
The Integrity Measurement Architecture (IMA) maintains a measurement list
—a record of system events used for integrity verification. The IMA event
logs are the entries within this measurement list, each representing a
specific event or measurement that contributes to the system's integrity
assessment.
This update introduces the ability to trim, or remove, N entries from the
current measurement list. Trimming involves deleting N entries from the
list and clearing the corresponding entries from the hash table. This
action atomically truncates the measurement list, ensuring that no new
measurements can be added until the operation is complete. Importantly,
only one writer can initiate this trimming process at a time, maintaining
consistency and preventing race conditions.
A userspace interface, ima_trim_log, has been provided for this purpose.
By writing a number N to this interface, userspace can request the kernel
to trim N entries from the IMA event logs. When this interface is read,
it returns the number of entries trimmed during the last operation. This
value is not preserved across kexec soft reboots, as it is not considered
important information.
To maintain a complete record, userspace is responsible for concatenating
and storing the logs before initiating trimming. Userspace can then send
the collected data to remote verifiers for validation. After receiving
confirmation from the remote verifiers, userspace may instruct the kernel
to proceed with trimming the IMA event logs accordingly.
The primary benefit of this solution is the ability to free valuable
kernel memory by delegating the task of reconstructing the full
measurement list from log chunks to userspace. Trust is not required in
userspace for the integrity of the measurement list, as its integrity is
cryptographically protected by the Trusted Platform Module (TPM).
Multiple readers are allowed to access the ima_trim_log interface
concurrently, while only one writer can trigger log trimming at any time.
During trimming, readers do not see the list and cannot access it while
deletion is in progress, ensuring atomicity.
The time required for trimming is minimal, and IMA event logs are briefly
on hold during this process, preventing read or add operations. This short
interruption has no impact on the overall functionality of IMA.
steven chen (1):
IMA event log trimming
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_fs.c | 78 ++++++++++++++++++++++++++++++
security/integrity/ima/ima_queue.c | 42 ++++++++++++++++
3 files changed, 122 insertions(+)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-02 22:07 UTC (permalink / raw)
To: Jonathan McDowell
Cc: linux-integrity, Stefano Garzarella, stable, Peter Huewe,
Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ard Biesheuvel,
linux-kernel, keyrings, linux-security-module
In-Reply-To: <aS9Dv5CKGNOzpsN7@kernel.org>
On Tue, Dec 02, 2025 at 09:53:39PM +0200, Jarkko Sakkinen wrote:
> On Tue, Dec 02, 2025 at 04:26:09PM +0000, Jonathan McDowell wrote:
> > On Mon, Dec 01, 2025 at 09:39:58PM +0200, Jarkko Sakkinen wrote:
> > > 'name_size' does not have any range checks, and it just directly indexes
> > > with TPM_ALG_ID, which could lead into memory corruption at worst.
> > >
> > > Address the issue by only processing known values and returning -EINVAL for
> > > unrecognized values.
> > >
> > > Make also 'tpm_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible so
> > > that errors are detected before causing any spurious TPM traffic.
> > >
> > > End also the authorization session on failure in both of the functions, as
> > > the session state would be then by definition corrupted.
> > >
> > > Cc: stable@vger.kernel.org # v6.10+
> > > Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > > v3:
> > > - Add two missing 'tpm2_end_auth_session' calls to the fallback paths of
> > > 'tpm_buf_fill_hmac_session'.
> > > - Rewrote the commit message.
> > > - End authorization session on failure in 'tpm2_buf_append_name' and
> > > 'tpm_buf_fill_hmac_session'.
> > > v2:
> > > There was spurious extra field added to tpm2_hash by mistake.
> > > ---
> > > drivers/char/tpm/tpm2-cmd.c | 23 +++-
> > > drivers/char/tpm/tpm2-sessions.c | 131 +++++++++++++++-------
> > > include/linux/tpm.h | 6 +-
> > > security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
> > > 4 files changed, 136 insertions(+), 53 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 5b6ccf901623..4473b81122e8 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -187,7 +187,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > > }
> > >
> > > if (!disable_pcr_integrity) {
> > > - tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > + if (rc) {
> > > + tpm_buf_destroy(&buf);
> > > + return rc;
> > > + }
> > > tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > } else {
> > > tpm_buf_append_handle(chip, &buf, pcr_idx);
> > > @@ -202,8 +206,14 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > > chip->allocated_banks[i].digest_size);
> > > }
> > >
> > > - if (!disable_pcr_integrity)
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (!disable_pcr_integrity) {
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc) {
> > > + tpm_buf_destroy(&buf);
> > > + return rc;
> > > + }
> > > + }
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
> > > if (!disable_pcr_integrity)
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > > @@ -261,7 +271,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> > > | TPM2_SA_CONTINUE_SESSION,
> > > NULL, 0);
> > > tpm_buf_append_u16(&buf, num_bytes);
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + err = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (err) {
> > > + tpm_buf_destroy(&buf);
> > > + return err;
> > > + }
> > > +
> > > err = tpm_transmit_cmd(chip, &buf,
> > > offsetof(struct tpm2_get_random_out,
> > > buffer),
> > > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> > > index 6d03c224e6b2..33ad0d668e1a 100644
> > > --- a/drivers/char/tpm/tpm2-sessions.c
> > > +++ b/drivers/char/tpm/tpm2-sessions.c
> > > @@ -144,16 +144,24 @@ struct tpm2_auth {
> > > /*
> > > * Name Size based on TPM algorithm (assumes no hash bigger than 255)
> > > */
> > > -static u8 name_size(const u8 *name)
> > > +static int name_size(const u8 *name)
> > > {
> > > - static u8 size_map[] = {
> > > - [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
> > > - [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
> > > - [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
> > > - [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
> > > - };
> > > - u16 alg = get_unaligned_be16(name);
> > > - return size_map[alg] + 2;
> > > + u16 hash_alg = get_unaligned_be16(name);
> > > +
> > > + switch (hash_alg) {
> > > + case TPM_ALG_SHA1:
> > > + return SHA1_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SHA256:
> > > + return SHA256_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SHA384:
> > > + return SHA384_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SHA512:
> > > + return SHA512_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SM3_256:
> > > + return SM3256_DIGEST_SIZE + 2;
> > > + }
> > > +
> >
> > Can we/should we perhaps print a warning here if we don't know the
> > algorithm?
>
> I think it is a good idea to do that right now.
>
> Also, it'd be better to not have SM3 label as SM2/SM3 is not supported
> at this point. I'm working simulatenously on an improved feature and
> that slipped from that work:
>
> My big picture roadmap for this feature, and how to make it useful is:
>
> 1. tpm.integrity_mode=disabled/permissive/enforced. Permissive means
> here that the feature is conditionally enabled if algorithms that are
> required to enable the HMAC pipe are available.
> 2. tpm.integrity_handle=0x00000000/0x81??????.
>
> >
> > > + return -EINVAL;
> > > }
> > >
> > > static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > > @@ -161,6 +169,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > > struct tpm_header *head = (struct tpm_header *)buf->data;
> > > off_t offset = TPM_HEADER_SIZE;
> > > u32 tot_len = be32_to_cpu(head->length);
> > > + int ret;
> > > u32 val;
> > >
> > > /* we're starting after the header so adjust the length */
> > > @@ -172,9 +181,15 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > > return -EINVAL;
> > > offset += val;
> > > /* name */
> > > +
> > > val = tpm_buf_read_u16(buf, &offset);
> > > - if (val != name_size(&buf->data[offset]))
> > > + ret = name_size(&buf->data[offset]);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (val != ret)
> > > return -EINVAL;
> > > +
> > > memcpy(name, &buf->data[offset], val);
> > > /* forget the rest */
> > > return 0;
> > > @@ -221,46 +236,70 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
> > > * As with most tpm_buf operations, success is assumed because failure
> > > * will be caused by an incorrect programming model and indicated by a
> > > * kernel message.
> > > + *
> > > + * Ends the authorization session on failure.
> > > */
> > > -void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > - u32 handle, u8 *name)
> > > +int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > + u32 handle, u8 *name)
> > > {
> > > #ifdef CONFIG_TCG_TPM2_HMAC
> > > enum tpm2_mso_type mso = tpm2_handle_mso(handle);
> > > struct tpm2_auth *auth;
> > > int slot;
> > > + int ret;
> > > #endif
> > >
> > > if (!tpm2_chip_auth(chip)) {
> > > tpm_buf_append_handle(chip, buf, handle);
> > > - return;
> > > + return 0;
> > > }
> > >
> > > #ifdef CONFIG_TCG_TPM2_HMAC
> > > slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
> > > if (slot >= AUTH_MAX_NAMES) {
> > > - dev_err(&chip->dev, "TPM: too many handles\n");
> > > - return;
> > > + dev_err(&chip->dev, "too many handles\n");
> > > + ret = -EIO;
> > > + goto err;
> > > }
> > > auth = chip->auth;
> > > - WARN(auth->session != tpm_buf_length(buf),
> > > - "name added in wrong place\n");
> > > + if (auth->session != tpm_buf_length(buf)) {
> > > + dev_err(&chip->dev, "session state malformed");
> > > + ret = -EIO;
> > > + goto err;
> > > + }
> > > tpm_buf_append_u32(buf, handle);
> > > auth->session += 4;
> > >
> > > if (mso == TPM2_MSO_PERSISTENT ||
> > > mso == TPM2_MSO_VOLATILE ||
> > > mso == TPM2_MSO_NVRAM) {
> > > - if (!name)
> > > - tpm2_read_public(chip, handle, auth->name[slot]);
> > > + if (!name) {
> > > + ret = tpm2_read_public(chip, handle, auth->name[slot]);
> > > + if (ret)
> > > + goto err;
> > > + }
> > > } else {
> > > - if (name)
> > > - dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
> >
> > We're dropping the error message here; is there a reason for that?
>
> Thanks, I'll add it back.
>
> >
> > > + if (name) {
> > > + ret = -EIO;
> > > + goto err;
> > > + }
> > > }
> > >
> > > auth->name_h[slot] = handle;
> > > - if (name)
> > > - memcpy(auth->name[slot], name, name_size(name));
> > > + if (name) {
> > > + ret = name_size(name);
> > > + if (ret < 0)
> > > + goto err;
> > > +
> > > + memcpy(auth->name[slot], name, ret);
> > > + }
> > > +#endif
> > > + return 0;
> > > +
> > > +#ifdef CONFIG_TCG_TPM2_HMAC
> > > +err:
> > > + tpm2_end_auth_session(chip);
> > > + return tpm_ret_to_err(ret);
> > > #endif
> > > }
> > > EXPORT_SYMBOL_GPL(tpm_buf_append_name);
> > > @@ -533,11 +572,9 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
> > > * encryption key and encrypts the first parameter of the command
> > > * buffer with it.
> > > *
> > > - * As with most tpm_buf operations, success is assumed because failure
> > > - * will be caused by an incorrect programming model and indicated by a
> > > - * kernel message.
> > > + * Ends the authorization session on failure.
> > > */
> > > -void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > +int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > {
> > > u32 cc, handles, val;
> > > struct tpm2_auth *auth = chip->auth;
> > > @@ -549,9 +586,12 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > u8 cphash[SHA256_DIGEST_SIZE];
> > > struct sha256_ctx sctx;
> > > struct hmac_sha256_ctx hctx;
> > > + int ret;
> > >
> > > - if (!auth)
> > > - return;
> > > + if (!auth) {
> > > + ret = -EINVAL;
> > > + goto err;
> > > + }
> > >
> > > /* save the command code in BE format */
> > > auth->ordinal = head->ordinal;
> > > @@ -560,9 +600,10 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > >
> > > i = tpm2_find_cc(chip, cc);
> > > if (i < 0) {
> > > - dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
> >
> > Again, I think it's generally helpful to have the error message given that
> > the return (EINVAL) does not help narrow down which value is bad.
>
> Agreed.
>
> >
> > > - return;
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > > +
> > > attrs = chip->cc_attrs_tbl[i];
> > >
> > > handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
> > > @@ -576,9 +617,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > u32 handle = tpm_buf_read_u32(buf, &offset_s);
> > >
> > > if (auth->name_h[i] != handle) {
> > > - dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
> > > - i);
> > > - return;
> > > + dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > > }
> > > /* point offset_s to the start of the sessions */
> > > @@ -609,12 +650,14 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > offset_s += len;
> > > }
> > > if (offset_s != offset_p) {
> > > - dev_err(&chip->dev, "TPM session length is incorrect\n");
> > > - return;
> > > + dev_err(&chip->dev, "session length is incorrect\n");
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > > if (!hmac) {
> > > - dev_err(&chip->dev, "TPM could not find HMAC session\n");
> > > - return;
> > > + dev_err(&chip->dev, "could not find HMAC session\n");
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > >
> > > /* encrypt before HMAC */
> > > @@ -646,8 +689,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > if (mso == TPM2_MSO_PERSISTENT ||
> > > mso == TPM2_MSO_VOLATILE ||
> > > mso == TPM2_MSO_NVRAM) {
> > > - sha256_update(&sctx, auth->name[i],
> > > - name_size(auth->name[i]));
> > > + ret = name_size(auth->name[i]);
> > > + if (ret < 0)
> > > + goto err;
> > > +
> > > + sha256_update(&sctx, auth->name[i], ret);
> > > } else {
> > > __be32 h = cpu_to_be32(auth->name_h[i]);
> > >
> > > @@ -668,6 +714,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> > > hmac_sha256_update(&hctx, &auth->attrs, 1);
> > > hmac_sha256_final(&hctx, hmac);
> > > + return 0;
> > > +
> > > +err:
> > > + tpm2_end_auth_session(chip);
> > > + return ret;
> > > }
> > > EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
> > >
> > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > index 0e9e043f728c..1a59f0190eb3 100644
> > > --- a/include/linux/tpm.h
> > > +++ b/include/linux/tpm.h
> > > @@ -528,8 +528,8 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
> > > #endif
> > > }
> > >
> > > -void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > - u32 handle, u8 *name);
> > > +int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > + u32 handle, u8 *name);
> > > void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
> > > u8 attributes, u8 *passphrase,
> > > int passphraselen);
> > > @@ -562,7 +562,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
> > > #ifdef CONFIG_TCG_TPM2_HMAC
> > >
> > > int tpm2_start_auth_session(struct tpm_chip *chip);
> > > -void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
> > > +int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
> > > int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
> > > int rc);
> > > void tpm2_end_auth_session(struct tpm_chip *chip);
> > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> > > index e165b117bbca..7672a4376dad 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > > @@ -283,7 +283,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> > > goto out_put;
> > > }
> > >
> > > - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + if (rc)
> > > + goto out;
> > > +
> > > tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> > > options->keyauth, TPM_DIGEST_SIZE);
> > >
> > > @@ -331,7 +334,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> > > goto out;
> > > }
> > >
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc)
> > > + goto out;
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > > if (rc)
> > > @@ -438,7 +444,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > > return rc;
> > > }
> > >
> > > - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + if (rc)
> > > + goto out;
> > > +
> > > tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
> > > TPM_DIGEST_SIZE);
> > >
> > > @@ -450,7 +459,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > > goto out;
> > > }
> > >
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc)
> > > + goto out;
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > > if (!rc)
> > > @@ -497,7 +509,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> > > return rc;
> > > }
> > >
> > > - tpm_buf_append_name(chip, &buf, blob_handle, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + if (rc)
> > > + goto out;
> > >
> > > if (!options->policyhandle) {
> > > tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
> > > @@ -522,7 +536,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> > > NULL, 0);
> > > }
> > >
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc)
> > > + goto out;
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > >
> > > --
> > > 2.52.0
> > >
> >
> > J.
> >
> > --
> > Be Ye Not Lost Among Precepts of Order
>
> BR, Jarkko
Addressed in: https://lore.kernel.org/linux-integrity/20251202202643.107108-2-jarkko@kernel.org/
BR, Jarkko
^ permalink raw reply
* [GIT PULL] integrity: subsystem updates for v6.19
From: Mimi Zohar @ 2025-12-02 21:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-integrity, linux-kernel, Roberto Sassu
Hi Linus,
There are two bug fixes: defer credentials checking from the bprm_check_security
hook to the bprm_creds_from_file security hook, properly ignore IMA policy rules
based on undefined SELinux labels.
And two IMA policy rule extensions: extend IMA to limit including file hashes in
the audit logs (dont_audit action), define a new filesystem subtype policy
option (fs_subtype).
And extend IMA to support commit b1ae6dc41eaa ("module: add in-kernel support
for decompressing") by deferring the IMA signature verification in
kernel_read_file() to after the kernel module is decompressed.
thanks,
Mimi
The following changes since commit 3a8660878839faadb4f1a6dd72c3179c1df56787:
Linux 6.18-rc1 (2025-10-12 13:42:36 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/ tags/integrity-v6.19
for you to fetch changes up to 738c9738e690f5cea24a3ad6fd2d9a323cf614f6:
ima: Handle error code returned by ima_filter_rule_match() (2025-11-21 07:24:01 -0500)
----------------------------------------------------------------
integrity-v6.19
----------------------------------------------------------------
Coiby Xu (1):
ima: Access decompressed kernel module to verify appended signature
Jann Horn (2):
ima: add dont_audit action to suppress audit actions
ima: add fs_subtype condition for distinguishing FUSE instances
Roberto Sassu (1):
ima: Attach CREDS_CHECK IMA hook to bprm_creds_from_file LSM hook
Zhao Yipeng (1):
ima: Handle error code returned by ima_filter_rule_match()
Documentation/ABI/testing/ima_policy | 3 +-
include/linux/kernel_read_file.h | 1 +
kernel/module/main.c | 17 ++++++++--
security/integrity/ima/ima_main.c | 62 +++++++++++++++++++++++++++---------
security/integrity/ima/ima_policy.c | 62 ++++++++++++++++++++++++++++++++----
security/ipe/hooks.c | 1 +
security/selinux/hooks.c | 5 +--
7 files changed, 123 insertions(+), 28 deletions(-)
^ permalink raw reply
* [PATCH v2 4/4] tpm2-sessions: Open code tpm_buf_append_hmac_session()
From: Jarkko Sakkinen @ 2025-12-02 20:26 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, Jarkko Sakkinen, Jonathan McDowell, James Bottomley,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251202202643.107108-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Open code 'tpm_buf_append_hmac_session_opt' to the call site, as it only
masks a call sequence and does otherwise nothing particularly useful.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
---
drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++---
include/linux/tpm.h | 23 -----------------------
security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
3 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index f1e9c35f13a2..94cb887cb4a9 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -270,9 +270,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
do {
tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
- | TPM2_SA_CONTINUE_SESSION,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf,
+ TPM2_SA_ENCRYPT |
+ TPM2_SA_CONTINUE_SESSION,
+ NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
tpm_buf_append_u16(&buf, num_bytes);
err = tpm_buf_fill_hmac_session(chip, &buf);
if (err) {
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index e3a6e2fb41a7..a93df0efebe4 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -535,29 +535,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
u8 *passphrase, int passphraselen);
-static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
- struct tpm_buf *buf,
- u8 attributes,
- u8 *passphrase,
- int passphraselen)
-{
- struct tpm_header *head;
- int offset;
-
- if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
- } else {
- offset = buf->handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf->data;
-
- /*
- * If the only sessions are optional, the command tag must change to
- * TPM2_ST_NO_SESSIONS.
- */
- if (tpm_buf_length(buf) == offset)
- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
- }
-}
#ifdef CONFIG_TCG_TPM2_HMAC
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 7672a4376dad..e6b95111ac7d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -494,8 +494,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle)
{
+ struct tpm_header *head;
struct tpm_buf buf;
u16 data_len;
+ int offset;
u8 *data;
int rc;
@@ -532,8 +534,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm2_buf_append_auth(&buf, options->policyhandle,
NULL /* nonce */, 0, 0,
options->blobauth, options->blobauth_len);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
}
rc = tpm_buf_fill_hmac_session(chip, &buf);
--
2.52.0
^ permalink raw reply related
* [PATCH v2 3/4] tpm2-sessions: Remove 'attributes' parameter from tpm_buf_append_auth
From: Jarkko Sakkinen @ 2025-12-02 20:26 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, Jarkko Sakkinen, Jonathan McDowell, Roberto Sassu,
Mimi Zohar
In-Reply-To: <20251202202643.107108-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Remove 'attributes' parameter from 'tpm_buf_append_auth', as it is not used
by the function.
Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
---
drivers/char/tpm/tpm2-cmd.c | 2 +-
drivers/char/tpm/tpm2-sessions.c | 5 ++---
include/linux/tpm.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 58a8477cda85..f1e9c35f13a2 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -198,7 +198,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
} else {
tpm_buf_append_handle(chip, &buf, pcr_idx);
- tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
+ tpm_buf_append_auth(chip, &buf, NULL, 0);
}
tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 964f44c2255d..7c7a9bc6ba2d 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -311,7 +311,7 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
- u8 attributes, u8 *passphrase, int passphrase_len)
+ u8 *passphrase, int passphrase_len)
{
/* offset tells us where the sessions area begins */
int offset = buf->handles * 4 + TPM_HEADER_SIZE;
@@ -372,8 +372,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
#endif
if (!tpm2_chip_auth(chip)) {
- tpm_buf_append_auth(chip, buf, attributes, passphrase,
- passphrase_len);
+ tpm_buf_append_auth(chip, buf, passphrase, passphrase_len);
return;
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 1a59f0190eb3..e3a6e2fb41a7 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -534,7 +534,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
- u8 attributes, u8 *passphrase, int passphraselen);
+ u8 *passphrase, int passphraselen);
static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
struct tpm_buf *buf,
u8 attributes,
--
2.52.0
^ permalink raw reply related
* [PATCH v2 2/4] tpm2-sessions: Fix tpm2_read_public range checks
From: Jarkko Sakkinen @ 2025-12-02 20:26 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, stable, James Bottomley, Ard Biesheuvel
In-Reply-To: <20251202202643.107108-1-jarkko@kernel.org>
'tpm2_read_public' has some rudimentary range checks but the function
does not ensure that the response buffer has enough bytes for the full
TPMT_HA payload.
Re-implement the function with necessary checks and validation.
Cc: stable@vger.kernel.org # v6.10+
Fixes: d0a25bb961e6 ("tpm: Add HMAC session name/handle append")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm2-cmd.c | 3 ++
drivers/char/tpm/tpm2-sessions.c | 77 +++++++++++++++++---------------
2 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 4473b81122e8..58a8477cda85 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -11,8 +11,11 @@
* used by the kernel internally.
*/
+#include "linux/dev_printk.h"
+#include "linux/tpm.h"
#include "tpm.h"
#include <crypto/hash_info.h>
+#include <linux/unaligned.h>
static bool disable_pcr_integrity;
module_param(disable_pcr_integrity, bool, 0444);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index dc6bee7e1ef6..964f44c2255d 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -163,54 +163,59 @@ static int name_size(const u8 *name)
}
}
-static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
+static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
{
- struct tpm_header *head = (struct tpm_header *)buf->data;
+ u32 mso = tpm2_handle_mso(handle);
off_t offset = TPM_HEADER_SIZE;
- u32 tot_len = be32_to_cpu(head->length);
- int ret;
- u32 val;
-
- /* we're starting after the header so adjust the length */
- tot_len -= TPM_HEADER_SIZE;
-
- /* skip public */
- val = tpm_buf_read_u16(buf, &offset);
- if (val > tot_len)
- return -EINVAL;
- offset += val;
- /* name */
-
- val = tpm_buf_read_u16(buf, &offset);
- ret = name_size(&buf->data[offset]);
- if (ret < 0)
- return ret;
+ struct tpm_buf buf;
+ int rc, rc2;
- if (val != ret)
+ if (mso != TPM2_MSO_PERSISTENT && mso != TPM2_MSO_VOLATILE &&
+ mso != TPM2_MSO_NVRAM)
return -EINVAL;
- memcpy(name, &buf->data[offset], val);
- /* forget the rest */
- return 0;
-}
-
-static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
-{
- struct tpm_buf buf;
- int rc;
-
rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
if (rc)
return rc;
tpm_buf_append_u32(&buf, handle);
- rc = tpm_transmit_cmd(chip, &buf, 0, "read public");
- if (rc == TPM2_RC_SUCCESS)
- rc = tpm2_parse_read_public(name, &buf);
- tpm_buf_destroy(&buf);
+ rc = tpm_transmit_cmd(chip, &buf, 0, "TPM2_ReadPublic");
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return tpm_ret_to_err(rc);
+ }
- return rc;
+ /* Skip TPMT_PUBLIC: */
+ offset += tpm_buf_read_u16(&buf, &offset);
+
+ /*
+ * Ensure space for the length field of TPM2B_NAME and hashAlg field of
+ * TPMT_HA (the extra four bytes).
+ */
+ if (offset + 4 > tpm_buf_length(&buf)) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ rc = tpm_buf_read_u16(&buf, &offset);
+ rc2 = name_size(&buf.data[offset]);
+
+ if (rc2 < 0)
+ return rc2;
+
+ if (rc != rc2) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ if (offset + rc > tpm_buf_length(&buf)) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ memcpy(name, &buf.data[offset], rc);
+ return 0;
}
#endif /* CONFIG_TCG_TPM2_HMAC */
--
2.52.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox