* [PATCH v7 09/12] tpm: Change tpm_get_random() opportunistic
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David S . Miller, Herbert Xu, Peter Huewe,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
hwrng framework does not have a requirement that the all bytes requested
need to be provided. By enforcing such a requirement internally, TPM driver
can cause unpredictability in latency, as a single tpm_get_random() call
can result multiple TPM commands.
Especially, when TCG_TPM2_HMAC is enabled, extra roundtrips could have
significant effect to the system latency.
Thus, send TPM command only once and return bytes received instead of
committing to the number of requested bytes.
Cc: David S. Miller <davem@davemloft.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v7:
- Given that hwrng is now only caller for tpm_get_random(), remove the
wait parameter.
v4:
- Fixed grammar mistakes.
---
drivers/char/tpm/tpm-interface.c | 28 +++++-----------------------
1 file changed, 5 insertions(+), 23 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d157be738612..677dcef05dfb 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -626,10 +626,6 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
*/
int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
{
- u32 num_bytes = max;
- u8 *out_ptr = out;
- int retries = 5;
- int total = 0;
int rc;
if (!out || !max || max > TPM_MAX_RNG_DATA)
@@ -646,28 +642,14 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
rc = tpm2_start_auth_session(chip);
if (rc)
return rc;
- }
-
- do {
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- rc = tpm2_get_random(chip, out_ptr, num_bytes);
- else
- rc = tpm1_get_random(chip, out_ptr, num_bytes);
-
- if (rc < 0)
- goto err;
-
- out_ptr += rc;
- total += rc;
- num_bytes -= rc;
- } while (retries-- && total < max);
- tpm_put_ops(chip);
- return total ? total : -EIO;
+ rc = tpm2_get_random(chip, out, max);
+ } else {
+ rc = tpm1_get_random(chip, out, max);
+ }
-err:
tpm_put_ops(chip);
- return rc;
+ return rc != 0 ? rc : -EIO;
}
EXPORT_SYMBOL_GPL(tpm_get_random);
--
2.39.5
^ permalink raw reply related
* [PATCH v7 08/12] tpm: Orchestrate TPM commands in tpm_get_random()
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
tpm1_get_random() and tpm2_get_random() contain duplicate orchestration
code. Consolidate orchestration to tpm_get_random().
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm-interface.c | 175 +++++++++++++++++++++++++++++--
drivers/char/tpm/tpm.h | 2 -
drivers/char/tpm/tpm1-cmd.c | 69 ------------
drivers/char/tpm/tpm2-cmd.c | 104 ------------------
4 files changed, 164 insertions(+), 186 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f745a098908b..d157be738612 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -26,7 +26,7 @@
#include <linux/suspend.h>
#include <linux/freezer.h>
#include <linux/tpm_eventlog.h>
-
+#include <linux/tpm_command.h>
#include "tpm.h"
/*
@@ -486,19 +486,153 @@ int tpm_pm_resume(struct device *dev)
}
EXPORT_SYMBOL_GPL(tpm_pm_resume);
+struct tpm1_get_random_out {
+ __be32 rng_data_len;
+ u8 rng_data[TPM_MAX_RNG_DATA];
+} __packed;
+
+static int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+{
+ struct tpm1_get_random_out *resp;
+ struct tpm_buf buf;
+ u32 recd;
+ int rc;
+
+ if (!out || !max || max > TPM_MAX_RNG_DATA)
+ return -EINVAL;
+
+ rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GETRANDOM);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(&buf, max);
+
+ rc = tpm_transmit_cmd(chip, &buf, sizeof(resp->rng_data_len), "TPM_GetRandom");
+ if (rc) {
+ if (rc > 0)
+ rc = -EIO;
+ goto err;
+ }
+
+ resp = (struct tpm1_get_random_out *)&buf.data[TPM_HEADER_SIZE];
+
+ recd = be32_to_cpu(resp->rng_data_len);
+ if (recd > max) {
+ rc = -EIO;
+ goto err;
+ }
+
+ if (buf.length < TPM_HEADER_SIZE + sizeof(resp->rng_data_len) + recd) {
+ rc = -EIO;
+ goto err;
+ }
+
+ memcpy(out, resp->rng_data, recd);
+ tpm_buf_destroy(&buf);
+ return recd;
+
+err:
+ tpm_buf_destroy(&buf);
+ return rc;
+}
+
+struct tpm2_get_random_out {
+ __be16 size;
+ u8 buffer[TPM_MAX_RNG_DATA];
+} __packed;
+
+static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+{
+ struct tpm2_get_random_out *resp;
+ struct tpm_header *head;
+ struct tpm_buf buf;
+ off_t offset;
+ u32 recd;
+ int ret;
+
+ if (!out || !max || max > TPM_MAX_RNG_DATA)
+ return -EINVAL;
+
+ ret = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+ if (ret)
+ return ret;
+
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf,
+ TPM2_SA_ENCRYPT | TPM2_SA_CONTINUE_SESSION,
+ NULL, 0);
+ } else {
+ head = (struct tpm_header *)buf.data;
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
+ tpm_buf_append_u16(&buf, max);
+
+ ret = tpm_buf_fill_hmac_session(chip, &buf);
+ if (ret) {
+ tpm_buf_destroy(&buf);
+ return ret;
+ }
+
+ ret = tpm_transmit_cmd(chip, &buf, offsetof(struct tpm2_get_random_out, buffer),
+ "TPM2_GetRandom");
+
+ ret = tpm_buf_check_hmac_response(chip, &buf, ret);
+ if (ret) {
+ if (ret > 0)
+ ret = -EIO;
+
+ goto out;
+ }
+
+ head = (struct tpm_header *)buf.data;
+ offset = TPM_HEADER_SIZE;
+
+ /* Skip the parameter size field: */
+ if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
+ offset += 4;
+
+ resp = (struct tpm2_get_random_out *)&buf.data[offset];
+ recd = min_t(u32, be16_to_cpu(resp->size), max);
+
+ if (tpm_buf_length(&buf) <
+ TPM_HEADER_SIZE + offsetof(struct tpm2_get_random_out, buffer) + recd) {
+ ret = -EIO;
+ goto out;
+ }
+
+ memcpy(out, resp->buffer, recd);
+ return recd;
+
+out:
+ tpm2_end_auth_session(chip);
+ tpm_buf_destroy(&buf);
+ return ret;
+}
+
/**
- * tpm_get_random() - get random bytes from the TPM's RNG
- * @chip: a &struct tpm_chip instance, %NULL for the default chip
- * @out: destination buffer for the random bytes
- * @max: the max number of bytes to write to @out
+ * tpm_get_random() - Get random bytes from the TPM's RNG
+ * @chip: A &tpm_chip instance. Whenset to %NULL, the default chip is used.
+ * @out: Destination buffer for the acquired random bytes.
+ * @max: The maximum number of bytes to write to @out.
+ *
+ * Iterates pulling more bytes from TPM up until all of the @max bytes have been
+ * received.
*
- * Return: number of random bytes read or a negative error value.
+ * Returns the number of random bytes read on success.
+ * Returns -EINVAL when @out is NULL, or @max is not between zero and
+ * %TPM_MAX_RNG_DATA.
+ * Returns tpm_transmit_cmd() error codes when the TPM command results an
+ * error.
*/
int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
{
+ u32 num_bytes = max;
+ u8 *out_ptr = out;
+ int retries = 5;
+ int total = 0;
int rc;
- if (!out || max > TPM_MAX_RNG_DATA)
+ if (!out || !max || max > TPM_MAX_RNG_DATA)
return -EINVAL;
if (!chip)
@@ -508,11 +642,30 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
if (rc)
return rc;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- rc = tpm2_get_random(chip, out, max);
- else
- rc = tpm1_get_random(chip, out, max);
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ rc = tpm2_start_auth_session(chip);
+ if (rc)
+ return rc;
+ }
+
+ do {
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ rc = tpm2_get_random(chip, out_ptr, num_bytes);
+ else
+ rc = tpm1_get_random(chip, out_ptr, num_bytes);
+
+ if (rc < 0)
+ goto err;
+
+ out_ptr += rc;
+ total += rc;
+ num_bytes -= rc;
+ } while (retries-- && total < max);
+
+ tpm_put_ops(chip);
+ return total ? total : -EIO;
+err:
tpm_put_ops(chip);
return rc;
}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 02c07fef41ba..f698d01401de 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -251,7 +251,6 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc, size_t min_cap_length);
-int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
int tpm1_get_pcr_allocation(struct tpm_chip *chip);
unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm_pm_suspend(struct device *dev);
@@ -291,7 +290,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digests);
-int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
u32 *value, const char *desc);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index b49a790f1bd5..0604e11c9778 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -511,75 +511,6 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
}
EXPORT_SYMBOL_GPL(tpm1_getcap);
-#define TPM_ORD_GET_RANDOM 70
-struct tpm1_get_random_out {
- __be32 rng_data_len;
- u8 rng_data[TPM_MAX_RNG_DATA];
-} __packed;
-
-/**
- * tpm1_get_random() - get random bytes from the TPM's RNG
- * @chip: a &struct tpm_chip instance
- * @dest: destination buffer for the random bytes
- * @max: the maximum number of bytes to write to @dest
- *
- * Return:
- * * number of bytes read
- * * -errno (positive TPM return codes are masked to -EIO)
- */
-int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
-{
- struct tpm1_get_random_out *out;
- u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
- struct tpm_buf buf;
- u32 total = 0;
- int retries = 5;
- u32 recd;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
- if (rc)
- return rc;
-
- do {
- tpm_buf_append_u32(&buf, num_bytes);
-
- rc = tpm_transmit_cmd(chip, &buf, sizeof(out->rng_data_len),
- "attempting get random");
- if (rc) {
- if (rc > 0)
- rc = -EIO;
- goto out;
- }
-
- out = (struct tpm1_get_random_out *)&buf.data[TPM_HEADER_SIZE];
-
- recd = be32_to_cpu(out->rng_data_len);
- if (recd > num_bytes) {
- rc = -EFAULT;
- goto out;
- }
-
- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE +
- sizeof(out->rng_data_len) + recd) {
- rc = -EFAULT;
- goto out;
- }
- memcpy(dest, out->rng_data, recd);
-
- dest += recd;
- total += recd;
- num_bytes -= recd;
-
- tpm_buf_reset(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
- } while (retries-- && total < max);
-
- rc = total ? (int)total : -EIO;
-out:
- tpm_buf_destroy(&buf);
- return rc;
-}
-
#define TPM_ORD_PCRREAD 21
int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
{
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 1f561ad3bdcf..461e85c3abe5 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -239,110 +239,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
return rc;
}
-struct tpm2_get_random_out {
- __be16 size;
- u8 buffer[TPM_MAX_RNG_DATA];
-} __packed;
-
-/**
- * tpm2_get_random() - get random bytes from the TPM RNG
- *
- * @chip: a &tpm_chip instance
- * @dest: destination buffer
- * @max: the max number of random bytes to pull
- *
- * Return:
- * size of the buffer on success,
- * -errno otherwise (positive TPM return codes are masked to -EIO)
- */
-int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
-{
- struct tpm2_get_random_out *out;
- struct tpm_header *head;
- struct tpm_buf buf;
- u32 recd;
- u32 num_bytes = max;
- int err;
- int total = 0;
- int retries = 5;
- u8 *dest_ptr = dest;
- off_t offset;
-
- if (!num_bytes || max > TPM_MAX_RNG_DATA)
- return -EINVAL;
-
- err = tpm2_start_auth_session(chip);
- if (err)
- return err;
-
- err = tpm_buf_init(&buf, 0, 0);
- if (err) {
- tpm2_end_auth_session(chip);
- return err;
- }
-
- do {
- tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- 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) {
- tpm_buf_destroy(&buf);
- return err;
- }
-
- err = tpm_transmit_cmd(chip, &buf,
- offsetof(struct tpm2_get_random_out,
- buffer),
- "attempting get random");
- err = tpm_buf_check_hmac_response(chip, &buf, err);
- if (err) {
- if (err > 0)
- err = -EIO;
- goto out;
- }
-
- head = (struct tpm_header *)buf.data;
- offset = TPM_HEADER_SIZE;
- /* Skip the parameter size field: */
- if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
- offset += 4;
-
- out = (struct tpm2_get_random_out *)&buf.data[offset];
- recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
- if (tpm_buf_length(&buf) <
- TPM_HEADER_SIZE +
- offsetof(struct tpm2_get_random_out, buffer) +
- recd) {
- err = -EFAULT;
- goto out;
- }
- memcpy(dest_ptr, out->buffer, recd);
-
- dest_ptr += recd;
- total += recd;
- num_bytes -= recd;
- } while (retries-- && total < max);
-
- tpm_buf_destroy(&buf);
-
- return total ? total : -EIO;
-out:
- tpm_buf_destroy(&buf);
- tpm2_end_auth_session(chip);
- return err;
-}
-
/**
* tpm2_flush_context() - execute a TPM2_FlushContext command
* @chip: TPM chip to use
--
2.39.5
^ permalink raw reply related
* [PATCH v7 07/12] tpm2-sessions: Remove AUTH_MAX_NAMES
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
In all of the call sites only one session is ever append. Thus, reduce
AUTH_MAX_NAMES, which leads into removing constant completely.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm2-sessions.c | 31 +++++++++++--------------------
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 3bc3c31cf512..37570dc088cf 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -72,9 +72,6 @@
#include <crypto/sha2.h>
#include <crypto/utils.h>
-/* maximum number of names the TPM must remember for authorization */
-#define AUTH_MAX_NAMES 3
-
#define AES_KEY_BYTES AES_KEYSIZE_128
#define AES_KEY_BITS (AES_KEY_BYTES*8)
@@ -136,8 +133,8 @@ struct tpm2_auth {
* handle, but they are part of the session by name, which
* we must compute and remember
*/
- u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
- u16 name_size_tbl[AUTH_MAX_NAMES];
+ u8 name[TPM2_MAX_NAME_SIZE];
+ u16 name_size;
};
#ifdef CONFIG_TCG_TPM2_HMAC
@@ -261,11 +258,14 @@ EXPORT_SYMBOL_GPL(tpm2_read_public);
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
u32 handle, u8 *name, u16 name_size)
{
-#ifdef CONFIG_TCG_TPM2_HMAC
struct tpm2_auth *auth;
- int slot;
int ret;
-#endif
+
+ if (tpm_buf_length(buf) != TPM_HEADER_SIZE) {
+ dev_err(&chip->dev, "too many handles\n");
+ ret = -EIO;
+ goto err;
+ }
if (!tpm2_chip_auth(chip)) {
tpm_buf_append_handle(chip, buf, handle);
@@ -273,12 +273,6 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
}
#ifdef CONFIG_TCG_TPM2_HMAC
- slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
- if (slot >= AUTH_MAX_NAMES) {
- dev_err(&chip->dev, "too many handles\n");
- ret = -EIO;
- goto err;
- }
auth = chip->auth;
if (auth->session != tpm_buf_length(buf)) {
dev_err(&chip->dev, "session state malformed");
@@ -287,16 +281,14 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
}
tpm_buf_append_u32(buf, handle);
auth->session += 4;
- memcpy(auth->name[slot], name, name_size);
- auth->name_size_tbl[slot] = name_size;
+ memcpy(auth->name, name, name_size);
+ auth->name_size = name_size;
#endif
return 0;
-#ifdef CONFIG_TCG_TPM2_HMAC
err:
tpm2_end_auth_session(chip);
return ret;
-#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -665,8 +657,7 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
/* ordinal is already BE */
sha256_update(&sctx, (u8 *)&head->ordinal, sizeof(head->ordinal));
/* add the handle names */
- for (i = 0; i < handles; i++)
- sha256_update(&sctx, auth->name[i], auth->name_size_tbl[i]);
+ sha256_update(&sctx, auth->name, auth->name_size);
if (offset_s != tpm_buf_length(buf))
sha256_update(&sctx, &buf->data[offset_s],
tpm_buf_length(buf) - offset_s);
--
2.39.5
^ permalink raw reply related
* [PATCH v7 06/12] KEYS: trusted: Re-orchestrate tpm2_read_public() calls
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, James Bottomley,
Mimi Zohar, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
tpm2_load_cmd() and tpm2_unseal_cmd() use the same parent, and calls to
tpm_buf_append_name() cause the exact same TPM2_ReadPublic command to be
sent to the chip, causing unnecessary traffic.
1. Export tpm2_read_public in order to make it callable from
'trusted_tpm2'.
2. Re-orchestrate tpm2_seal_trusted() and tpm2_unseal_trusted() in order to
halve the name resolutions required:
2a. Move tpm2_read_public() calls into trusted_tpm2.
2b. Pass TPM name to tpm_buf_append_name().
2c. Rework tpm_buf_append_name() to use the pre-resolved name.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm2-cmd.c | 3 +-
drivers/char/tpm/tpm2-sessions.c | 95 +++++------------
include/linux/tpm.h | 10 +-
security/keys/trusted-keys/trusted_tpm2.c | 124 ++++++++++++++--------
4 files changed, 118 insertions(+), 114 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 3a77be7ebf4a..1f561ad3bdcf 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -202,7 +202,8 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
}
if (!disable_pcr_integrity) {
- rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+ rc = tpm_buf_append_name(chip, &buf, pcr_idx, (u8 *)&pcr_idx,
+ sizeof(u32));
if (rc) {
tpm_buf_destroy(&buf);
return rc;
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 525b8622d1c3..3bc3c31cf512 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -136,8 +136,8 @@ struct tpm2_auth {
* handle, but they are part of the session by name, which
* we must compute and remember
*/
- u32 name_h[AUTH_MAX_NAMES];
u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
+ u16 name_size_tbl[AUTH_MAX_NAMES];
};
#ifdef CONFIG_TCG_TPM2_HMAC
@@ -163,7 +163,17 @@ static int name_size(const u8 *name)
}
}
-static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
+/**
+ * tpm2_read_public: Resolve TPM name for a handle
+ * @chip: TPM chip to use.
+ * @handle: TPM handle.
+ * @name: A buffer for returning the name blob. Must have a
+ * capacity of 'SHA512_DIGET_SIZE + 2' bytes at minimum
+ *
+ * Returns size of TPM handle name of success.
+ * Returns tpm_transmit_cmd error codes when TPM2_ReadPublic fails.
+ */
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
{
u32 mso = tpm2_handle_mso(handle);
off_t offset = TPM_HEADER_SIZE;
@@ -219,14 +229,16 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
memcpy(name, &buf.data[offset], rc);
return name_size_alg;
}
+EXPORT_SYMBOL_GPL(tpm2_read_public);
#endif /* CONFIG_TCG_TPM2_HMAC */
/**
- * tpm_buf_append_name() - add a handle area to the buffer
- * @chip: the TPM chip structure
- * @buf: The buffer to be appended
- * @handle: The handle to be appended
- * @name: The name of the handle (may be NULL)
+ * tpm_buf_append_name() - Append a handle and store TPM name
+ * @chip: TPM chip to use.
+ * @buf: TPM buffer containing the TPM command in-transit.
+ * @handle: TPM handle to be appended.
+ * @name: TPM name of the handle
+ * @name_size: Size of the TPM name.
*
* In order to compute session HMACs, we need to know the names of the
* objects pointed to by the handles. For most objects, this is simply
@@ -243,15 +255,14 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
* will be caused by an incorrect programming model and indicated by a
* kernel message.
*
- * Ends the authorization session on failure.
+ * Returns zero on success.
+ * Returns -EIO when the authorization area state is malformed.
*/
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name)
+ u32 handle, u8 *name, u16 name_size)
{
#ifdef CONFIG_TCG_TPM2_HMAC
- enum tpm2_mso_type mso = tpm2_handle_mso(handle);
struct tpm2_auth *auth;
- u16 name_size_alg;
int slot;
int ret;
#endif
@@ -276,36 +287,15 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
}
tpm_buf_append_u32(buf, handle);
auth->session += 4;
-
- if (mso == TPM2_MSO_PERSISTENT ||
- mso == TPM2_MSO_VOLATILE ||
- mso == TPM2_MSO_NVRAM) {
- if (!name) {
- ret = tpm2_read_public(chip, handle, auth->name[slot]);
- if (ret < 0)
- goto err;
-
- name_size_alg = ret;
- }
- } else {
- 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_alg);
+ memcpy(auth->name[slot], name, name_size);
+ auth->name_size_tbl[slot] = name_size;
#endif
return 0;
#ifdef CONFIG_TCG_TPM2_HMAC
err:
tpm2_end_auth_session(chip);
- return tpm_ret_to_err(ret);
+ return ret;
#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -613,22 +603,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
attrs = chip->cc_attrs_tbl[i];
handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+ offset_s += handles * sizeof(u32);
- /*
- * just check the names, it's easy to make mistakes. This
- * would happen if someone added a handle via
- * tpm_buf_append_u32() instead of tpm_buf_append_name()
- */
- for (i = 0; i < handles; i++) {
- u32 handle = tpm_buf_read_u32(buf, &offset_s);
-
- if (auth->name_h[i] != handle) {
- dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
- ret = -EIO;
- goto err;
- }
- }
- /* point offset_s to the start of the sessions */
val = tpm_buf_read_u32(buf, &offset_s);
/* point offset_p to the start of the parameters */
offset_p = offset_s + val;
@@ -689,23 +665,8 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
/* ordinal is already BE */
sha256_update(&sctx, (u8 *)&head->ordinal, sizeof(head->ordinal));
/* add the handle names */
- for (i = 0; i < handles; i++) {
- enum tpm2_mso_type mso = tpm2_handle_mso(auth->name_h[i]);
-
- if (mso == TPM2_MSO_PERSISTENT ||
- mso == TPM2_MSO_VOLATILE ||
- mso == TPM2_MSO_NVRAM) {
- 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]);
-
- sha256_update(&sctx, (u8 *)&h, 4);
- }
- }
+ for (i = 0; i < handles; i++)
+ sha256_update(&sctx, auth->name[i], auth->name_size_tbl[i]);
if (offset_s != tpm_buf_length(buf))
sha256_update(&sctx, &buf->data[offset_s],
tpm_buf_length(buf) - offset_s);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index e10f2096eae7..72610f1aa402 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -543,7 +543,7 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
}
int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name);
+ u32 handle, u8 *name, u16 name_size);
void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
@@ -557,6 +557,7 @@ 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);
+int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name);
#else
#include <linux/unaligned.h>
@@ -580,6 +581,13 @@ static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
{
return rc;
}
+
+static inline int tpm2_read_public(struct tpm_chip *chip, u32 handle,
+ void *name)
+{
+ memcpy(name, &handle, sizeof(u32));
+ return sizeof(u32);
+}
#endif /* CONFIG_TCG_TPM2_HMAC */
#endif
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 3666e3e48eab..3de84b30b655 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -203,8 +203,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
+ u8 parent_name[TPM2_MAX_NAME_SIZE];
off_t offset = TPM_HEADER_SIZE;
struct tpm_buf buf, sized;
+ u16 parent_name_size;
int blob_len = 0;
int hash;
u32 flags;
@@ -221,6 +223,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (rc)
return rc;
+ rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+ if (rc < 0)
+ goto out_put;
+
+ parent_name_size = rc;
+
rc = tpm2_start_auth_session(chip);
if (rc)
goto out_put;
@@ -238,7 +246,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out_put;
}
- rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
goto out;
@@ -325,21 +334,25 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
}
/**
- * tpm2_load_cmd() - execute a TPM2_Load command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: returned blob handle
+ * tpm2_load_cmd() - Execute TPM2_Load
+ * @chip: TPM chip to use.
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
+ * @parent_name: A cryptographic name, i.e. a TPMT_HA blob, of the
+ * parent key.
+ * @blob: The decoded payload for the key.
+ * @blob_handle: On success, will contain handle to the loaded keyedhash
+ * blob.
*
- * Return: 0 on success.
- * -E2BIG on wrong payload size.
- * -EPERM on tpm error status.
- * < 0 error from tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
*/
static int tpm2_load_cmd(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options,
+ u8 *parent_name,
+ u16 parent_name_size,
+ const u8 *blob,
u32 *blob_handle)
{
u8 *blob_ref __free(kfree) = NULL;
@@ -347,27 +360,13 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
- u8 *blob, *pub;
+ const u8 *pub;
int rc;
u32 attrs;
- rc = tpm2_key_decode(payload, options, &blob);
- if (rc) {
- /* old form */
- blob = payload->blob;
- payload->old_format = 1;
- } else {
- /* Bind for cleanup: */
- blob_ref = blob;
- }
-
- /* new format carries keyhandle but old format doesn't */
- if (!options->keyhandle)
- return -EINVAL;
-
/* must be big enough for at least the two be16 size counts */
if (payload->blob_len < 4)
- return -EINVAL;
+ return -E2BIG;
private_len = get_unaligned_be16(blob);
@@ -403,7 +402,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
return rc;
}
- rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
goto out;
@@ -435,20 +435,23 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
}
/**
- * tpm2_unseal_cmd() - execute a TPM2_Unload command
+ * tpm2_unseal_cmd() - Execute TPM2_Unload
*
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: blob handle
+ * @chip: TPM chip to use
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
+ * @parent_name: A cryptographic name, i.e. a TPMT_HA blob, of the
+ * parent key.
+ * @blob_handle: Handle to the loaded keyedhash blob.
*
- * Return: 0 on success
- * -EPERM on tpm error status
- * < 0 error from tpm_send
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load fails.
*/
static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options,
+ u8 *parent_name,
+ u16 parent_name_size,
u32 blob_handle)
{
struct tpm_buf buf;
@@ -466,7 +469,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
return rc;
}
- rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+ parent_name_size);
if (rc)
goto out;
@@ -539,30 +543,60 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
}
/**
- * tpm2_unseal_trusted() - unseal the payload of a trusted key
+ * tpm2_unseal_trusted() - Unseal a trusted key
+ * @chip: TPM chip to use.
+ * @payload: Key data in clear text.
+ * @options: Trusted key options.
*
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: Same as with tpm_send.
+ * Return -E2BIG when the blob size is too small for all the data.
+ * Return -EINVAL when parent's key handle has not been set.
+ * Returns tpm_transmit_cmd() error codes when either TPM2_Load or TPM2_Unseal
+ * fails.
*/
int tpm2_unseal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
+ u8 *blob_ref __free(kfree) = NULL;
+ u8 parent_name[TPM2_MAX_NAME_SIZE];
+ u16 parent_name_size;
u32 blob_handle;
+ u8 *blob;
int rc;
+ /*
+ * Try to decode the provided blob as an ASN.1 blob. Assume that the
+ * blob is in the legacy format if decoding does not end successfully.
+ */
+ rc = tpm2_key_decode(payload, options, &blob);
+ if (rc) {
+ blob = payload->blob;
+ payload->old_format = 1;
+ } else {
+ blob_ref = blob;
+ }
+
+ if (!options->keyhandle)
+ return -EINVAL;
+
rc = tpm_try_get_ops(chip);
if (rc)
return rc;
- rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+ rc = tpm2_read_public(chip, options->keyhandle, parent_name);
+ if (rc < 0)
+ goto out;
+
+ parent_name_size = rc;
+
+ rc = tpm2_load_cmd(chip, payload, options, parent_name,
+ parent_name_size, blob, &blob_handle);
if (rc)
goto out;
- rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+ rc = tpm2_unseal_cmd(chip, payload, options, parent_name,
+ parent_name_size, blob_handle);
+
tpm2_flush_context(chip, blob_handle);
out:
--
2.39.5
^ permalink raw reply related
* [PATCH v7 05/12] KEYS: trusted: Remove dead branch from tpm2_unseal_cmd
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
TPM2_Unseal requires TPM2_ST_SESSIONS, and tpm2_unseal_cmd() always does
set up either password or HMAC session.
Remove the branch in tpm2_unseal_cmd() conditionally setting
TPM2_ST_NO_SESSIONS. It is faulty but luckily it is never exercised at
run-time, and thus does not cause regressions.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
security/keys/trusted-keys/trusted_tpm2.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index d3a5c5f2b926..3666e3e48eab 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -451,10 +451,8 @@ 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;
@@ -495,14 +493,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm_buf_append_u16(&buf, options->blobauth_len);
tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
- if (tpm2_chip_auth(chip)) {
+ 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.39.5
^ permalink raw reply related
* [PATCH v7 04/12] KEYS: trusted: Open code tpm2_buf_append()
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, David Howells,
Jarkko Sakkinen, Paul Moore, James Morris, Serge E. Hallyn,
James Bottomley, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
tpm2_buf_append_auth() has a single call site and most of its parameters
are redundant. Open code it to the call site so that less cross-referencing
is required while browsing the source code.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@earth.li>
---
v6:
- Trimmed the patch by removing comment update as it is out of scope.
---
security/keys/trusted-keys/trusted_tpm2.c | 40 ++++-------------------
1 file changed, 7 insertions(+), 33 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a7ea4a1c3bed..d3a5c5f2b926 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -190,36 +190,6 @@ int tpm2_key_priv(void *context, size_t hdrlen,
return 0;
}
-/**
- * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
- *
- * @buf: an allocated tpm_buf instance
- * @session_handle: session handle
- * @nonce: the session nonce, may be NULL if not used
- * @nonce_len: the session nonce length, may be 0 if not used
- * @attributes: the session attributes
- * @hmac: the session HMAC or password, may be NULL if not used
- * @hmac_len: the session HMAC or password length, maybe 0 if not used
- */
-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
- const u8 *nonce, u16 nonce_len,
- u8 attributes,
- const u8 *hmac, u16 hmac_len)
-{
- tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
- tpm_buf_append_u32(buf, session_handle);
- tpm_buf_append_u16(buf, nonce_len);
-
- if (nonce && nonce_len)
- tpm_buf_append(buf, nonce, nonce_len);
-
- tpm_buf_append_u8(buf, attributes);
- tpm_buf_append_u16(buf, hmac_len);
-
- if (hmac && hmac_len)
- tpm_buf_append(buf, hmac, hmac_len);
-}
-
/**
* tpm2_seal_trusted() - seal the payload of a trusted key
*
@@ -518,9 +488,13 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
* could repeat our actions with the exfiltrated
* password.
*/
- tpm2_buf_append_auth(&buf, options->policyhandle,
- NULL /* nonce */, 0, 0,
- options->blobauth, options->blobauth_len);
+ tpm_buf_append_u32(&buf, 9 + options->blobauth_len);
+ tpm_buf_append_u32(&buf, options->policyhandle);
+ tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u8(&buf, 0);
+ tpm_buf_append_u16(&buf, options->blobauth_len);
+ tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
+
if (tpm2_chip_auth(chip)) {
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
} else {
--
2.39.5
^ permalink raw reply related
* [PATCH v7 03/12] tpm2-sessions: Define TPM2_NAME_MAX_SIZE
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
Define TPM2_NAME_MAX_SIZE, which describes the maximum size for hashes
encoded as TPMT_HA, which the prime identifier used for persistent and
transient keys in TPM2 protocol.
Set its value to 'SHA512_DIGEST_SIZE + 2', as SHA512 has the largest
digest size of the algorithms in TCG algorithm repository.
In additionl, rename TPM2_NAME_SIZE as TPM2_NULL_NAME_SIZE in order to
avoid any possible confusion.
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v6:
- Rewrote the commit message.
v2:
- Rename TPM2_NAME_SIZE as TPM2_NULL_NAME_SIZE.
---
drivers/char/tpm/tpm-sysfs.c | 2 +-
drivers/char/tpm/tpm2-sessions.c | 2 +-
include/linux/tpm.h | 37 +++++++++++++++++++++-----------
3 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 94231f052ea7..4a6a27ee295d 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -314,7 +314,7 @@ static ssize_t null_name_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct tpm_chip *chip = to_tpm_chip(dev);
- int size = TPM2_NAME_SIZE;
+ int size = TPM2_NULL_NAME_SIZE;
bin2hex(buf, chip->null_key_name, size);
size *= 2;
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 4149379665c4..525b8622d1c3 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -137,7 +137,7 @@ struct tpm2_auth {
* we must compute and remember
*/
u32 name_h[AUTH_MAX_NAMES];
- u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
+ u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
};
#ifdef CONFIG_TCG_TPM2_HMAC
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 202da079d500..e10f2096eae7 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -27,9 +27,33 @@
#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
+/*
+ * SHA-512 is, as of today, the largest digest in the TCG algorithm repository.
+ */
#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
+
+/*
+ * A TPM name digest i.e., TPMT_HA, is a concatenation of TPM_ALG_ID of the
+ * name algorithm and hash of TPMT_PUBLIC.
+ */
+#define TPM2_MAX_NAME_SIZE (TPM2_MAX_DIGEST_SIZE + 2)
+
+/*
+ * The maximum number of PCR banks.
+ */
#define TPM2_MAX_PCR_BANKS 8
+/*
+ * fixed define for the size of a name. This is actually HASHALG size
+ * plus 2, so 32 for SHA256
+ */
+#define TPM2_NULL_NAME_SIZE 34
+
+/*
+ * The maximum size for an object context
+ */
+#define TPM2_MAX_CONTEXT_SIZE 4096
+
struct tpm_chip;
struct trusted_key_payload;
struct trusted_key_options;
@@ -139,17 +163,6 @@ struct tpm_chip_seqops {
/* fixed define for the curve we use which is NIST_P256 */
#define EC_PT_SZ 32
-/*
- * fixed define for the size of a name. This is actually HASHALG size
- * plus 2, so 32 for SHA256
- */
-#define TPM2_NAME_SIZE 34
-
-/*
- * The maximum size for an object context
- */
-#define TPM2_MAX_CONTEXT_SIZE 4096
-
struct tpm_chip {
struct device dev;
struct device devs;
@@ -211,7 +224,7 @@ struct tpm_chip {
/* saved context for NULL seed */
u8 null_key_context[TPM2_MAX_CONTEXT_SIZE];
/* name of NULL seed */
- u8 null_key_name[TPM2_NAME_SIZE];
+ u8 null_key_name[TPM2_NULL_NAME_SIZE];
u8 null_ec_key_x[EC_PT_SZ];
u8 null_ec_key_y[EC_PT_SZ];
struct tpm2_auth *auth;
--
2.39.5
^ permalink raw reply related
* [PATCH v7 02/12] KEYS: trusted: Use get_random_bytes_wait() instead of tpm_get_random()
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Eric Biggers, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
Substitute remaining tpm_get_random() calls in trusted_tpm1.c with
get_random_bytes_wait() thus aligning random number generation for TPM 1.2
with the removal of '.get_random' callback.
Cc: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
security/keys/trusted-keys/trusted_tpm1.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 7ce7e31bcdfb..3d75bb6f9689 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -371,13 +371,10 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
unsigned char ononce[TPM_NONCE_SIZE];
int ret;
- ret = tpm_get_random(chip, ononce, TPM_NONCE_SIZE);
+ ret = get_random_bytes_wait(ononce, TPM_NONCE_SIZE);
if (ret < 0)
return ret;
- if (ret != TPM_NONCE_SIZE)
- return -EIO;
-
tpm_buf_reset(tb, TPM_TAG_RQU_COMMAND, TPM_ORD_OSAP);
tpm_buf_append_u16(tb, type);
tpm_buf_append_u32(tb, handle);
@@ -464,15 +461,10 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
memcpy(td->xorwork + SHA1_DIGEST_SIZE, sess.enonce, SHA1_DIGEST_SIZE);
sha1(td->xorwork, SHA1_DIGEST_SIZE * 2, td->xorhash);
- ret = tpm_get_random(chip, td->nonceodd, TPM_NONCE_SIZE);
+ ret = get_random_bytes_wait(td->nonceodd, TPM_NONCE_SIZE);
if (ret < 0)
goto out;
- if (ret != TPM_NONCE_SIZE) {
- ret = -EIO;
- goto out;
- }
-
ordinal = htonl(TPM_ORD_SEAL);
datsize = htonl(datalen);
pcrsize = htonl(pcrinfosize);
@@ -575,14 +567,10 @@ static int tpm_unseal(struct tpm_buf *tb,
}
ordinal = htonl(TPM_ORD_UNSEAL);
- ret = tpm_get_random(chip, nonceodd, TPM_NONCE_SIZE);
+ ret = get_random_bytes_wait(nonceodd, TPM_NONCE_SIZE);
if (ret < 0)
return ret;
- if (ret != TPM_NONCE_SIZE) {
- pr_info("tpm_get_random failed (%d)\n", ret);
- return -EIO;
- }
ret = TSS_authhmac(authdata1, keyauth, TPM_NONCE_SIZE,
enonce1, nonceodd, cont, sizeof(uint32_t),
&ordinal, bloblen, blob, 0, 0);
--
2.39.5
^ permalink raw reply related
* [PATCH v7 01/12] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Eric Biggers, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251216074454.2192499-1-jarkko@kernel.org>
1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
use should be pooled rather than directly used. This both reduces
latency and improves its predictability.
2. Linux is better off overall if every subsystem uses the same source for
generating the random numbers required.
Thus, unset '.get_random', which causes fallback to kernel_get_random().
One might argue that TPM RNG should be used for the generated trusted keys,
so that they have matching entropy with the TPM internally generated
objects.
This argument does have some weight into it but as far cryptography goes,
FIPS certification sets the exact bar, not which exact FIPS certified RNG
will be used. Thus, the rational choice is obviously to pick the lowest
latency path, which is kernel RNG.
Finally, there is an actual defence in depth benefit when using kernel RNG
as it helps to mitigate TPM firmware bugs concerning RNG implementation,
given the obfuscation by the other entropy sources.
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v7:
- A new patch. Simplifies follow up patches.
---
security/keys/trusted-keys/trusted_tpm1.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..7ce7e31bcdfb 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -6,6 +6,16 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+/**
+ * DOC: Random Number Generation
+ *
+ * tpm_get_random() was previously used here as the RNG in order to have equal
+ * entropy with the objects fully inside the TPM. However, as far as goes,
+ * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
+ * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
+ * associated with the RNG.
+ */
+
#include <crypto/hash_info.h>
#include <crypto/sha1.h>
#include <crypto/utils.h>
@@ -936,11 +946,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
return ret;
}
-static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
-{
- return tpm_get_random(chip, key, key_len);
-}
-
static int __init init_digests(void)
{
int i;
@@ -992,6 +997,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
.init = trusted_tpm_init,
.seal = trusted_tpm_seal,
.unseal = trusted_tpm_unseal,
- .get_random = trusted_tpm_get_random,
.exit = trusted_tpm_exit,
};
--
2.39.5
^ permalink raw reply related
* [PATCH v7 00/12] Streamline TPM2 HMAC sessions
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
This patch set contains accumulated patches, which gradually improve
TPM2 HMAC session management and TPM driver memory management.
RNG test
========
I run this test both TPM1 and TPM2 chips using QEMU and swtpm:
#!/bin/sh
ctrl_c() {
kill -15 $TEST_PID
echo 0 > tracing_on
echo nop > current_tracer
echo > kprobe_events
echo > set_ftrace_filter
echo BYE
exit
}
trap ctrl_c EXIT INT
mount -t tracefs none /sys/kernel/tracing
set -e
cd /sys/kernel/tracing
echo function > current_tracer
echo p:tpm_get_random tpm_get_random > kprobe_events
echo tpm_get_random > set_ftrace_filter
echo 1 > tracing_on
TEST_PID=$(cat /dev/hwrng > /dev/null &)
echo > trace
cat trace_pipe &
sleep 10
Change Log
==========
v7:
- Updated cover letter to match better the current state of the patch
set.
v6:
- OK, so I decided to send one more update with managed allocations
moved to the tail so that it does not block reviewing more trivial
patches.
- Trimmed some of the patches and improved commit messages.
v5:
- I decided to add the managed allocation patch to this and take it from
the master branch for the time being, as it needs more eyes despite
having already one reviewed-by tag (especially tested-by tags).
Jarkko Sakkinen (12):
KEYS: trusted: Use get_random-fallback for TPM
KEYS: trusted: Use get_random_bytes_wait() instead of tpm_get_random()
tpm2-sessions: Define TPM2_NAME_MAX_SIZE
KEYS: trusted: Open code tpm2_buf_append()
KEYS: trusted: Remove dead branch from tpm2_unseal_cmd
KEYS: trusted: Re-orchestrate tpm2_read_public() calls
tpm2-sessions: Remove AUTH_MAX_NAMES
tpm: Orchestrate TPM commands in tpm_get_random()
tpm: Change tpm_get_random() opportunistic
tpm-buf: Merge TPM_BUF_BOUNDARY_ERROR and TPM_BUF_OVERFLOW
tpm-buf: Implement managed allocations
tpm-buf: Remove tpm_buf_append_handle
drivers/char/tpm/tpm-buf.c | 154 ++++-----
drivers/char/tpm/tpm-interface.c | 145 ++++++++-
drivers/char/tpm/tpm-sysfs.c | 23 +-
drivers/char/tpm/tpm.h | 3 -
drivers/char/tpm/tpm1-cmd.c | 198 ++++--------
drivers/char/tpm/tpm2-cmd.c | 371 +++++++---------------
drivers/char/tpm/tpm2-sessions.c | 272 ++++++----------
drivers/char/tpm/tpm2-space.c | 44 ++-
drivers/char/tpm/tpm_vtpm_proxy.c | 30 +-
include/linux/tpm.h | 77 +++--
security/keys/trusted-keys/trusted_tpm1.c | 70 ++--
security/keys/trusted-keys/trusted_tpm2.c | 329 ++++++++++---------
12 files changed, 776 insertions(+), 940 deletions(-)
--
2.39.5
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-16 7:38 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sumit Garg, Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Michael Chan, Pavan Chebbi,
Rafał Miłecki, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <dhunzydod4d7vj73llpuqemxb5er2ja4emxusr66irwf77jhhb@es4yd2axzl25>
Hi Uwe,
On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello Sumit,
>
> On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > the objective of this series is to make tee driver stop using callbacks
> > > in struct device_driver. These were superseded by bus methods in 2006
> > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > methods.")) but nobody cared to convert all subsystems accordingly.
> > >
> > > Here the tee drivers are converted. The first commit is somewhat
> > > unrelated, but simplifies the conversion (and the drivers). It
> > > introduces driver registration helpers that care about setting the bus
> > > and owner. (The latter is missing in all drivers, so by using these
> > > helpers the drivers become more correct.)
> > >
> > > The patches #4 - #17 depend on the first two, so if they should be
> > > applied to their respective subsystem trees these must contain the first
> > > two patches first.
> >
> > Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
> > drivers.
>
> Thanks for your feedback. I will prepare a v2 and address your comments
> (whitespace issues and wrong callback in the shutdown method).
>
> > > Note that after patch #2 is applied, unconverted drivers provoke a
> > > warning in driver_register(), so it would be good for the user
> > > experience if the whole series goes in during a single merge window.
> >
> > +1
> >
> > I suggest the whole series goes via the Jens tree since there shouldn't
> > be any chances for conflict here.
> >
> > > So
> > > I guess an immutable branch containing the frist three patches that can
> > > be merged into the other subsystem trees would be sensible.
> > >
> > > After all patches are applied, tee_bus_type can be made private to
> > > drivers/tee as it's not used in other places any more.
> > >
> >
> > Feel free to make the tee_bus_type private as the last patch in the series
> > such that any followup driver follows this clean approach.
>
> There is a bit more to do for that than I'm willing to invest. With my
> patch series applied `tee_bus_type` is still used in
> drivers/tee/optee/device.c and drivers/tee/tee_core.c.
Oh I see, I guess we need to come with some helpers around device
register/unregister from TEE subsystem as well. Let's plan that for a
followup patch-set, I don't want this patch-set to be bloated more.
> Maybe it's
> sensible to merge these two files into a single one.
It's not possible as the design for TEE bus is to have TEE
implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
register devices on the bus.
>
> The things I wonder about additionally are:
>
> - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
> drivers but not devices.
Yeah since the devices are rather added by the TEE implementation driver.
>
> - optee_register_device() calls device_create_file() on
> &optee_device->dev after device_register(&optee_device->dev).
> (Attention half-knowledge!) I think device_create_file() should not
> be called on an already registered device (or you have to send a
> uevent afterwards). This should probably use type attribute groups.
> (Or the need_supplicant attribute should be dropped as it isn't very
> useful. This would maybe be considered an ABI change however.)
The reasoning for this attribute should be explained by commit:
7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
In summary it's due to a weird dependency for devices we have with the
user-space daemon: tee-supplicant.
>
> - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> optee devices in its error path (optee_unregister_devices())?
This is mostly to take care of if any device got registered before the
failure occured. Let me know if you have a better way to address that.
-Sumit
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: James Bottomley @ 2025-12-16 6:48 UTC (permalink / raw)
To: Jarkko Sakkinen, Eric Biggers
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list, Jason A. Donenfeld
In-Reply-To: <aUB5IsJeWhFvX-cA@kernel.org>
On Mon, 2025-12-15 at 23:09 +0200, Jarkko Sakkinen wrote:
> Using combined entropy also decreases corrateral damage caused by
> e.g., a buggy TPM firmware, which does happen sometimes in the wild.
Just to allay concerns on this point: the random number generator of a
physical TPM is always based on a hardware entropy generating element.
NIST specifies (and FIPS testing requires) that this hardware element
conform to SP 800-90B which is about 84 pages of how a RNG should be
conditioned and tested (and certified), so there should be very little
chance of issues here.
While TPMs have had problems caused by buggy firmware in the past, it's
always affected areas that the FIPS testing doesn't cover in such depth
(like the Infineon weak prime problem). People should feel confident
in the TPM random number generator (particularly because it's the
primary boot time entropy source for the in-kernel RNG on most
laptops).
Regards,
James
^ permalink raw reply
* Re: [PATCH] lsm: fix kernel-doc struct member names
From: Paul Moore @ 2025-12-16 2:51 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: Randy Dunlap, James Morris, Serge E. Hallyn,
linux-security-module
In-Reply-To: <20251214201539.2188497-1-rdunlap@infradead.org>
On Dec 14, 2025 Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Use the correct struct member names to avoid kernel-doc warnings:
>
> Warning: include/linux/lsm_hooks.h:83 struct member 'name' not described
> in 'lsm_id'
> Warning: include/linux/lsm_hooks.h:183 struct member 'initcall_device' not
> described in 'lsm_info'
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: linux-security-module@vger.kernel.org
> ---
> include/linux/lsm_hooks.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks Randy, merged into lsm/dev.
--
paul-moore.com
^ permalink raw reply
* [PATCH v3] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:49 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Eric Biggers, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
use should be pooled rather than directly used. This both reduces
latency and improves its predictability.
2. Linux is better off overall if every subsystem uses the same source for
generating the random numbers required.
Thus, unset '.get_random', which causes fallback to kernel_get_random().
One might argue that TPM RNG should be used for the generated trusted keys,
so that they have matching entropy with the TPM internally generated
objects.
This argument does have some weight into it but as far cryptography goes,
FIPS certification sets the exact bar, not which exact FIPS certified RNG
will be used. Thus, the rational choice is obviously to pick the lowest
latency path, which is kernel RNG.
Finally, there is an actual defence in depth benefit when using kernel RNG
as it helps to mitigate TPM firmware bugs concerning RNG implementation,
given the obfuscation by the other entropy sources.
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
- Fixed typos in the commit message.
- Moved the documentation comment to the correct location.
v2:
- Added Eric's reviewed-by tag.
- Addressed concerns from James by writing more details to the commit
message and documenting random number generation to the source
code.
---
security/keys/trusted-keys/trusted_tpm1.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..7ce7e31bcdfb 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -6,6 +6,16 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+/**
+ * DOC: Random Number Generation
+ *
+ * tpm_get_random() was previously used here as the RNG in order to have equal
+ * entropy with the objects fully inside the TPM. However, as far as goes,
+ * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
+ * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
+ * associated with the RNG.
+ */
+
#include <crypto/hash_info.h>
#include <crypto/sha1.h>
#include <crypto/utils.h>
@@ -936,11 +946,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
return ret;
}
-static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
-{
- return tpm_get_random(chip, key, key_len);
-}
-
static int __init init_digests(void)
{
int i;
@@ -992,6 +997,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
.init = trusted_tpm_init,
.seal = trusted_tpm_seal,
.unseal = trusted_tpm_unseal,
- .get_random = trusted_tpm_get_random,
.exit = trusted_tpm_exit,
};
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v2] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:25 UTC (permalink / raw)
To: linux-integrity
Cc: Eric Biggers, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251215231438.565522-1-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 01:14:38AM +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> generating the random numbers required.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> One might argue that TPM RNG should be used so that generated trusted keys
> have the matching entropy with the TPM internally generated objects.
>
> This argument does some weight into it but as far cryptography goes, FIPS
> certification sets the exact bar, not which exact FIPS certified RNG will
> be used. Thus, the rational choice is obviously to pick the lowest latency
> path.
>
> Finally, there also some actual defence in depth benefits on using kernel
> RNG. E.g., it helps to mitigate TPM firmware bugs concerning RNG
> implementation, which do happen in the wild occasionally.
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
I noticed also some typos in the commit message.
I think I will also supplement this with a patch that unexports
tpm_get_random(), as the patch zeros the external call sites.
Full encapsulation to the driver is exactly should aim for in order to
make hwrng easier target for further optimizations.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:17 UTC (permalink / raw)
To: linux-integrity
Cc: Eric Biggers, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251215231438.565522-1-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 01:14:38AM +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> generating the random numbers required.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> One might argue that TPM RNG should be used so that generated trusted keys
> have the matching entropy with the TPM internally generated objects.
>
> This argument does some weight into it but as far cryptography goes, FIPS
> certification sets the exact bar, not which exact FIPS certified RNG will
> be used. Thus, the rational choice is obviously to pick the lowest latency
> path.
>
> Finally, there also some actual defence in depth benefits on using kernel
> RNG. E.g., it helps to mitigate TPM firmware bugs concerning RNG
> implementation, which do happen in the wild occasionally.
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v2:
> - Added Eric's reviewed-by tag.
> - Addressed concerns from James by writing more details to the commit
> message and documenting random number generation to the source
> code.
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> security/keys/trusted-keys/trusted_tpm2.c | 9 +++++++++
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 636acb66a4f6..33b7739741c3 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> return ret;
> }
>
> -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> -{
> - return tpm_get_random(chip, key, key_len);
> -}
> -
> static int __init init_digests(void)
> {
> int i;
> @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> .init = trusted_tpm_init,
> .seal = trusted_tpm_seal,
> .unseal = trusted_tpm_unseal,
> - .get_random = trusted_tpm_get_random,
> .exit = trusted_tpm_exit,
> };
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index a7ea4a1c3bed..d16be47f1305 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -2,6 +2,15 @@
> /*
> * Copyright (C) 2004 IBM Corporation
> * Copyright (C) 2014 Intel Corporation
> +
> +/**
> + * DOC: Random Number Generation
> + *
> + * tpm_get_random() was previously used here as the RNG in order to have equal
> + * entropy with the objects fully inside the TPM. However, as far as goes,
> + * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
> + * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
> + * associated with the RNG.
> */
Sorry, this should have gone to trusted_tpm1.c :-)
>
> #include <linux/asn1_encoder.h>
> --
> 2.39.5
>
BR, Jarkko
^ permalink raw reply
* [PATCH v2] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:14 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Eric Biggers, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
use should be pooled rather than directly used. This both reduces
latency and improves its predictability.
2. Linux is better off overall if every subsystem uses the same source for
generating the random numbers required.
Thus, unset '.get_random', which causes fallback to kernel_get_random().
One might argue that TPM RNG should be used so that generated trusted keys
have the matching entropy with the TPM internally generated objects.
This argument does some weight into it but as far cryptography goes, FIPS
certification sets the exact bar, not which exact FIPS certified RNG will
be used. Thus, the rational choice is obviously to pick the lowest latency
path.
Finally, there also some actual defence in depth benefits on using kernel
RNG. E.g., it helps to mitigate TPM firmware bugs concerning RNG
implementation, which do happen in the wild occasionally.
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
- Added Eric's reviewed-by tag.
- Addressed concerns from James by writing more details to the commit
message and documenting random number generation to the source
code.
---
security/keys/trusted-keys/trusted_tpm1.c | 6 ------
security/keys/trusted-keys/trusted_tpm2.c | 9 +++++++++
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..33b7739741c3 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
return ret;
}
-static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
-{
- return tpm_get_random(chip, key, key_len);
-}
-
static int __init init_digests(void)
{
int i;
@@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
.init = trusted_tpm_init,
.seal = trusted_tpm_seal,
.unseal = trusted_tpm_unseal,
- .get_random = trusted_tpm_get_random,
.exit = trusted_tpm_exit,
};
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a7ea4a1c3bed..d16be47f1305 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -2,6 +2,15 @@
/*
* Copyright (C) 2004 IBM Corporation
* Copyright (C) 2014 Intel Corporation
+
+/**
+ * DOC: Random Number Generation
+ *
+ * tpm_get_random() was previously used here as the RNG in order to have equal
+ * entropy with the objects fully inside the TPM. However, as far as goes,
+ * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
+ * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
+ * associated with the RNG.
*/
#include <linux/asn1_encoder.h>
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v5 1/6] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-15 22:21 UTC (permalink / raw)
To: m; +Cc: gnoack, jack, linux-security-module, mic, utilityemal77, xandfury
In-Reply-To: <ef02e290-84b0-4de9-85aa-bf94d38c0c44@maowtm.org>
On 12/14/25 17:53, Tingmao Wang wrote:
> On 12/14/25 17:05, Justin Suess wrote:
>> [...]
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index d4f47d20361a..6ab3e7bd1c81 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -127,10 +127,39 @@ struct landlock_ruleset_attr {
>> * allowed_access in the passed in rule_attr. When this flag is
>> * present, the caller is also allowed to pass in an empty
>> * allowed_access.
>> + * %LANDLOCK_ADD_RULE_NO_INHERIT
>> + * When set on a rule being added to a ruleset, this flag disables the
>> + * inheritance of access rights and flags from parent objects.
>> + *
>> + * This flag currently applies only to filesystem rules. Adding it to
>> + * non-filesystem rules will return -EINVAL, unless future extensions
>> + * of Landlock define other hierarchical object types.
>> + *
>> + * By default, Landlock filesystem rules inherit allowed accesses from
>> + * ancestor directories: if a parent directory grants certain rights,
>> + * those rights also apply to its children. A rule marked with
>> + * LANDLOCK_ADD_RULE_NO_INHERIT stops this propagation at the directory
>> + * covered by the rule. Descendants of that directory continue to inherit
>> + * normally unless they also have rules using this flag.
>> + *
>> + * If a regular file is marked with this flag, it will not inherit any
>> + * access rights from its parent directories; only the accesses explicitly
>> + * allowed by the rule will apply to that file.
>> + *
>> + * This flag also enforces parent-directory restrictions: rename, rmdir,
>> + * link, and other operations that would change the directory's immediate
>> + * parent subtree are denied up to the VFS root. This prevents
>> + * sandboxed processes from manipulating the filesystem hierarchy to evade
>> + * restrictions (e.g., via sandbox-restart attacks).
>> + *
>> + * In addition, this flag blocks the inheritance of rule-layer flags
> tbh I feel that it's less confusing to just say "rule flags" (instead of
> "rule-layer flags").
Agreed. I'll change it here and in any other locations it pops up, I'll have to see.
>> + * (such as the quiet flag) from parent directories to the object covered
>> + * by this rule.
>> */
>>
>> /* clang-format off */
>> #define LANDLOCK_ADD_RULE_QUIET (1U << 0)
>> +#define LANDLOCK_ADD_RULE_NO_INHERIT (1U << 1)
>> /* clang-format on */
>>
>> /**
>> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
>> index 0b589263ea42..8d8623ea857f 100644
>> --- a/security/landlock/fs.c
>> +++ b/security/landlock/fs.c
>> @@ -317,6 +317,37 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
>> LANDLOCK_ACCESS_FS_IOCTL_DEV)
>> /* clang-format on */
>>
>> +enum landlock_walk_result {
>> + LANDLOCK_WALK_CONTINUE,
>> + LANDLOCK_WALK_STOP_REAL_ROOT,
>> + LANDLOCK_WALK_MOUNT_ROOT,
>> +};
>> +
>> +static enum landlock_walk_result landlock_walk_path_up(struct path *const path)
>> +{
>> + while (path->dentry == path->mnt->mnt_root) {
>> + if (!follow_up(path))
>> + return LANDLOCK_WALK_STOP_REAL_ROOT;
>> + }
>> +
>> + if (unlikely(IS_ROOT(path->dentry))) {
>> + if (likely(path->mnt->mnt_flags & MNT_INTERNAL))
>> + return LANDLOCK_WALK_MOUNT_ROOT;
> imo, LANDLOCK_WALK_MOUNT_ROOT is a somewhat confusing name for this,
> especially in the context that if we see this in
> is_access_to_paths_allowed() we allow access unconditionally.
>
> Would LANDLOCK_WALK_INTERNAL be a better name here?
>
Yeah that seems better. LANDLOCK_WALK_INTERNAL seems like a better name.
Plus some documenting comments in the landlock_walk_result are warranted.
I'll fix it in the next version.
>> + dput(path->dentry);
>> + path->dentry = dget(path->mnt->mnt_root);
>> + return LANDLOCK_WALK_CONTINUE;
>> + }
>> +
>> + struct dentry *const parent = dget_parent(path->dentry);
>> +
>> + dput(path->dentry);
>> + path->dentry = parent;
>> + return LANDLOCK_WALK_CONTINUE;
>> +}
>> +
>> +static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
>> + const struct dentry *const dentry);
>> +
>> /*
>> * @path: Should have been checked by get_path_from_fd().
>> */
>> @@ -344,6 +375,48 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>> return PTR_ERR(id.key.object);
>> mutex_lock(&ruleset->lock);
>> err = landlock_insert_rule(ruleset, id, access_rights, flags);
>> + if (err || !(flags & LANDLOCK_ADD_RULE_NO_INHERIT))
>> + goto out_unlock;
>> +
>> + /* Create ancestor rules and set has_no_inherit_descendant flags */
>> + struct path walker = *path;
>> +
>> + path_get(&walker);
>> + while (landlock_walk_path_up(&walker) != LANDLOCK_WALK_STOP_REAL_ROOT) {
> Why not landlock_walk_path_up(&walker) == LANDLOCK_WALK_CONTINUE here?
> I'm not sure if it's actually possible to end up with an infinite loop by
> ignoring LANDLOCK_WALK_MOUNT_ROOT (i.e. not sure if "internal" mounts can
> have disconnected dentries), but it seems safer to write to loop in a way
> such that if that happens, we exit.
I don't *think* it's possible to end up in an infinite loop this way, but you never know.
I'll definitely take your suggestion because it's semantically clearer at the very least.
>
>> + struct landlock_rule *ancestor_rule;
>> +
>> + if (WARN_ON_ONCE(!walker.dentry || d_is_negative(walker.dentry))) {
>> + err = -EIO;
>> + break;
>> + }
>> +
>> + ancestor_rule = (struct landlock_rule *)find_rule(ruleset, walker.dentry);
>> + if (!ancestor_rule) {
>> + struct landlock_id ancestor_id = {
>> + .type = LANDLOCK_KEY_INODE,
>> + .key.object = get_inode_object(d_backing_inode(walker.dentry)),
>> + };
>> +
>> + if (IS_ERR(ancestor_id.key.object)) {
>> + err = PTR_ERR(ancestor_id.key.object);
>> + break;
>> + }
>> + err = landlock_insert_rule(ruleset, ancestor_id, 0, 0);
>> + landlock_put_object(ancestor_id.key.object);
>> + if (err)
>> + break;
>> +
>> + ancestor_rule = (struct landlock_rule *)
>> + find_rule(ruleset, walker.dentry);
>> + }
>> + if (WARN_ON_ONCE(!ancestor_rule || ancestor_rule->num_layers != 1)) {
>> + err = -EIO;
>> + break;
>> + }
>> + ancestor_rule->layers[0].flags.has_no_inherit_descendant = true;
>> + }
>> + path_put(&walker);
>> +out_unlock:
>> mutex_unlock(&ruleset->lock);
>> /*
>> * No need to check for an error because landlock_insert_rule()
>> @@ -772,8 +845,10 @@ static bool is_access_to_paths_allowed(
>> _layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
>> layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
>> (*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
>> - struct collected_rule_flags *rule_flags_parent1 = &log_request_parent1->rule_flags;
>> - struct collected_rule_flags *rule_flags_parent2 = &log_request_parent2->rule_flags;
>> + struct collected_rule_flags *rule_flags_parent1 =
>> + &log_request_parent1->rule_flags;
>> + struct collected_rule_flags *rule_flags_parent2 =
>> + log_request_parent2 ? &log_request_parent2->rule_flags : NULL;
> Good point, I think the original was still safe because it would not be
> used by landlock_unmask_layers anyway, but this is better. I will take
> this in the next version, thanks!
No problem. I actually meant to put this as a review under your patch as
a comment but I pulled it in accidentally.
Rebasing off your patch has been a breeze btw 🙂
>
>> if (!access_request_parent1 && !access_request_parent2)
>> return true;
>> @@ -784,7 +859,7 @@ static bool is_access_to_paths_allowed(
>> if (is_nouser_or_private(path->dentry))
>> return true;
>>
>> - if (WARN_ON_ONCE(!layer_masks_parent1))
>> + if (WARN_ON_ONCE(!layer_masks_parent1 || !log_request_parent1))
>> return false;
>>
>> allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
>> @@ -851,6 +926,7 @@ static bool is_access_to_paths_allowed(
>> */
>> while (true) {
>> const struct landlock_rule *rule;
>> + enum landlock_walk_result walk_res;
>>
>> /*
>> * If at least all accesses allowed on the destination are
>> @@ -910,46 +986,14 @@ static bool is_access_to_paths_allowed(
>> if (allowed_parent1 && allowed_parent2)
>> break;
>>
>> -jump_up:
>> - if (walker_path.dentry == walker_path.mnt->mnt_root) {
>> - if (follow_up(&walker_path)) {
>> - /* Ignores hidden mount points. */
>> - goto jump_up;
>> - } else {
>> - /*
>> - * Stops at the real root. Denies access
>> - * because not all layers have granted access.
>> - */
>> - break;
>> - }
>> - }
>> -
>> - if (unlikely(IS_ROOT(walker_path.dentry))) {
>> - if (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) {
>> - /*
>> - * Stops and allows access when reaching disconnected root
>> - * directories that are part of internal filesystems (e.g. nsfs,
>> - * which is reachable through /proc/<pid>/ns/<namespace>).
>> - */
>> - allowed_parent1 = true;
>> - allowed_parent2 = true;
>> - break;
>> - }
>> -
>> - /*
>> - * We reached a disconnected root directory from a bind mount.
>> - * Let's continue the walk with the mount point we missed.
>> - */
> I think we might want to preserve these comments.
Agreed. Thank you, I missed those. I'll preserve them in the next version.
>
>> - dput(walker_path.dentry);
>> - walker_path.dentry = walker_path.mnt->mnt_root;
>> - dget(walker_path.dentry);
>> - } else {
>> - struct dentry *const parent_dentry =
>> - dget_parent(walker_path.dentry);
>> -
>> - dput(walker_path.dentry);
>> - walker_path.dentry = parent_dentry;
>> + walk_res = landlock_walk_path_up(&walker_path);
>> + if (walk_res == LANDLOCK_WALK_MOUNT_ROOT) {
>> + allowed_parent1 = true;
>> + allowed_parent2 = true;
>> + break;
>> }
>> + if (walk_res != LANDLOCK_WALK_CONTINUE)
>> + break;
>> }
>> path_put(&walker_path);
>>
>> @@ -963,7 +1007,7 @@ static bool is_access_to_paths_allowed(
>> ARRAY_SIZE(*layer_masks_parent1);
>> }
>>
>> - if (!allowed_parent2) {
>> + if (!allowed_parent2 && log_request_parent2) {
>> log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
>> log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
>> log_request_parent2->audit.u.path = *path;
>> @@ -1037,8 +1081,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
>> * collect_domain_accesses - Walk through a file path and collect accesses
>> *
>> * @domain: Domain to check against.
>> - * @mnt_root: Last directory to check.
>> - * @dir: Directory to start the walk from.
>> + * @mnt_root: Last path element to check.
>> + * @dir: Directory path to start the walk from.
>> * @layer_masks_dom: Where to store the collected accesses.
>> *
>> * This helper is useful to begin a path walk from the @dir directory to a
>> @@ -1060,29 +1104,31 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
>> */
>> static bool collect_domain_accesses(
>> const struct landlock_ruleset *const domain,
>> - const struct dentry *const mnt_root, struct dentry *dir,
>> + const struct path *const mnt_root, const struct path *const dir,
>> layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS],
>> struct collected_rule_flags *const rule_flags)
>> {
> This function only walks up to the mountpoint of dir. If dir is changed
> from a *dentry to a *path, wouldn't mnt_root be redundant? Since
> mnt_root->dentry is always going to be dir->mnt->mnt_root. This also
> means that they can't accidentally not be the same.
Good catch, yeah they should be redundant.
I'll remove the mnt_root parameter in the next version.
>
>> unsigned long access_dom;
>> bool ret = false;
>> + struct path walker;
>>
>> if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
>> return true;
>> - if (is_nouser_or_private(dir))
>> + if (is_nouser_or_private(dir->dentry))
>> return true;
>>
>> access_dom = landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
>> layer_masks_dom,
>> LANDLOCK_KEY_INODE);
>>
>> - dget(dir);
>> + walker = *dir;
>> + path_get(&walker);
>> while (true) {
>> - struct dentry *parent_dentry;
>> + enum landlock_walk_result walk_res;
>>
>> /* Gets all layers allowing all domain accesses. */
>> if (landlock_unmask_layers(
>> - find_rule(domain, dir), access_dom, layer_masks_dom,
>> + find_rule(domain, walker.dentry), access_dom, layer_masks_dom,
>> ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
>> /*
>> * Stops when all handled accesses are allowed by at
>> @@ -1091,22 +1137,69 @@ static bool collect_domain_accesses(
>> ret = true;
>> break;
>> }
>> -
>> - /*
>> - * Stops at the mount point or the filesystem root for a disconnected
>> - * directory.
>> - */
>> - if (dir == mnt_root || unlikely(IS_ROOT(dir)))
>> + if (walker.dentry == mnt_root->dentry && walker.mnt == mnt_root->mnt)
>> + break;
>> + walk_res = landlock_walk_path_up(&walker);
>> + if (walk_res != LANDLOCK_WALK_CONTINUE)
>> break;
>> -
>> - parent_dentry = dget_parent(dir);
>> - dput(dir);
>> - dir = parent_dentry;
>> }
>> - dput(dir);
>> + path_put(&walker);
>> return ret;
>> }
>>
>> +/**
>> + * deny_no_inherit_topology_change - deny topology changes on sealed paths
>> + * @subject: Subject performing the operation (contains the domain).
>> + * @path: Path whose dentry is the target of the topology modification.
>> + *
>> + * Checks whether any domain layers are sealed against topology changes at
>> + * @path. If so, emit an audit record and return -EACCES. Otherwise return 0.
>> + */
>> +static int deny_no_inherit_topology_change(const struct landlock_cred_security
>> + *subject,
>> + const struct path *const path)
> Since you're not using path->mnt here (except for a NULL check), would it
> be easier to just pass the dentry instead? In that case you wouldn't have
> to do an inline initializer in current_check_refer_path / hook_path_*
> below as well.
Yeah, this was leftover before I did some refactoring and removed
the mark_no_inherit_ancestors. Good catch.
I'll address this in the next version.
>
>> +{
>> + layer_mask_t sealed_layers = 0;
>> + layer_mask_t override_layers = 0;
>> + const struct landlock_rule *rule;
>> + u32 layer_index;
>> + unsigned long audit_layer_index;
>> +
>> + if (WARN_ON_ONCE(!subject || !path || !path->dentry || !path->mnt ||
>> + d_is_negative(path->dentry)))
>> + return 0;
>> +
>> + rule = find_rule(subject->domain, path->dentry);
>> + if (!rule)
>> + return 0;
>> +
>> + for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
>> + const struct landlock_layer *layer = &rule->layers[layer_index];
>> + layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
>> +
>> + if (layer->flags.no_inherit ||
>> + layer->flags.has_no_inherit_descendant)
>> + sealed_layers |= layer_bit;
>> + else
>> + override_layers |= layer_bit;
>> + }
>> +
>> + sealed_layers &= ~override_layers;
>> + if (!sealed_layers)
>> + return 0;
>> +
>> + audit_layer_index = __ffs((unsigned long)sealed_layers);
>> + landlock_log_denial(subject, &(struct landlock_request) {
>> + .type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
>> + .audit = {
>> + .type = LSM_AUDIT_DATA_DENTRY,
>> + .u.dentry = path->dentry,
>> + },
>> + .layer_plus_one = audit_layer_index + 1,
>> + });
>> + return -EACCES;
>> +}
>> +
>> /**
>> * current_check_refer_path - Check if a rename or link action is allowed
>> *
>> @@ -1191,6 +1284,21 @@ static int current_check_refer_path(struct dentry *const old_dentry,
>> access_request_parent2 =
>> get_mode_access(d_backing_inode(old_dentry)->i_mode);
>> if (removable) {
>> + int err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = new_dir->mnt,
>> + .dentry = old_dentry });
>> +
>> + if (err)
>> + return err;
>> + if (exchange) {
>> + err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = new_dir->mnt,
>> + .dentry = new_dentry });
>> + if (err)
>> + return err;
>> + }
>> access_request_parent1 |= maybe_remove(old_dentry);
>> access_request_parent2 |= maybe_remove(new_dentry);
>> }
>> @@ -1232,12 +1340,15 @@ static int current_check_refer_path(struct dentry *const old_dentry,
>> old_dentry->d_parent;
>>
>> /* new_dir->dentry is equal to new_dentry->d_parent */
>> - allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
>> - old_parent,
>> + allow_parent1 = collect_domain_accesses(subject->domain,
>> + &mnt_dir,
>> + &(struct path){ .mnt = new_dir->mnt,
>> + .dentry = old_parent },
>> &layer_masks_parent1,
>> &request1.rule_flags);
>> - allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
>> - new_dir->dentry,
>> + allow_parent2 = collect_domain_accesses(subject->domain, &mnt_dir,
>> + &(struct path){ .mnt = new_dir->mnt,
>> + .dentry = new_dir->dentry },
>> &layer_masks_parent2,
>> &request2.rule_flags);
>>
>> @@ -1583,12 +1694,37 @@ static int hook_path_symlink(const struct path *const dir,
>> static int hook_path_unlink(const struct path *const dir,
>> struct dentry *const dentry)
>> {
>> + const struct landlock_cred_security *const subject =
>> + landlock_get_applicable_subject(current_cred(), any_fs, NULL);
>> + int err;
>> +
>> + if (subject) {
>> + err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = dir->mnt,
>> + .dentry = dentry });
>> + if (err)
>> + return err;
>> + }
>> return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
>> }
>>
>> static int hook_path_rmdir(const struct path *const dir,
>> struct dentry *const dentry)
>> {
>> + const struct landlock_cred_security *const subject =
>> + landlock_get_applicable_subject(current_cred(), any_fs, NULL);
>> + int err;
>> +
>> + if (subject) {
>> + err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = dir->mnt,
>> + .dentry = dentry });
>> + if (err)
>> + return err;
>> + }
>> +
>> return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
>> }
>>
>> [...]
Overall I'm feeling pretty good about this series, but if either you or Mickaël have any more feedback I'd like to hear it.
I'll wait until your next quiet flag version comes and do a rebase before sending the revisions.
Sorry for the double tap Tingmao, I forgot to cc the mailing list :(
Regards,
Justin
^ permalink raw reply
* Re: [PATCH v2 15/17] KEYS: trusted: Make use of tee bus methods
From: Jarkko Sakkinen @ 2025-12-15 22:04 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, Sumit Garg, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
linux-integrity, keyrings, linux-security-module, op-tee,
linux-kernel, Sumit Garg
In-Reply-To: <ad8aaa343c1e8523659259290f63aea8be906977.1765791463.git.u.kleine-koenig@baylibre.com>
On Mon, Dec 15, 2025 at 03:16:45PM +0100, Uwe Kleine-König wrote:
> The tee bus got dedicated callbacks for probe and remove.
> Make use of these. This fixes a runtime warning about the driver needing
> to be converted to the bus methods.
>
> Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> security/keys/trusted-keys/trusted_tee.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
> index 3cea9a377955..6e465c8bef5e 100644
> --- a/security/keys/trusted-keys/trusted_tee.c
> +++ b/security/keys/trusted-keys/trusted_tee.c
> @@ -202,9 +202,9 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> return 0;
> }
>
> -static int trusted_key_probe(struct device *dev)
> +static int trusted_key_probe(struct tee_client_device *rng_device)
> {
> - struct tee_client_device *rng_device = to_tee_client_device(dev);
> + struct device *dev = &rng_device->dev;
> int ret;
> struct tee_ioctl_open_session_arg sess_arg;
I'm sorry but cannot help saying but these not being in reverse tree
order hurts my eyes ;-)
I.e., I'd personally move declaration of sess_arg right after rng_device
despite being additional change to the scope of the patch.
That said, Sumit has the ultimate veto right here, and this not any kind
of fault in this patch so I will obviously ack the patch;
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> @@ -244,13 +244,11 @@ static int trusted_key_probe(struct device *dev)
> return ret;
> }
>
> -static int trusted_key_remove(struct device *dev)
> +static void trusted_key_remove(struct tee_client_device *dev)
> {
> unregister_key_type(&key_type_trusted);
> tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
> tee_client_close_context(pvt_data.ctx);
> -
> - return 0;
> }
>
> static const struct tee_client_device_id trusted_key_id_table[] = {
> @@ -261,11 +259,11 @@ static const struct tee_client_device_id trusted_key_id_table[] = {
> MODULE_DEVICE_TABLE(tee, trusted_key_id_table);
>
> static struct tee_client_driver trusted_key_driver = {
> + .probe = trusted_key_probe,
> + .remove = trusted_key_remove,
> .id_table = trusted_key_id_table,
> .driver = {
> .name = DRIVER_NAME,
> - .probe = trusted_key_probe,
> - .remove = trusted_key_remove,
> },
> };
>
> --
> 2.47.3
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 14/17] KEYS: trusted: Migrate to use tee specific driver registration function
From: Jarkko Sakkinen @ 2025-12-15 22:01 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, Sumit Garg, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
linux-integrity, keyrings, linux-security-module, op-tee,
linux-kernel, Sumit Garg
In-Reply-To: <687c004c32718ba7044ffa9165f33842267a745d.1765791463.git.u.kleine-koenig@baylibre.com>
On Mon, Dec 15, 2025 at 03:16:44PM +0100, Uwe Kleine-König wrote:
> The tee subsystem recently got a set of dedicated functions to register
> (and unregister) a tee driver. Make use of them. These care for setting the
> driver's bus (so the explicit assignment can be dropped) and the driver
> owner (which is an improvement this driver benefits from).
>
> Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> security/keys/trusted-keys/trusted_tee.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
> index aa3d477de6db..3cea9a377955 100644
> --- a/security/keys/trusted-keys/trusted_tee.c
> +++ b/security/keys/trusted-keys/trusted_tee.c
> @@ -264,7 +264,6 @@ static struct tee_client_driver trusted_key_driver = {
> .id_table = trusted_key_id_table,
> .driver = {
> .name = DRIVER_NAME,
> - .bus = &tee_bus_type,
> .probe = trusted_key_probe,
> .remove = trusted_key_remove,
> },
> @@ -272,12 +271,12 @@ static struct tee_client_driver trusted_key_driver = {
>
> static int trusted_tee_init(void)
> {
> - return driver_register(&trusted_key_driver.driver);
> + return tee_client_driver_register(&trusted_key_driver);
> }
>
> static void trusted_tee_exit(void)
> {
> - driver_unregister(&trusted_key_driver.driver);
> + tee_client_driver_unregister(&trusted_key_driver);
> }
>
> struct trusted_key_ops trusted_key_tee_ops = {
> --
> 2.47.3
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 00/46] Allow inlining C helpers into Rust when using LTO
From: Danilo Krummrich @ 2025-12-15 21:40 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, linux-kernel, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Peter Zijlstra, Boqun Feng,
Elle Rhumsaa, Carlos Llamas, Yury Norov, Andreas Hindborg,
linux-block, FUJITA Tomonori, Miguel Ojeda, Michael Turquette,
Stephen Boyd, linux-clk, Benno Lossin, Thomas Gleixner,
Rafael J. Wysocki, Viresh Kumar, linux-pm, Paul Moore,
Serge Hallyn, linux-security-module, Daniel Almeida,
Abdiel Janulgue, Robin Murphy, Lyude Paul, Alexander Viro,
Christian Brauner, Jan Kara, linux-fsdevel, Josh Poimboeuf,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Brendan Higgins,
David Gow, Rae Moar, linux-kselftest, Andrew Morton,
Liam R. Howlett, Andrew Ballance, maple-tree, linux-mm,
Lorenzo Stoakes, Uladzislau Rezki, Vitaly Wool, Rob Herring,
Saravana Kannan, devicetree, Bjorn Helgaas,
Krzysztof Wilczyński, linux-pci, Remo Senekowitsch,
Paul E. McKenney, rcu, Will Deacon, Fiona Behrens, Gary Guo,
Liam Girdwood, Mark Brown, Alexandre Courbot, Vlastimil Babka,
Christoph Lameter, David Rientjes, Ingo Molnar, Waiman Long,
Mitchell Levy, Frederic Weisbecker, Anna-Maria Behnsen,
John Stultz, linux-usb, Tejun Heo, Lai Jiangshan, Matthew Wilcox,
Tamir Duberstein
In-Reply-To: <20251202-define-rust-helper-v1-0-a2e13cbc17a6@google.com>
On Tue Dec 2, 2025 at 8:37 PM CET, Alice Ryhl wrote:
Applied to driver-core-testing, thanks!
> Alice Ryhl (46):
> rust: auxiliary: add __rust_helper to helpers
> rust: device: add __rust_helper to helpers
> rust: dma: add __rust_helper to helpers
> rust: io: add __rust_helper to helpers
> rust: irq: add __rust_helper to helpers
> rust: pci: add __rust_helper to helpers
[ Consider latest helper additions. - Danilo ]
> rust: platform: add __rust_helper to helpers
> rust: property: add __rust_helper to helpers
> rust: scatterlist: add __rust_helper to helpers
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 21:09 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list,
Jason A. Donenfeld
In-Reply-To: <aUBxKqL5hFibwI3r@kernel.org>
On Mon, Dec 15, 2025 at 10:35:57PM +0200, Jarkko Sakkinen wrote:
> On Mon, Dec 15, 2025 at 08:09:39PM +0000, Eric Biggers wrote:
> > On Sun, Dec 14, 2025 at 11:32:36PM +0200, Jarkko Sakkinen wrote:
> > > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> > > use should be pooled rather than directly used. This both reduces
> > > latency and improves its predictability.
> > >
> > > 2. Linux is better off overall if every subsystem uses the same source for
> > > the random bistream as the de-facto choice, unless *force majeure*
> > > reasons point to some other direction.
> > >
> > > In the case, of TPM there is no reason for trusted keys to invoke TPM
> > > directly.
> > >
> > > Thus, unset '.get_random', which causes fallback to kernel_get_random().
> > >
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > > security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> > > 1 file changed, 6 deletions(-)
> > >
> > > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > > index 636acb66a4f6..33b7739741c3 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > > @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> > > return ret;
> > > }
> > >
> > > -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> > > -{
> > > - return tpm_get_random(chip, key, key_len);
> > > -}
> > > -
> > > static int __init init_digests(void)
> > > {
> > > int i;
> > > @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> > > .init = trusted_tpm_init,
> > > .seal = trusted_tpm_seal,
> > > .unseal = trusted_tpm_unseal,
> > > - .get_random = trusted_tpm_get_random,
> > > .exit = trusted_tpm_exit,
> > > };
> >
> > Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> >
> > Agreed that kernel code should prefer the standard Linux RNG whenever
> > possible. Note that the standard Linux RNG already incorporates entropy
> > from hardware RNGs, when available.
>
> I get also the argument of using TPM RNG here just for the sake of
> matching the creation with fully internally generated TPM objects.
>
> I'm a bit little in-between what to do with this patch.
>
> I suggested a comment to James. Other alternative would be do this
> change and update this patch with a comment:
>
> /*
> * tpm_get_random() was used previously here as the RNG in order to match
> * rng with the objects generated internally inside the TPM. However, since
> * e.g., FIPS certification requires kernel crypto and rng to be FIPS
> * certified, formally kernel_get_random() is equally legit source for
> * the random numbers.
> */
>
> It's longish but I think this fully covers the whole issue.
>
> And if there is ever need to return to this, it's a good remainder of
> the design choices.
I'll supplement the patch with that explanatory comment. I think the
previous discussions pointed out by James were useful reflection point
and that comment summarizes that discussion.
I'll add your reviewd-by to the next version, as no additional code
changes will be implemented.
I think that this discussion also implies that the callback itself is
somewhat questionable, perhaps even harmful. Same arguments apply also
to e.g., TEE trusted keys. IMHO, would be overall best for Linux to a
have a one single call path for generating random numbers.
Using combined entropy also decreases corrateral damage caused by e.g.,
a buggy TPM firmware, which does happen sometimes in the wild.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 20:35 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list,
Jason A. Donenfeld
In-Reply-To: <20251215200939.GA10539@google.com>
On Mon, Dec 15, 2025 at 08:09:39PM +0000, Eric Biggers wrote:
> On Sun, Dec 14, 2025 at 11:32:36PM +0200, Jarkko Sakkinen wrote:
> > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> > use should be pooled rather than directly used. This both reduces
> > latency and improves its predictability.
> >
> > 2. Linux is better off overall if every subsystem uses the same source for
> > the random bistream as the de-facto choice, unless *force majeure*
> > reasons point to some other direction.
> >
> > In the case, of TPM there is no reason for trusted keys to invoke TPM
> > directly.
> >
> > Thus, unset '.get_random', which causes fallback to kernel_get_random().
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index 636acb66a4f6..33b7739741c3 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> > return ret;
> > }
> >
> > -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> > -{
> > - return tpm_get_random(chip, key, key_len);
> > -}
> > -
> > static int __init init_digests(void)
> > {
> > int i;
> > @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> > .init = trusted_tpm_init,
> > .seal = trusted_tpm_seal,
> > .unseal = trusted_tpm_unseal,
> > - .get_random = trusted_tpm_get_random,
> > .exit = trusted_tpm_exit,
> > };
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
>
> Agreed that kernel code should prefer the standard Linux RNG whenever
> possible. Note that the standard Linux RNG already incorporates entropy
> from hardware RNGs, when available.
I get also the argument of using TPM RNG here just for the sake of
matching the creation with fully internally generated TPM objects.
I'm a bit little in-between what to do with this patch.
I suggested a comment to James. Other alternative would be do this
change and update this patch with a comment:
/*
* tpm_get_random() was used previously here as the RNG in order to match
* rng with the objects generated internally inside the TPM. However, since
* e.g., FIPS certification requires kernel crypto and rng to be FIPS
* certified, formally kernel_get_random() is equally legit source for
* the random numbers.
*/
It's longish but I think this fully covers the whole issue.
And if there is ever need to return to this, it's a good remainder of
the design choices.
>
> - Eric
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 20:25 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <5446f517848338b4ccac8d7bbedf4cc1ed315cb4.camel@HansenPartnership.com>
On Mon, Dec 15, 2025 at 09:01:49PM +0100, James Bottomley wrote:
> On Mon, 2025-12-15 at 21:43 +0200, Jarkko Sakkinen wrote:
> [...]
> > I think there is misunderstanding with FIPS.
> >
> > Having FIPS certificated RNG in TPM matters but it only matters only
> > in the sense that callers can be FIPS certified as they use that RNG
> > as a source.
> >
> > Using FIPS certified RNG does not magically make callers be FIPS
> > ceritified actors. The data is contaminated in that sense at the
> > point when kernel acquires it.
>
> I think FIPS certification is a red herring. The point being made in
> the original thread is about RNG quality. The argument essentially
> being that the quality of the TPM RNG is known at all points in time
> but the quality of the kernel RNG (particularly at start of day when
> the entropy pool is new) is less certain.
OK, that's fair point.
I.e., using TPM2_GetRandom here makes sense, not because of FIPS
certification per se but because it is guarantees matching entropy to
other types of keys generated with TPM2_Create (as everything uses the
same RNG).
I can buy this but think it would really make sense to add a comment to
the source code.
I was thinking something along the lines of:
/*
* tpm_get_random() is used here directly instead of relying kernel's
* RNG in order to match RNGs with objects generated by TPM internally.
*/
It does not mention FIPS explicitly because I think this is already
enforcing condition and thus enough. And e.g., applies also when one
uses an emulator (and thus useful tidbit for that use and purpose).
>
> Regards,
>
> James
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Eric Biggers @ 2025-12-15 20:09 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list,
Jason A. Donenfeld
In-Reply-To: <20251214213236.339586-1-jarkko@kernel.org>
On Sun, Dec 14, 2025 at 11:32:36PM +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> the random bistream as the de-facto choice, unless *force majeure*
> reasons point to some other direction.
>
> In the case, of TPM there is no reason for trusted keys to invoke TPM
> directly.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 636acb66a4f6..33b7739741c3 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> return ret;
> }
>
> -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> -{
> - return tpm_get_random(chip, key, key_len);
> -}
> -
> static int __init init_digests(void)
> {
> int i;
> @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> .init = trusted_tpm_init,
> .seal = trusted_tpm_seal,
> .unseal = trusted_tpm_unseal,
> - .get_random = trusted_tpm_get_random,
> .exit = trusted_tpm_exit,
> };
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Agreed that kernel code should prefer the standard Linux RNG whenever
possible. Note that the standard Linux RNG already incorporates entropy
from hardware RNGs, when available.
- Eric
^ permalink raw reply
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