* [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
* [PATCH v2 1/4] tpm2-sessions: fix out of range indexing in name_size
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, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ard Biesheuvel,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251202202643.107108-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>
---
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 | 6 +-
security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
4 files changed, 138 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..dc6bee7e1ef6 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: invalid name algorithm: 0x%04\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 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
^ permalink raw reply related
* [PATCH v2 0/4] tpm2-sessions: Fixes aime for v6.19-rc2
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
I collected the accumulated fixed for tpm2-sessions, geared towards
v6.19-rc2.
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 | 31 +---
security/keys/trusted-keys/trusted_tpm2.c | 41 ++++-
4 files changed, 199 insertions(+), 114 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-02 19:53 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: <aS8TIeviaippVAha@earth.li>
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
^ permalink raw reply
* Re: [PATCH] x86/kexec: Add a sanity check on previous kernel's ima kexec buffer
From: Mimi Zohar @ 2025-12-02 19:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Harshit Mogalapalli, henry.willard, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Andrew Morton,
Mike Rapoport (Microsoft), Jiri Bohac, Sourabh Jain, Guo Weikang,
Joel Granados, Alexander Graf, Sohil Mehta, Jonathan McDowell,
linux-kernel, yifei.l.liu, stable, Paul Webb, Roberto Sassu,
linux-integrity
In-Reply-To: <CAMj1kXFDAypcEAAFw=O6pS5zD5aujXUvo3_95p_2fJiESsSmgQ@mail.gmail.com>
[Cc: Roberto Sassu, linux-integrity]
On Tue, 2025-12-02 at 08:16 +0100, Ard Biesheuvel wrote:
> On Mon, 1 Dec 2025 at 22:43, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Mon, 2025-12-01 at 15:03 +0530, Harshit Mogalapalli wrote:
> > > Hi all,
> > >
> > > On 13/11/25 01:00, Harshit Mogalapalli wrote:
> > > > When the second-stage kernel is booted via kexec with a limiting command
> > > > line such as "mem=<size>", the physical range that contains the carried
> > > > over IMA measurement list may fall outside the truncated RAM leading to
> > > > a kernel panic.
> > > >
> > > > BUG: unable to handle page fault for address: ffff97793ff47000
> > > > RIP: ima_restore_measurement_list+0xdc/0x45a
> > > > #PF: error_code(0x0000) – not-present page
> > > >
> > > > Other architectures already validate the range with page_is_ram(), as
> > > > done in commit: cbf9c4b9617b ("of: check previous kernel's
> > > > ima-kexec-buffer against memory bounds") do a similar check on x86.
> >
> > It should be obvious that without carrying the measurement list across kexec,
> > that attestation will fail. Please mention it here in the patch description.
> >
>
> Couldn't we just use memremap() and be done with it? That will use the
> direct map if the memory is mapped, or vmap() it otherwise.
No, the IMA measurement list is not a continuous buffer, but a linked list of
records with varying types of fields and field sizes. The call to
ima_dump_measurement_list() marshals the measurement list into a buffer, while
ima_restore_measurement_list() unmarshals it.
--
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature
From: Mimi Zohar @ 2025-12-02 17:10 UTC (permalink / raw)
To: Coiby Xu, linux-integrity
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list,
open list:SECURITY SUBSYSTEM
In-Reply-To: <7jzx432acnp7mrma7om5ccvrc3ucvm5psscst62bcl5t5yklh7@yxhcvmrgbgli>
On Mon, 2025-12-01 at 11:15 +0800, Coiby Xu wrote:
> Hi Mimi,
>
> I think this patch set just fell off the radar. Can you take a look at
> it when time permits? Thanks! Btw, the patch set is still applicable to
> current next-integrity tree Linus and main tree.
Hi Coiby. I haven't forgotten about this patch, but would prefer upstreaming it
together with another EVM change. I'm really sorry about the delay.
--
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jonathan McDowell @ 2025-12-02 16:26 UTC (permalink / raw)
To: Jarkko Sakkinen
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: <20251201193958.896358-1-jarkko@kernel.org>
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?
>+ 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?
>+ 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.
>- 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
^ permalink raw reply
* [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-6.19-rc1-v3
From: Jarkko Sakkinen @ 2025-12-01 23:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Huewe, Jason Gunthorpe, David Howells, keyrings,
linux-integrity, linux-kernel
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(-)
^ permalink raw reply
* Re: [PATCH 0/4] tpm2-sessions: Fixes for v6.19
From: Jarkko Sakkinen @ 2025-12-01 23:27 UTC (permalink / raw)
To: linux-integrity; +Cc: Peter Huewe, Jason Gunthorpe, open list
In-Reply-To: <20251201224554.1717104-1-jarkko@kernel.org>
On Tue, Dec 02, 2025 at 12:45:48AM +0200, Jarkko Sakkinen wrote:
> I collected the accumulated fixed for tpm2-sessions, given that I need
> to still redo TPM driver pull request.
>
> Jarkko Sakkinen (4):
> tpm2-sessions: address out-of-range indexing
> 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 | 197 ++++++++++++++--------
> include/linux/tpm.h | 31 +---
> security/keys/trusted-keys/trusted_tpm2.c | 41 ++++-
> 4 files changed, 197 insertions(+), 114 deletions(-)
>
> --
> 2.52.0
>
Not putting to the first pull request tho but hopefully get these merged
during the cycle.
BR, Jarkko
^ permalink raw reply
* [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, stable, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, Ard Biesheuvel, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251201224554.1717104-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:
- 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;
+ }
+
+ 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");
+ 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);
- 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
^ permalink raw reply related
* [PATCH 4/4] tpm2-sessions: Open code tpm_buf_append_hmac_session()
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jarkko Sakkinen,
Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251201224554.1717104-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 3/4] tpm2-sessions: Remove 'attributes' parameter from tpm_buf_append_auth
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jarkko Sakkinen,
Jason Gunthorpe, Mimi Zohar, Roberto Sassu, open list
In-Reply-To: <20251201224554.1717104-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 afbca03f639e..04ec4eb394d9 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -310,7 +310,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;
@@ -371,8 +371,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 2/4] tpm2-sessions: Fix tpm2_read_public range checks
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, stable, Peter Huewe, Jason Gunthorpe,
James Bottomley, Ard Biesheuvel, open list
In-Reply-To: <20251201224554.1717104-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 33ad0d668e1a..afbca03f639e 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -164,54 +164,59 @@ static int name_size(const u8 *name)
return -EINVAL;
}
-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 1/4] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, stable, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, Ard Biesheuvel, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251201224554.1717104-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>
---
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;
+ }
+
+ 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");
+ 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);
- 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
^ permalink raw reply related
* [PATCH 0/4] tpm2-sessions: Fixes for v6.19
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity; +Cc: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, open list
I collected the accumulated fixed for tpm2-sessions, given that I need
to still redo TPM driver pull request.
Jarkko Sakkinen (4):
tpm2-sessions: address out-of-range indexing
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 | 197 ++++++++++++++--------
include/linux/tpm.h | 31 +---
security/keys/trusted-keys/trusted_tpm2.c | 41 ++++-
4 files changed, 197 insertions(+), 114 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: Are setuid shell scripts safe? (Implied by security_bprm_creds_for_exec)
From: David Laight @ 2025-12-01 21:39 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Roberto Sassu, Bernd Edlinger, 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 Mon, 01 Dec 2025 12:53:10 -0600
"Eric W. Biederman" <ebiederm@xmission.com> wrote:
> Roberto Sassu <roberto.sassu@huaweicloud.com> writes:
...
> 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 thought that was what was done - and stopped the problem of a user
flipping a symlink between a suid script and one the user had written.
It has only ever been done for suid scripts when the uid actually changes.
Which makes it possible to set the permissions so that owner can't
run the script!
(The kernel only needs 'x' access, the shell needs 'r' access, so with 'x+s'
the owner can't execute the script but everyone else can.)
There is a much older problem that probably only affected the original 1970s
'sh' (not even the SVSV/Sunos version) that quoted redirects on the command
line would get actioned when the parameter was substituted - which I think
means the original 'sh' did post-substitution syntax analysis (the same
as cmd.exe still does).
That doesn't affect any shells used since the early 1980s.
David
^ permalink raw reply
* [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-01 19:39 UTC (permalink / raw)
To: linux-integrity
Cc: Jonathan McDowell, Stefano Garzarella, Jarkko Sakkinen, 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
'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;
+ }
+
+ 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");
+ 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);
- 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
^ permalink raw reply related
* Re: Are setuid shell scripts safe? (Implied by security_bprm_creds_for_exec)
From: Eric W. Biederman @ 2025-12-01 18:53 UTC (permalink / raw)
To: Roberto Sassu
Cc: Bernd Edlinger, 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: <dca0f01500f9d6705dccf3b3ef616468b1f53f57.camel@huaweicloud.com>
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.
Eric
^ permalink raw reply
* Re: [RFC v1 0/1] Implement IMA Event Log Trimming
From: steven chen @ 2025-12-01 17:42 UTC (permalink / raw)
To: Roberto Sassu, Gregory Lumen
Cc: Anirudh Venkataramanan, linux-integrity, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E . Hallyn, linux-security-module,
Lakshmi Ramasubramanian, Sush Shringarputale
In-Reply-To: <3f85e98e2e4ef6a0de4fe4f6c2093791def1e30b.camel@huaweicloud.com>
On 11/27/2025 1:45 AM, Roberto Sassu wrote:
> On Wed, 2025-11-26 at 15:40 -0800, Gregory Lumen wrote:
>> Greetings Roberto,
>>
>> If I may chime in a bit:
>>
>>> The only way to make the verification of measurements list snapshots
>>> work is that the verification state is stored outside the system to
>>> evaluate (which can be assumed to be trusted), so that you are sure
>>> that the system is not advancing the PCR starting value by itself.
>> You are correct; to make the described approach work, an external source
>> of trust is required in order to detect unexpected or unauthorized
>> trimming of the event log (for example, by signing the trim-to PCR values
>> from the previous verification/attestation cycle). This should be true
>> regardless of the mechanism of trimming. More generally, I will go so far
>> as to suggest that any attempt to attest the integrity of a system using
>> IMA will likely fall into one of two general approaches: either the entire
>> IMA event log is retained (either in kernel or user space) from boot and
>> claims of system integrity are built by validating and examining the
>> entire log for signs of tampering, or an external source of trust is
>> introduced to allow incremental validation and examination of the log.
>> Other more innovative approaches may exist, but we make no such claims.
>>
>> I will also say that it should be possible to implement either approach to
>> attestation (retaining the entire log, or relying on an external source of
>> trust) with any sane implementation for IMA log trimming.
>>
>> As for our proposed implementation, storing the starting PCR values in the
>> kernel preserving the ability for any arbitrary user space entity to
>> validate the retained portion of the IMA event log against the TPM PCRs at
>> any time, without requiring awareness of other user space mechanisms
>> implemented by other entities that may be initiating IMA trimming
>> operations. My personal sense is that this capability is worth preserving,
>> but it is entirely possible the general consensus is that the value
>> offered does not balance against the additional technical complexity when
>> compared to simpler alternatives (discussed in a moment). To stress the
>> point, this capability would only enable validation of the integrity of
>> the retained portion of the event log and its continuity with the PCRs,
>> and could not be used to make any claims as to the overall integrity of
>> the system since, as you observed, an attacker who has successfully
>> compromised the system could simply trim the event log in order to discard
>> evidence of the compromise.
> Hi Gregory
>
> all you said can be implemented by maintaining the PCR starting value
> outside the system, in a trusted entity. This would allow the
> functionality you are hoping for to validate the retained portion of
> the measurement list.
>
> Keeping the PCR starting value in the kernel has the potential of
> misleading users that this is an information they can rely on. I would
> rather prefer to not run in such risk.
>
>> If the ability to validate the retained portion of the IMA event log is
>> not worth designing for, we could instead go with a simpler "Trim-to-N"
>> approach, where the user space interface allows for the specification of
>> an absolute index into the IMA log to be used as the trim position (as
>> opposed to using calculated PCR values to indicate trim position in our
>> current proposal). To protect against unexpected behavior in the event of
> >From implementation point of view, it looks much simpler to me to
> specify N relative to the current measurement list.
Hi Roberto,
I will send "trim N entries" patch out this week.
Regards,
Steven
>> concurrent trims, index counting would need to be fixed (hence absolute)
>> such that index 0 would always refer to the very first entry written
>> during boot, even if that entry has already been trimmed, with the number
>> of trimmed entries (and thus starting index of the retained log) exposed
>> to use space via a pseudo-file.
> In my draft patch [1] (still need to support trimming N entries instead
> of the full measurement list), the risk of concurrent trims does not
> exist because opening of the snapshot interface is exclusive (no one
> else can request trimming concurrently).
>
> If a more elaborated contention of remote attestation agent is
> required, that could be done at user space level. I'm hoping to keep in
> the kernel only the minimum code necessary for the remote attestation
> to work.
>
> Roberto
>
> [1] https://github.com/robertosassu/linux/commit/b0bd002b6caa9d5d4f4d0db2a041b1fd91f33f8a
>
>> With such a trim approach, it should be possible to implement either
>> general attestation approach: retaining the entire log (copy the log to
>> user space, then trim the copied entries), or relying on an external
>> source of trust (quote, determine the log index corresponding to the quote
>> plus PCRs, trim, then securely store the trim position/starting PCRs for
>> future cycles).
>>
>> -Gregory Lumen
^ permalink raw reply
* [PATCH] tpm2-sessions: Fix tpm2_read_public range checks
From: Jarkko Sakkinen @ 2025-12-01 16:55 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, stable, Peter Huewe, Jason Gunthorpe,
James Bottomley, Ard Biesheuvel, linux-kernel
'tpm2_read_public' has some rudimentary range checks but does not
explicitly check that both fields of TPMT_HA actually fit within the buffer
size limits.
Introduce a new function 'tpm2_resolve_name' to address all the possible
out-of-range issues, and in addition do handle type 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 | 95 ++++++++++++++++++++++++++++++++
drivers/char/tpm/tpm2-sessions.c | 78 +-------------------------
include/linux/tpm.h | 2 +
3 files changed, 98 insertions(+), 77 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index e63254135a74..d51272573004 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);
@@ -769,3 +772,95 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
return -1;
}
+
+/**
+ * tpm2_name_size() - Resolve size of a TPMT_HA instance
+ * @name: Pointer to TPMT_HA structure extracted from TPM2B_NAME.
+ *
+ * Calculate size of the TPMT_HA payload of TPM2B_NAME. It is used with
+ * transient keys, persistent and NV indexes.
+ *
+ * Returns zero when the hash size was successfully calculated.
+ * Returns -EINVAL when the hash algorithm was not recognized.
+ */
+int tpm2_name_size(const u8 *name)
+{
+ 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;
+ }
+
+ return -EINVAL;
+}
+
+/**
+ * tpm2_resolve_name - Resolve TPM object's name from the public area
+ * @handle: Persistent, transient or nv handle.
+ *
+ * Returns zero on success.
+ * Returns -EINVAL when handles are not of valid type.
+ * Returns -EIO if the transmission fails or response is malformed.
+ */
+int tpm2_resolve_name(struct tpm_chip *chip, u32 handle, void *name)
+{
+ u32 mso = tpm2_handle_mso(handle);
+ off_t offset = TPM_HEADER_SIZE;
+ int name_size, name_size_2;
+ struct tpm_buf buf;
+ int rc;
+
+ if (mso != TPM2_MSO_PERSISTENT && mso != TPM2_MSO_VOLATILE &&
+ mso != TPM2_MSO_NVRAM)
+ return -EINVAL;
+
+ 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, "TPM2_ReadPublic");
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return tpm_ret_to_err(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;
+ }
+
+ name_size = tpm_buf_read_u16(&buf, &offset);
+ name_size_2 = tpm2_name_size(&buf.data[offset]);
+
+ if (name_size != name_size_2) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ if (offset + name_size > tpm_buf_length(&buf)) {
+ tpm_buf_destroy(&buf);
+ return -EIO;
+ }
+
+ memcpy(name, &buf.data[offset], name_size);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tpm2_resolve_name);
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 82b9d9096fd1..7c85333d47c4 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -140,82 +140,6 @@ struct tpm2_auth {
u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
};
-#ifdef CONFIG_TCG_TPM2_HMAC
-
-/*
- * Calculate size of the TPMT_HA payload of TPM2B_NAME.
- */
-static int tpm2_name_size(const u8 *name)
-{
- 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;
- }
-
- return -EINVAL;
-}
-
-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 name_size_alg;
- 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);
- name_size_alg = tpm2_name_size(&buf->data[offset]);
- if (name_size_alg < 0)
- return name_size_alg;
-
- if (val != name_size_alg)
- 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);
-
- return rc;
-}
-#endif /* CONFIG_TCG_TPM2_HMAC */
-
/**
* tpm_buf_append_name() - add a handle area to the buffer
* @chip: the TPM chip structure
@@ -272,7 +196,7 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
mso == TPM2_MSO_VOLATILE ||
mso == TPM2_MSO_NVRAM) {
if (!name) {
- ret = tpm2_read_public(chip, handle, auth->name[slot]);
+ ret = tpm2_resolve_name(chip, handle, auth->name[slot]);
if (ret)
return tpm_ret_to_err(ret);
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 1a59f0190eb3..727e6c26feeb 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -477,6 +477,8 @@ extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
extern struct tpm_chip *tpm_default_chip(void);
void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
+int tpm2_name_size(const u8 *name);
+int tpm2_resolve_name(struct tpm_chip *chip, u32 handle, void *name);
static inline void tpm_buf_append_empty_auth(struct tpm_buf *buf, u32 handle)
{
--
2.52.0
^ permalink raw reply related
* Re: Are setuid shell scripts safe? (Implied by security_bprm_creds_for_exec)
From: Roberto Sassu @ 2025-12-01 16:49 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Bernd Edlinger, 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: <87v7iqtcev.fsf_-_@email.froward.int.ebiederm.org>
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 :)
> > 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...
> 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!
> 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.
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.
> 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.
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!
Roberto
^ permalink raw reply
* Are setuid shell scripts safe? (Implied by security_bprm_creds_for_exec)
From: Eric W. Biederman @ 2025-12-01 16:06 UTC (permalink / raw)
To: Roberto Sassu
Cc: Bernd Edlinger, 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: <6dc556a0a93c18fffec71322bf97441c74b3134e.camel@huaweicloud.com>
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?
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.
If the patch you posted for review works that helps sort that mess out.
> 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.
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.
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.
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.
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?
Eric
^ permalink raw reply
* Re: [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature
From: Coiby Xu @ 2025-12-01 3:15 UTC (permalink / raw)
To: linux-integrity, Mimi Zohar
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20250930022658.4033410-1-coxu@redhat.com>
On Tue, Sep 30, 2025 at 10:26:56AM +0800, Coiby Xu wrote:
>When both IMA and EVM fix modes are enabled, accessing a file with IMA
>signature but missing EVM HMAC won't cause security.evm to be fixed.
>
>Add a function evm_fix_hmac which will be explicitly called to fix EVM
>HMAC for this case.
>
>Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>Signed-off-by: Coiby Xu <coxu@redhat.com>
>---
> include/linux/evm.h | 8 ++++++++
> security/integrity/evm/evm_main.c | 28 +++++++++++++++++++++++++++
> security/integrity/ima/ima_appraise.c | 5 +++++
> 3 files changed, 41 insertions(+)
>
>diff --git a/include/linux/evm.h b/include/linux/evm.h
>index ddece4a6b25d..913f4573b203 100644
>--- a/include/linux/evm.h
>+++ b/include/linux/evm.h
>@@ -18,6 +18,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
> const char *xattr_name,
> void *xattr_value,
> size_t xattr_value_len);
>+int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
>+ const char *xattr_value, size_t xattr_value_len);
> int evm_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr, struct xattr *xattrs,
> int *xattr_count);
>@@ -51,6 +53,12 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
> {
> return INTEGRITY_UNKNOWN;
> }
>+
>+static inline int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
>+ const char *xattr_value, size_t xattr_value_len)
>+{
>+ return -EOPNOTSUPP;
>+}
> #endif
>
> static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
>diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>index 0add782e73ba..1b3edc6d26e9 100644
>--- a/security/integrity/evm/evm_main.c
>+++ b/security/integrity/evm/evm_main.c
>@@ -787,6 +787,34 @@ bool evm_revalidate_status(const char *xattr_name)
> return true;
> }
>
>+/**
>+ * evm_fix_hmac - Calculate the HMAC and add it to security.evm for fix mode
>+ * @dentry: pointer to the affected dentry which doesn't yet have security.evm
>+ * xattr
>+ * @xattr_name: pointer to the affected extended attribute name
>+ * @xattr_value: pointer to the new extended attribute value
>+ * @xattr_value_len: pointer to the new extended attribute value length
>+ *
>+ * Expects to be called with i_mutex locked.
>+ *
>+ * Return: 0 on success, -EPERM/-ENOMEM/-EOPNOTSUPP on failure
>+ */
>+int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
>+ const char *xattr_value, size_t xattr_value_len)
>+
>+{
>+ if (!evm_fixmode || !evm_revalidate_status((xattr_name)))
>+ return -EPERM;
>+
>+ if (!(evm_initialized & EVM_INIT_HMAC))
>+ return -EPERM;
>+
>+ if (is_unsupported_hmac_fs(dentry))
>+ return -EOPNOTSUPP;
>+
>+ return evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
>+}
>+
> /**
> * evm_inode_post_setxattr - update 'security.evm' to reflect the changes
> * @dentry: pointer to the affected dentry
>diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>index f435eff4667f..f48ef5ec185e 100644
>--- a/security/integrity/ima/ima_appraise.c
>+++ b/security/integrity/ima/ima_appraise.c
>@@ -601,6 +601,11 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
> xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
> if (!ima_fix_xattr(dentry, iint))
> status = INTEGRITY_PASS;
>+ } else if (status == INTEGRITY_NOLABEL) {
>+ if (!evm_fix_hmac(dentry, XATTR_NAME_IMA,
>+ (const char *)xattr_value,
>+ xattr_len))
>+ status = INTEGRITY_PASS;
> }
>
> /*
>
>base-commit: e129e479f2e444eaccd822717d418119d39d3d5c
>--
>2.51.0
>
Hi Mimi,
I think this patch set just fell off the radar. Can you take a look at
it when time permits? Thanks! Btw, the patch set is still applicable to
current next-integrity tree Linus and main tree.
--
Best regards,
Coiby
^ permalink raw reply
* [PATCH v2] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
From: Coiby Xu @ 2025-12-01 3:06 UTC (permalink / raw)
To: linux-modules
Cc: linux-integrity, kernel test robot, Aaron Tomlin, Daniel Gomez,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
open list:MODULE SUPPORT
In-Reply-To: <20251031080949.2001716-1-coxu@redhat.com>
Currently if set_module_sig_enforced is called with CONFIG_MODULE_SIG=n
e.g. [1], it can lead to a linking error,
ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
This happens because the actual implementation of
set_module_sig_enforced comes from CONFIG_MODULE_SIG but both the
function declaration and the empty stub definition are tied to
CONFIG_MODULES.
So bind set_module_sig_enforced to CONFIG_MODULE_SIG instead. This
allows (future) users to call set_module_sig_enforced directly without
the "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
Note this issue hasn't caused a real problem because all current callers
of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
use "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
[1] https://lore.kernel.org/lkml/20250928030358.3873311-1-coxu@redhat.com/
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
v2
- improve commit message as suggested by Daniel
- add Reviewed-by tags from Aaron and Daniel
include/linux/module.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index e135cc79acee..fa251958b138 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -769,8 +769,6 @@ static inline bool is_livepatch_module(struct module *mod)
#endif
}
-void set_module_sig_enforced(void);
-
void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data);
#else /* !CONFIG_MODULES... */
@@ -865,10 +863,6 @@ static inline bool module_requested_async_probing(struct module *module)
}
-static inline void set_module_sig_enforced(void)
-{
-}
-
/* Dereference module function descriptor */
static inline
void *dereference_module_function_descriptor(struct module *mod, void *ptr)
@@ -924,6 +918,8 @@ static inline bool retpoline_module_ok(bool has_retpoline)
#ifdef CONFIG_MODULE_SIG
bool is_module_sig_enforced(void);
+void set_module_sig_enforced(void);
+
static inline bool module_sig_ok(struct module *module)
{
return module->sig_ok;
@@ -934,6 +930,10 @@ static inline bool is_module_sig_enforced(void)
return false;
}
+static inline void set_module_sig_enforced(void)
+{
+}
+
static inline bool module_sig_ok(struct module *module)
{
return true;
base-commit: ac3fd01e4c1efce8f2c054cdeb2ddd2fc0fb150d
--
2.52.0
^ permalink raw reply related
* [PATCH v2] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-11-30 21:35 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, 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
'name_size' does not have any range checks, and it just directly indexes
with TPM_ALG_ID.
Address the issue by:
1. Rename 'name_size' as 'tpm2_name_size' so that it is bit easier to
recognize and make it a fallible function.
2. Check for only known algorithms in 'tpm2_name_size'. Return -EINVAL for
unrecognized algorithms.
3. In order to correctly propagate possible errors make also
'tpm2_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible and
address their possible errors at the call sites.
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>
---
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 | 108 ++++++++++++++--------
include/linux/tpm.h | 6 +-
security/keys/trusted-keys/trusted_tpm2.c | 38 ++++++--
4 files changed, 124 insertions(+), 51 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 5b6ccf901623..e63254135a74 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -187,7 +187,12 @@ 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) {
+ tpm2_end_auth_session(chip);
+ return rc;
+ }
+
tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
} else {
tpm_buf_append_handle(chip, &buf, pcr_idx);
@@ -202,8 +207,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) {
+ tpm2_end_auth_session(chip);
+ 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 +272,11 @@ 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)
+ goto out;
+
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..82b9d9096fd1 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -141,19 +141,28 @@ struct tpm2_auth {
};
#ifdef CONFIG_TCG_TPM2_HMAC
+
/*
- * Name Size based on TPM algorithm (assumes no hash bigger than 255)
+ * Calculate size of the TPMT_HA payload of TPM2B_NAME.
*/
-static u8 name_size(const u8 *name)
+static int tpm2_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;
+ }
+
+ return -EINVAL;
}
static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
@@ -161,6 +170,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 name_size_alg;
u32 val;
/* we're starting after the header so adjust the length */
@@ -172,9 +182,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]))
+ name_size_alg = tpm2_name_size(&buf->data[offset]);
+ if (name_size_alg < 0)
+ return name_size_alg;
+
+ if (val != name_size_alg)
return -EINVAL;
+
memcpy(name, &buf->data[offset], val);
/* forget the rest */
return 0;
@@ -222,46 +238,59 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
* will be caused by an incorrect programming model and indicated by a
* kernel message.
*/
-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 name_size;
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");
+ return -ENOMEM;
}
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");
+ return -EIO;
+ }
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)
+ return tpm_ret_to_err(ret);
+ }
} else {
if (name)
- dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
+ return -EINVAL;
}
auth->name_h[slot] = handle;
- if (name)
- memcpy(auth->name[slot], name, name_size(name));
+ if (name) {
+ name_size = tpm2_name_size(name);
+ if (name_size < 0)
+ return name_size;
+
+ memcpy(auth->name[slot], name, name_size);
+ }
#endif
+ return 0;
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -537,7 +566,7 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
* will be caused by an incorrect programming model and indicated by a
* kernel message.
*/
-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 +578,10 @@ 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 name_size;
if (!auth)
- return;
+ return -EINVAL;
/* save the command code in BE format */
auth->ordinal = head->ordinal;
@@ -559,10 +589,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
cc = be32_to_cpu(head->ordinal);
i = tpm2_find_cc(chip, cc);
- if (i < 0) {
- dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
- return;
- }
+ if (i < 0)
+ return -EINVAL;
+
attrs = chip->cc_attrs_tbl[i];
handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
@@ -576,9 +605,8 @@ 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);
+ return -EINVAL;
}
}
/* point offset_s to the start of the sessions */
@@ -609,12 +637,12 @@ 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");
+ return -EINVAL;
}
if (!hmac) {
- dev_err(&chip->dev, "TPM could not find HMAC session\n");
- return;
+ dev_err(&chip->dev, "could not find HMAC session\n");
+ return -EINVAL;
}
/* encrypt before HMAC */
@@ -646,8 +674,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]));
+ name_size = tpm2_name_size(auth->name[i]);
+ if (name_size < 0)
+ return name_size;
+
+ sha256_update(&sctx, auth->name[i], name_size);
} else {
__be32 h = cpu_to_be32(auth->name_h[i]);
@@ -668,6 +699,7 @@ 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;
}
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..33544b6bc105 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -283,7 +283,13 @@ 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) {
+ tpm_buf_destroy(&buf);
+ tpm2_end_auth_session(chip);
+ goto out_put;
+ }
+
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
options->keyauth, TPM_DIGEST_SIZE);
@@ -331,7 +337,12 @@ 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) {
+ tpm2_end_auth_session(chip);
+ goto out;
+ }
+
rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (rc)
@@ -438,7 +449,12 @@ 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) {
+ tpm2_end_auth_session(chip);
+ return rc;
+ }
+
tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
TPM_DIGEST_SIZE);
@@ -450,7 +466,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 +516,11 @@ 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) {
+ tpm2_end_auth_session(chip);
+ return rc;
+ }
if (!options->policyhandle) {
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
@@ -522,7 +545,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
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