* [PATCH v8 12/12] tpm-buf: Implement managed allocations
From: Jarkko Sakkinen @ 2025-12-16 9:21 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Ross Philipson, Stefan Berger, Peter Huewe,
Jarkko Sakkinen, 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: <20251216092147.2326606-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Decouple kzalloc from buffer creation, so that a managed allocation can be
used:
struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE,
GFP_KERNEL);
if (!buf)
return -ENOMEM;
tpm_buf_init(buf, TPM_BUFSIZE);
Alternatively, stack allocations are also possible:
u8 buf_data[512];
struct tpm_buf *buf = (struct tpm_buf *)buf_data;
tpm_buf_init(buf, sizeof(buf_data));
This is achieved by embedding buffer's header inside the allocated blob,
instead of having an outer wrapper.
Cc: Ross Philipson <ross.philipson@oracle.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
drivers/char/tpm/tpm-buf.c | 123 ++++++----
drivers/char/tpm/tpm-interface.c | 66 +++---
drivers/char/tpm/tpm-sysfs.c | 21 +-
drivers/char/tpm/tpm.h | 1 -
drivers/char/tpm/tpm1-cmd.c | 129 +++++------
drivers/char/tpm/tpm2-cmd.c | 266 ++++++++++------------
drivers/char/tpm/tpm2-sessions.c | 142 ++++++------
drivers/char/tpm/tpm2-space.c | 44 ++--
drivers/char/tpm/tpm_vtpm_proxy.c | 30 ++-
include/linux/tpm.h | 20 +-
security/keys/trusted-keys/trusted_tpm1.c | 36 +--
security/keys/trusted-keys/trusted_tpm2.c | 173 +++++++-------
12 files changed, 499 insertions(+), 552 deletions(-)
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index 11b6bcf2f43d..042da92f94e1 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -7,81 +7,107 @@
#include <linux/module.h>
#include <linux/tpm.h>
-/**
- * tpm_buf_init() - Allocate and initialize a TPM command
- * @buf: A &tpm_buf
- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
- * @ordinal: A command ordinal
- *
- * Return: 0 or -ENOMEM
- */
-int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size)
{
- buf->data = (u8 *)__get_free_page(GFP_KERNEL);
- if (!buf->data)
- return -ENOMEM;
-
- tpm_buf_reset(buf, tag, ordinal);
- return 0;
+ u32 buf_size_2 = (u32)buf->capacity + (u32)sizeof(*buf);
+
+ if (!buf->capacity) {
+ if (buf_size > TPM_BUFSIZE) {
+ WARN(1, "%s: size overflow: %u\n", __func__, buf_size);
+ buf->flags |= TPM_BUF_INVALID;
+ }
+ } else {
+ if (buf_size != buf_size_2) {
+ WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size,
+ buf_size_2);
+ buf->flags |= TPM_BUF_INVALID;
+ }
+ }
}
-EXPORT_SYMBOL_GPL(tpm_buf_init);
-/**
- * tpm_buf_reset() - Initialize a TPM command
- * @buf: A &tpm_buf
- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
- * @ordinal: A command ordinal
- */
-void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static void __tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal)
{
struct tpm_header *head = (struct tpm_header *)buf->data;
+ __tpm_buf_size_invariant(buf, buf_size);
+
+ if (buf->flags & TPM_BUF_INVALID)
+ return;
+
WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS &&
tag != TPM2_ST_SESSIONS && tag != 0);
buf->flags = 0;
buf->length = sizeof(*head);
+ buf->capacity = buf_size - sizeof(*buf);
head->tag = cpu_to_be16(tag);
head->length = cpu_to_be32(sizeof(*head));
head->ordinal = cpu_to_be32(ordinal);
}
-EXPORT_SYMBOL_GPL(tpm_buf_reset);
+
+static void __tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size)
+{
+ __tpm_buf_size_invariant(buf, buf_size);
+
+ if (buf->flags & TPM_BUF_INVALID)
+ return;
+
+ buf->flags = TPM_BUF_TPM2B;
+ buf->length = 2;
+ buf->capacity = buf_size - sizeof(*buf);
+ buf->data[0] = 0;
+ buf->data[1] = 0;
+}
/**
- * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer
- * @buf: A @tpm_buf
- *
- * Return: 0 or -ENOMEM
+ * tpm_buf_init() - Initialize a TPM command
+ * @buf: A &tpm_buf
+ * @buf_size: Size of the buffer.
*/
-int tpm_buf_init_sized(struct tpm_buf *buf)
+void tpm_buf_init(struct tpm_buf *buf, u16 buf_size)
{
- buf->data = (u8 *)__get_free_page(GFP_KERNEL);
- if (!buf->data)
- return -ENOMEM;
+ memset(buf, 0, buf_size);
+ __tpm_buf_reset(buf, buf_size, TPM_TAG_RQU_COMMAND, 0);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
- tpm_buf_reset_sized(buf);
- return 0;
+/**
+ * tpm_buf_init_sized() - Initialize a sized buffer
+ * @buf: A &tpm_buf
+ * @buf_size: Size of the buffer.
+ */
+void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size)
+{
+ memset(buf, 0, buf_size);
+ __tpm_buf_reset_sized(buf, buf_size);
}
EXPORT_SYMBOL_GPL(tpm_buf_init_sized);
/**
- * tpm_buf_reset_sized() - Initialize a sized buffer
+ * tpm_buf_reset() - Re-initialize a TPM command
* @buf: A &tpm_buf
+ * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
+ * @ordinal: A command ordinal
*/
-void tpm_buf_reset_sized(struct tpm_buf *buf)
+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
{
- buf->flags = TPM_BUF_TPM2B;
- buf->length = 2;
- buf->data[0] = 0;
- buf->data[1] = 0;
+ u16 buf_size = buf->capacity + sizeof(*buf);
+
+ __tpm_buf_reset(buf, buf_size, tag, ordinal);
}
-EXPORT_SYMBOL_GPL(tpm_buf_reset_sized);
+EXPORT_SYMBOL_GPL(tpm_buf_reset);
-void tpm_buf_destroy(struct tpm_buf *buf)
+/**
+ * tpm_buf_reset_sized() - Re-initialize a sized buffer
+ * @buf: A &tpm_buf
+ */
+void tpm_buf_reset_sized(struct tpm_buf *buf)
{
- free_page((unsigned long)buf->data);
+ u16 buf_size = buf->capacity + sizeof(*buf);
+
+ __tpm_buf_reset_sized(buf, buf_size);
}
-EXPORT_SYMBOL_GPL(tpm_buf_destroy);
+EXPORT_SYMBOL_GPL(tpm_buf_reset_sized);
/**
* tpm_buf_length() - Return the number of bytes consumed by the data
@@ -89,8 +115,11 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy);
*
* Return: The number of bytes consumed by the buffer
*/
-u32 tpm_buf_length(struct tpm_buf *buf)
+u16 tpm_buf_length(struct tpm_buf *buf)
{
+ if (buf->flags & TPM_BUF_INVALID)
+ return 0;
+
return buf->length;
}
EXPORT_SYMBOL_GPL(tpm_buf_length);
@@ -103,10 +132,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length);
*/
void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length)
{
+ u32 total_length = (u32)buf->length + (u32)new_length;
+
if (buf->flags & TPM_BUF_INVALID)
return;
- if ((buf->length + new_length) > PAGE_SIZE) {
+ if (total_length > (u32)buf->capacity) {
WARN(1, "tpm_buf: write overflow\n");
buf->flags |= TPM_BUF_INVALID;
return;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 677dcef05dfb..0810e58aca7a 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -494,46 +494,38 @@ struct tpm1_get_random_out {
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;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, max);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GETRANDOM);
+ tpm_buf_append_u32(buf, max);
- rc = tpm_transmit_cmd(chip, &buf, sizeof(resp->rng_data_len), "TPM_GetRandom");
+ rc = tpm_transmit_cmd(chip, buf, sizeof(resp->rng_data_len), "TPM_GetRandom");
if (rc) {
if (rc > 0)
rc = -EIO;
- goto err;
+ return rc;
}
- resp = (struct tpm1_get_random_out *)&buf.data[TPM_HEADER_SIZE];
+ 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 (recd > max)
+ return -EIO;
- if (buf.length < TPM_HEADER_SIZE + sizeof(resp->rng_data_len) + recd) {
- rc = -EIO;
- goto err;
- }
+ if (buf->length < TPM_HEADER_SIZE + sizeof(resp->rng_data_len) + recd)
+ return -EIO;
memcpy(out, resp->rng_data, recd);
- tpm_buf_destroy(&buf);
return recd;
-
-err:
- tpm_buf_destroy(&buf);
- return rc;
}
struct tpm2_get_random_out {
@@ -545,7 +537,6 @@ 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;
@@ -553,30 +544,30 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
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;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, &buf,
+ tpm_buf_append_hmac_session(chip, buf,
TPM2_SA_ENCRYPT | TPM2_SA_CONTINUE_SESSION,
NULL, 0);
} else {
- head = (struct tpm_header *)buf.data;
+ head = (struct tpm_header *)buf->data;
head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
}
- tpm_buf_append_u16(&buf, max);
+ tpm_buf_append_u16(buf, max);
- ret = tpm_buf_fill_hmac_session(chip, &buf);
- if (ret) {
- tpm_buf_destroy(&buf);
+ ret = tpm_buf_fill_hmac_session(chip, buf);
+ if (ret)
return ret;
- }
- ret = tpm_transmit_cmd(chip, &buf, offsetof(struct tpm2_get_random_out, buffer),
+ ret = tpm_transmit_cmd(chip, buf, offsetof(struct tpm2_get_random_out, buffer),
"TPM2_GetRandom");
- ret = tpm_buf_check_hmac_response(chip, &buf, ret);
+ ret = tpm_buf_check_hmac_response(chip, buf, ret);
if (ret) {
if (ret > 0)
ret = -EIO;
@@ -584,17 +575,17 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
goto out;
}
- head = (struct tpm_header *)buf.data;
+ 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];
+ resp = (struct tpm2_get_random_out *)&buf->data[offset];
recd = min_t(u32, be16_to_cpu(resp->size), max);
- if (tpm_buf_length(&buf) <
+ if (tpm_buf_length(buf) <
TPM_HEADER_SIZE + offsetof(struct tpm2_get_random_out, buffer) + recd) {
ret = -EIO;
goto out;
@@ -605,7 +596,6 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
out:
tpm2_end_auth_session(chip);
- tpm_buf_destroy(&buf);
return ret;
}
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 4a6a27ee295d..0f321c307bc6 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -32,28 +32,29 @@ struct tpm_readpubek_out {
static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- struct tpm_buf tpm_buf;
struct tpm_readpubek_out *out;
int i;
char *str = buf;
struct tpm_chip *chip = to_tpm_chip(dev);
char anti_replay[20];
+ struct tpm_buf *tpm_buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tpm_buf)
+ return -ENOMEM;
+
memset(&anti_replay, 0, sizeof(anti_replay));
if (tpm_try_get_ops(chip))
return 0;
- if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK))
- goto out_ops;
-
- tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
+ tpm_buf_init(tpm_buf, TPM_BUFSIZE);
+ tpm_buf_reset(tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK);
+ tpm_buf_append(tpm_buf, anti_replay, sizeof(anti_replay));
- if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE,
- "attempting to read the PUBEK"))
- goto out_buf;
+ if (tpm_transmit_cmd(chip, tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, "TPM_ReadPubek"))
+ goto out_ops;
- out = (struct tpm_readpubek_out *)&tpm_buf.data[10];
+ out = (struct tpm_readpubek_out *)&tpm_buf->data[10];
str +=
sprintf(str,
"Algorithm: %4ph\n"
@@ -71,8 +72,6 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
for (i = 0; i < 256; i += 16)
str += sprintf(str, "%16ph\n", &out->modulus[i]);
-out_buf:
- tpm_buf_destroy(&tpm_buf);
out_ops:
tpm_put_ops(chip);
return str - buf;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f698d01401de..c4bbd8f04605 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -32,7 +32,6 @@
#endif
#define TPM_MINOR 224 /* officially assigned */
-#define TPM_BUFSIZE 4096
#define TPM_NUM_DEVICES 65536
#define TPM_RETRY 50
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 0604e11c9778..d8f16e66666b 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -323,20 +323,14 @@ unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
*/
static int tpm1_startup(struct tpm_chip *chip)
{
- struct tpm_buf buf;
- int rc;
-
- dev_info(&chip->dev, "starting up the TPM manually\n");
-
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
- if (rc < 0)
- return rc;
-
- tpm_buf_append_u16(&buf, TPM_ST_CLEAR);
-
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM");
- tpm_buf_destroy(&buf);
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
+ tpm_buf_append_u16(buf, TPM_ST_CLEAR);
+ return tpm_transmit_cmd(chip, buf, 0, "TPM_Startup");
}
int tpm1_get_timeouts(struct tpm_chip *chip)
@@ -463,50 +457,47 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
const char *log_msg)
{
- struct tpm_buf buf;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
- if (rc)
- return rc;
-
- tpm_buf_append_u32(&buf, pcr_idx);
- tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
-
- rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, log_msg);
- tpm_buf_destroy(&buf);
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
+ tpm_buf_append_u32(buf, pcr_idx);
+ tpm_buf_append(buf, hash, TPM_DIGEST_SIZE);
+ return tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE, log_msg);
}
#define TPM_ORD_GET_CAP 101
ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc, size_t min_cap_length)
{
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_CAP);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_CAP);
if (subcap_id == TPM_CAP_VERSION_1_1 ||
subcap_id == TPM_CAP_VERSION_1_2) {
- tpm_buf_append_u32(&buf, subcap_id);
- tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u32(buf, subcap_id);
+ tpm_buf_append_u32(buf, 0);
} else {
if (subcap_id == TPM_CAP_FLAG_PERM ||
subcap_id == TPM_CAP_FLAG_VOL)
- tpm_buf_append_u32(&buf, TPM_CAP_FLAG);
+ tpm_buf_append_u32(buf, TPM_CAP_FLAG);
else
- tpm_buf_append_u32(&buf, TPM_CAP_PROP);
+ tpm_buf_append_u32(buf, TPM_CAP_PROP);
- tpm_buf_append_u32(&buf, 4);
- tpm_buf_append_u32(&buf, subcap_id);
+ tpm_buf_append_u32(buf, 4);
+ tpm_buf_append_u32(buf, subcap_id);
}
- rc = tpm_transmit_cmd(chip, &buf, min_cap_length, desc);
+ rc = tpm_transmit_cmd(chip, buf, min_cap_length, desc);
if (!rc)
- *cap = *(cap_t *)&buf.data[TPM_HEADER_SIZE + 4];
- tpm_buf_destroy(&buf);
+ *cap = *(cap_t *)&buf->data[TPM_HEADER_SIZE + 4];
return rc;
}
EXPORT_SYMBOL_GPL(tpm1_getcap);
@@ -514,29 +505,24 @@ EXPORT_SYMBOL_GPL(tpm1_getcap);
#define TPM_ORD_PCRREAD 21
int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
{
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, pcr_idx);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD);
+ tpm_buf_append_u32(buf, pcr_idx);
- rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE,
- "attempting to read a pcr value");
+ rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE, "TPM_PCRRead");
if (rc)
- goto out;
-
- if (tpm_buf_length(&buf) < TPM_DIGEST_SIZE) {
- rc = -EFAULT;
- goto out;
- }
+ return rc;
- memcpy(res_buf, &buf.data[TPM_HEADER_SIZE], TPM_DIGEST_SIZE);
+ if (buf->length < TPM_DIGEST_SIZE)
+ return -EFAULT;
-out:
- tpm_buf_destroy(&buf);
+ memcpy(res_buf, &buf->data[TPM_HEADER_SIZE], TPM_DIGEST_SIZE);
return rc;
}
@@ -550,16 +536,13 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
*/
static int tpm1_continue_selftest(struct tpm_chip *chip)
{
- struct tpm_buf buf;
- int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_CONTINUE_SELFTEST);
- if (rc)
- return rc;
-
- rc = tpm_transmit_cmd(chip, &buf, 0, "continue selftest");
- tpm_buf_destroy(&buf);
- return rc;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_CONTINUE_SELFTEST);
+ return tpm_transmit_cmd(chip, buf, 0, "TPM_ContinueSelfTest");
}
/**
@@ -673,22 +656,24 @@ int tpm1_auto_startup(struct tpm_chip *chip)
int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
{
u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
- struct tpm_buf buf;
unsigned int try;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/* for buggy tpm, flush pcrs with extend to selected dummy */
if (tpm_suspend_pcr)
rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash,
"extending dummy pcr before suspend");
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
- if (rc)
- return rc;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
+
/* now do the actual savestate */
for (try = 0; try < TPM_RETRY; try++) {
- rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
+ rc = tpm_transmit_cmd(chip, buf, 0, NULL);
/*
* If the TPM indicates that it is too busy to respond to
* this command then retry before giving up. It can take
@@ -703,7 +688,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
break;
tpm_msleep(TPM_TIMEOUT_RETRY);
- tpm_buf_reset(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
}
if (rc)
@@ -713,8 +698,6 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
dev_warn(&chip->dev, "TPM savestate took %dms\n",
try * TPM_TIMEOUT_RETRY);
- tpm_buf_destroy(&buf);
-
return rc;
}
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 8ba6cd6819f1..f066efb54a2c 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -119,12 +119,15 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
{
int i;
int rc;
- struct tpm_buf buf;
struct tpm2_pcr_read_out *out;
u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
u16 digest_size;
u16 expected_digest_size = 0;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
if (pcr_idx >= TPM2_PLATFORM_PCR)
return -EINVAL;
@@ -139,36 +142,31 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
expected_digest_size = chip->allocated_banks[i].digest_size;
}
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
- if (rc)
- return rc;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
- tpm_buf_append_u32(&buf, 1);
- tpm_buf_append_u16(&buf, digest->alg_id);
- tpm_buf_append_u8(&buf, TPM2_PCR_SELECT_MIN);
- tpm_buf_append(&buf, (const unsigned char *)pcr_select,
+ tpm_buf_append_u32(buf, 1);
+ tpm_buf_append_u16(buf, digest->alg_id);
+ tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN);
+ tpm_buf_append(buf, (const unsigned char *)pcr_select,
sizeof(pcr_select));
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to read a pcr value");
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_PCR_Read");
if (rc)
- goto out;
+ return rc;
- out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
+ out = (struct tpm2_pcr_read_out *)&buf->data[TPM_HEADER_SIZE];
digest_size = be16_to_cpu(out->digest_size);
if (digest_size > sizeof(digest->digest) ||
- (!digest_size_ptr && digest_size != expected_digest_size)) {
- rc = -EINVAL;
- goto out;
- }
+ (!digest_size_ptr && digest_size != expected_digest_size))
+ return rc;
if (digest_size_ptr)
*digest_size_ptr = digest_size;
memcpy(digest->digest, out->digest, digest_size);
-out:
- tpm_buf_destroy(&buf);
return rc;
}
@@ -184,57 +182,53 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digests)
{
- struct tpm_buf buf;
int rc;
int i;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
if (!disable_pcr_integrity) {
rc = tpm2_start_auth_session(chip);
if (rc)
return rc;
}
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
- if (rc) {
- if (!disable_pcr_integrity)
- tpm2_end_auth_session(chip);
- return rc;
- }
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
if (!disable_pcr_integrity) {
- rc = tpm_buf_append_name(chip, &buf, pcr_idx, (u8 *)&pcr_idx,
+ rc = tpm_buf_append_name(chip, buf, pcr_idx, (u8 *)&pcr_idx,
sizeof(u32));
- if (rc) {
- tpm_buf_destroy(&buf);
+ if (rc)
return rc;
- }
- tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
+ tpm_buf_append_hmac_session(chip, buf, 0, NULL, 0);
} else {
- tpm_buf_append_u32(&buf, pcr_idx);
- tpm_buf_append_auth(chip, &buf, NULL, 0);
+ tpm_buf_append_u32(buf, pcr_idx);
+ tpm_buf_append_auth(chip, buf, NULL, 0);
}
- tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
+ tpm_buf_append_u32(buf, chip->nr_allocated_banks);
for (i = 0; i < chip->nr_allocated_banks; i++) {
- tpm_buf_append_u16(&buf, digests[i].alg_id);
- tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
+ tpm_buf_append_u16(buf, digests[i].alg_id);
+ tpm_buf_append(buf, (const unsigned char *)&digests[i].digest,
chip->allocated_banks[i].digest_size);
}
+ if (buf->flags & TPM_BUF_INVALID)
+ return -EINVAL;
if (!disable_pcr_integrity) {
- rc = tpm_buf_fill_hmac_session(chip, &buf);
- if (rc) {
- tpm_buf_destroy(&buf);
+ rc = tpm_buf_fill_hmac_session(chip, buf);
+ if (rc)
return rc;
- }
}
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
- if (!disable_pcr_integrity)
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_PCR_Extend");
- tpm_buf_destroy(&buf);
+ if (!disable_pcr_integrity)
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
return rc;
}
@@ -246,20 +240,18 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
*/
void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
{
- struct tpm_buf buf;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
- if (rc) {
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf) {
dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
handle);
return;
}
- tpm_buf_append_u32(&buf, handle);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
+ tpm_buf_append_u32(buf, handle);
- tpm_transmit_cmd(chip, &buf, 0, "flushing context");
- tpm_buf_destroy(&buf);
+ tpm_transmit_cmd(chip, buf, 0, "TPM2_FlushContext");
}
EXPORT_SYMBOL_GPL(tpm2_flush_context);
@@ -286,19 +278,20 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
const char *desc)
{
struct tpm2_get_cap_out *out;
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- return rc;
- tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
- tpm_buf_append_u32(&buf, property_id);
- tpm_buf_append_u32(&buf, 1);
- rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_TPM_PROPERTIES);
+ tpm_buf_append_u32(buf, property_id);
+ tpm_buf_append_u32(buf, 1);
+ rc = tpm_transmit_cmd(chip, buf, 0, NULL);
if (!rc) {
- out = (struct tpm2_get_cap_out *)
- &buf.data[TPM_HEADER_SIZE];
+ out = (struct tpm2_get_cap_out *)&buf->data[TPM_HEADER_SIZE];
/*
* To prevent failing boot up of some systems, Infineon TPM2.0
* returns SUCCESS on TPM2_Startup in field upgrade mode. Also
@@ -310,7 +303,6 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
else
rc = -ENODATA;
}
- tpm_buf_destroy(&buf);
return rc;
}
EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
@@ -327,15 +319,14 @@ EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
*/
void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
{
- struct tpm_buf buf;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
- if (rc)
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
return;
- tpm_buf_append_u16(&buf, shutdown_type);
- tpm_transmit_cmd(chip, &buf, 0, "stopping the TPM");
- tpm_buf_destroy(&buf);
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+ tpm_buf_append_u16(buf, shutdown_type);
+ tpm_transmit_cmd(chip, buf, 0, "TPM2_Shutdown");
}
/**
@@ -353,20 +344,19 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
*/
static int tpm2_do_selftest(struct tpm_chip *chip)
{
- struct tpm_buf buf;
int full;
int rc;
- for (full = 0; full < 2; full++) {
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
- if (rc)
- return rc;
-
- tpm_buf_append_u8(&buf, full);
- rc = tpm_transmit_cmd(chip, &buf, 0,
- "attempting the self test");
- tpm_buf_destroy(&buf);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+ for (full = 0; full < 2; full++) {
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+ tpm_buf_append_u8(buf, full);
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_SelfTest");
if (rc == TPM2_RC_TESTING)
rc = TPM2_RC_SUCCESS;
if (rc == TPM2_RC_INITIALIZE || rc == TPM2_RC_SUCCESS)
@@ -391,23 +381,24 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
int tpm2_probe(struct tpm_chip *chip)
{
struct tpm_header *out;
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- return rc;
- tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
- tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
- tpm_buf_append_u32(&buf, 1);
- rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_TPM_PROPERTIES);
+ tpm_buf_append_u32(buf, TPM_PT_TOTAL_COMMANDS);
+ tpm_buf_append_u32(buf, 1);
+ rc = tpm_transmit_cmd(chip, buf, 0, NULL);
/* We ignore TPM return codes on purpose. */
if (rc >= 0) {
- out = (struct tpm_header *)buf.data;
+ out = (struct tpm_header *)buf->data;
if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
chip->flags |= TPM_CHIP_FLAG_TPM2;
}
- tpm_buf_destroy(&buf);
return 0;
}
EXPORT_SYMBOL_GPL(tpm2_probe);
@@ -447,7 +438,6 @@ struct tpm2_pcr_selection {
ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
{
struct tpm2_pcr_selection pcr_selection;
- struct tpm_buf buf;
void *marker;
void *end;
void *pcr_select_offset;
@@ -459,39 +449,37 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
int rc;
int i = 0;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
- tpm_buf_append_u32(&buf, 0);
- tpm_buf_append_u32(&buf, 1);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_PCRS);
+ tpm_buf_append_u32(buf, 0);
+ tpm_buf_append_u32(buf, 1);
- rc = tpm_transmit_cmd(chip, &buf, 9, "get tpm pcr allocation");
+ rc = tpm_transmit_cmd(chip, buf, 9, "TPM2_GetCapability(PCRS)");
if (rc)
- goto out;
+ return rc;
- nr_possible_banks = be32_to_cpup(
- (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
+ nr_possible_banks = be32_to_cpup((__be32 *)&buf->data[TPM_HEADER_SIZE + 5]);
if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
pr_err("tpm: out of bank capacity: %u > %u\n",
nr_possible_banks, TPM2_MAX_PCR_BANKS);
- rc = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
- marker = &buf.data[TPM_HEADER_SIZE + 9];
+ marker = &buf->data[TPM_HEADER_SIZE + 9];
- rsp_len = be32_to_cpup((__be32 *)&buf.data[2]);
- end = &buf.data[rsp_len];
+ rsp_len = be32_to_cpup((__be32 *)&buf->data[2]);
+ end = &buf->data[rsp_len];
for (i = 0; i < nr_possible_banks; i++) {
pcr_select_offset = marker +
offsetof(struct tpm2_pcr_selection, size_of_select);
- if (pcr_select_offset >= end) {
- rc = -EFAULT;
- break;
- }
+ if (pcr_select_offset >= end)
+ return -EFAULT;
memcpy(&pcr_selection, marker, sizeof(pcr_selection));
hash_alg = be16_to_cpu(pcr_selection.hash_alg);
@@ -503,7 +491,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
rc = tpm2_init_bank_info(chip, nr_alloc_banks);
if (rc < 0)
- break;
+ return rc;
nr_alloc_banks++;
}
@@ -515,21 +503,21 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
}
chip->nr_allocated_banks = nr_alloc_banks;
-out:
- tpm_buf_destroy(&buf);
-
- return rc;
+ return 0;
}
int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
{
- struct tpm_buf buf;
u32 nr_commands;
__be32 *attrs;
u32 cc;
int i;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL);
if (rc)
goto out;
@@ -546,30 +534,24 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
goto out;
}
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- goto out;
-
- tpm_buf_append_u32(&buf, TPM2_CAP_COMMANDS);
- tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
- tpm_buf_append_u32(&buf, nr_commands);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_COMMANDS);
+ tpm_buf_append_u32(buf, TPM2_CC_FIRST);
+ tpm_buf_append_u32(buf, nr_commands);
- rc = tpm_transmit_cmd(chip, &buf, 9 + 4 * nr_commands, NULL);
- if (rc) {
- tpm_buf_destroy(&buf);
+ rc = tpm_transmit_cmd(chip, buf, 9 + 4 * nr_commands, NULL);
+ if (rc)
goto out;
- }
- if (nr_commands !=
- be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE + 5])) {
+ if (nr_commands != be32_to_cpup((__be32 *)&buf->data[TPM_HEADER_SIZE + 5])) {
rc = -EFAULT;
- tpm_buf_destroy(&buf);
goto out;
}
chip->nr_commands = nr_commands;
- attrs = (__be32 *)&buf.data[TPM_HEADER_SIZE + 9];
+ attrs = (__be32 *)&buf->data[TPM_HEADER_SIZE + 9];
for (i = 0; i < nr_commands; i++, attrs++) {
chip->cc_attrs_tbl[i] = be32_to_cpup(attrs);
cc = chip->cc_attrs_tbl[i] & 0xFFFF;
@@ -581,8 +563,6 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
}
}
- tpm_buf_destroy(&buf);
-
out:
if (rc > 0)
rc = -ENODEV;
@@ -603,20 +583,14 @@ EXPORT_SYMBOL_GPL(tpm2_get_cc_attrs_tbl);
static int tpm2_startup(struct tpm_chip *chip)
{
- struct tpm_buf buf;
- int rc;
-
- dev_info(&chip->dev, "starting up the TPM manually\n");
-
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP);
- if (rc < 0)
- return rc;
-
- tpm_buf_append_u16(&buf, TPM2_SU_CLEAR);
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM");
- tpm_buf_destroy(&buf);
-
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP);
+ tpm_buf_append_u16(buf, TPM2_SU_CLEAR);
+ return tpm_transmit_cmd(chip, buf, 0, "TPM2_Startup");
}
/**
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 016bed63a755..0926da478932 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -175,7 +175,6 @@ int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
u32 mso = tpm2_handle_mso(handle);
off_t offset = TPM_HEADER_SIZE;
int rc, name_size_alg;
- struct tpm_buf buf;
if (mso != TPM2_MSO_PERSISTENT && mso != TPM2_MSO_VOLATILE &&
mso != TPM2_MSO_NVRAM) {
@@ -183,47 +182,41 @@ int tpm2_read_public(struct tpm_chip *chip, u32 handle, void *name)
return sizeof(u32);
}
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, handle);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
+ tpm_buf_append_u32(buf, handle);
- rc = tpm_transmit_cmd(chip, &buf, 0, "TPM2_ReadPublic");
- if (rc) {
- tpm_buf_destroy(&buf);
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_ReadPublic");
+ if (rc)
return tpm_ret_to_err(rc);
- }
/* Skip TPMT_PUBLIC: */
- offset += tpm_buf_read_u16(&buf, &offset);
+ offset += tpm_buf_read_u16(buf, &offset);
/*
* Ensure space for the length field of TPM2B_NAME and hashAlg field of
* TPMT_HA (the extra four bytes).
*/
- if (offset + 4 > tpm_buf_length(&buf)) {
- tpm_buf_destroy(&buf);
+ if (offset + 4 > tpm_buf_length(buf))
return -EIO;
- }
- rc = tpm_buf_read_u16(&buf, &offset);
- name_size_alg = name_size(&buf.data[offset]);
+ rc = tpm_buf_read_u16(buf, &offset);
+ name_size_alg = name_size(&buf->data[offset]);
if (name_size_alg < 0)
return name_size_alg;
- if (rc != name_size_alg) {
- tpm_buf_destroy(&buf);
+ if (rc != name_size_alg)
return -EIO;
- }
- if (offset + rc > tpm_buf_length(&buf)) {
- tpm_buf_destroy(&buf);
+ if (offset + rc > tpm_buf_length(buf))
return -EIO;
- }
- memcpy(name, &buf.data[offset], rc);
+ memcpy(name, &buf->data[offset], rc);
return name_size_alg;
}
EXPORT_SYMBOL_GPL(tpm2_read_public);
@@ -929,7 +922,6 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
int tpm2_start_auth_session(struct tpm_chip *chip)
{
struct tpm2_auth *auth;
- struct tpm_buf buf;
u32 null_key;
int rc;
@@ -938,6 +930,10 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
return 0;
}
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
auth = kzalloc(sizeof(*auth), GFP_KERNEL);
if (!auth)
return -ENOMEM;
@@ -948,41 +944,37 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
auth->session = TPM_HEADER_SIZE;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
- if (rc)
- goto out;
-
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
/* salt key handle */
- tpm_buf_append_u32(&buf, null_key);
+ tpm_buf_append_u32(buf, null_key);
/* bind key handle */
- tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+ tpm_buf_append_u32(buf, TPM2_RH_NULL);
/* nonce caller */
get_random_bytes(auth->our_nonce, sizeof(auth->our_nonce));
- tpm_buf_append_u16(&buf, sizeof(auth->our_nonce));
- tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
+ tpm_buf_append_u16(buf, sizeof(auth->our_nonce));
+ tpm_buf_append(buf, auth->our_nonce, sizeof(auth->our_nonce));
/* append encrypted salt and squirrel away unencrypted in auth */
- tpm_buf_append_salt(&buf, chip, auth);
+ tpm_buf_append_salt(buf, chip, auth);
/* session type (HMAC, audit or policy) */
- tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
+ tpm_buf_append_u8(buf, TPM2_SE_HMAC);
/* symmetric encryption parameters */
/* symmetric algorithm */
- tpm_buf_append_u16(&buf, TPM_ALG_AES);
+ tpm_buf_append_u16(buf, TPM_ALG_AES);
/* bits for symmetric algorithm */
- tpm_buf_append_u16(&buf, AES_KEY_BITS);
+ tpm_buf_append_u16(buf, AES_KEY_BITS);
/* symmetric algorithm mode (must be CFB) */
- tpm_buf_append_u16(&buf, TPM_ALG_CFB);
+ tpm_buf_append_u16(buf, TPM_ALG_CFB);
/* hash algorithm for session */
- tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
+ tpm_buf_append_u16(buf, TPM_ALG_SHA256);
- rc = tpm_ret_to_err(tpm_transmit_cmd(chip, &buf, 0, "StartAuthSession"));
+ rc = tpm_ret_to_err(tpm_transmit_cmd(chip, buf, 0, "TPM2_StartAuthSession"));
tpm2_flush_context(chip, null_key);
if (rc == TPM2_RC_SUCCESS)
- rc = tpm2_parse_start_auth_session(auth, &buf);
-
- tpm_buf_destroy(&buf);
+ rc = tpm2_parse_start_auth_session(auth, buf);
if (rc == TPM2_RC_SUCCESS) {
chip->auth = auth;
@@ -1204,18 +1196,18 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
u32 *handle, u8 *name)
{
int rc;
- struct tpm_buf buf;
- struct tpm_buf template;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- rc = tpm_buf_init_sized(&template);
- if (rc) {
- tpm_buf_destroy(&buf);
- return rc;
- }
+ struct tpm_buf *template __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!template)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
+ tpm_buf_init_sized(template, TPM_BUFSIZE);
/*
* create the template. Note: in order for userspace to
@@ -1227,75 +1219,71 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
*/
/* key type */
- tpm_buf_append_u16(&template, TPM_ALG_ECC);
+ tpm_buf_append_u16(template, TPM_ALG_ECC);
/* name algorithm */
- tpm_buf_append_u16(&template, TPM_ALG_SHA256);
+ tpm_buf_append_u16(template, TPM_ALG_SHA256);
/* object properties */
- tpm_buf_append_u32(&template, TPM2_OA_NULL_KEY);
+ tpm_buf_append_u32(template, TPM2_OA_NULL_KEY);
/* sauth policy (empty) */
- tpm_buf_append_u16(&template, 0);
+ tpm_buf_append_u16(template, 0);
/* BEGIN parameters: key specific; for ECC*/
/* symmetric algorithm */
- tpm_buf_append_u16(&template, TPM_ALG_AES);
+ tpm_buf_append_u16(template, TPM_ALG_AES);
/* bits for symmetric algorithm */
- tpm_buf_append_u16(&template, AES_KEY_BITS);
+ tpm_buf_append_u16(template, AES_KEY_BITS);
/* algorithm mode (must be CFB) */
- tpm_buf_append_u16(&template, TPM_ALG_CFB);
+ tpm_buf_append_u16(template, TPM_ALG_CFB);
/* scheme (NULL means any scheme) */
- tpm_buf_append_u16(&template, TPM_ALG_NULL);
+ tpm_buf_append_u16(template, TPM_ALG_NULL);
/* ECC Curve ID */
- tpm_buf_append_u16(&template, TPM2_ECC_NIST_P256);
+ tpm_buf_append_u16(template, TPM2_ECC_NIST_P256);
/* KDF Scheme */
- tpm_buf_append_u16(&template, TPM_ALG_NULL);
+ tpm_buf_append_u16(template, TPM_ALG_NULL);
/* unique: key specific; for ECC it is two zero size points */
- tpm_buf_append_u16(&template, 0);
- tpm_buf_append_u16(&template, 0);
+ tpm_buf_append_u16(template, 0);
+ tpm_buf_append_u16(template, 0);
/* END parameters */
/* primary handle */
- tpm_buf_append_u32(&buf, hierarchy);
- tpm_buf_append_empty_auth(&buf, TPM2_RS_PW);
+ tpm_buf_append_u32(buf, hierarchy);
+ tpm_buf_append_empty_auth(buf, TPM2_RS_PW);
/* sensitive create size is 4 for two empty buffers */
- tpm_buf_append_u16(&buf, 4);
+ tpm_buf_append_u16(buf, 4);
/* sensitive create auth data (empty) */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* sensitive create sensitive data (empty) */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* the public template */
- tpm_buf_append(&buf, template.data, template.length);
- tpm_buf_destroy(&template);
+ tpm_buf_append(buf, template->data, template->length);
/* outside info (empty) */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* creation PCR (none) */
- tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u32(buf, 0);
- rc = tpm_transmit_cmd(chip, &buf, 0,
- "attempting to create NULL primary");
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_CreatePrimary");
if (rc == TPM2_RC_SUCCESS)
- rc = tpm2_parse_create_primary(chip, &buf, handle, hierarchy,
+ rc = tpm2_parse_create_primary(chip, buf, handle, hierarchy,
name);
- tpm_buf_destroy(&buf);
-
return rc;
}
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 60354cd53b5c..cbf86ff5931f 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -71,24 +71,25 @@ void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
unsigned int *offset, u32 *handle)
{
- struct tpm_buf tbuf;
struct tpm2_context *ctx;
unsigned int body_size;
int rc;
- rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
- if (rc)
- return rc;
+ struct tpm_buf *tbuf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tbuf)
+ return -ENOMEM;
+
+ tpm_buf_init(tbuf, TPM_BUFSIZE);
+ tpm_buf_reset(tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
ctx = (struct tpm2_context *)&buf[*offset];
body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
- tpm_buf_append(&tbuf, &buf[*offset], body_size);
+ tpm_buf_append(tbuf, &buf[*offset], body_size);
- rc = tpm_transmit_cmd(chip, &tbuf, 4, NULL);
+ rc = tpm_transmit_cmd(chip, tbuf, 4, NULL);
if (rc < 0) {
dev_warn(&chip->dev, "%s: failed with a system error %d\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
rc == TPM2_RC_REFERENCE_H0) {
@@ -103,64 +104,55 @@ int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
* flushed outside the space
*/
*handle = 0;
- tpm_buf_destroy(&tbuf);
return -ENOENT;
} else if (tpm2_rc_value(rc) == TPM2_RC_INTEGRITY) {
- tpm_buf_destroy(&tbuf);
return -EINVAL;
} else if (rc > 0) {
dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
}
- *handle = be32_to_cpup((__be32 *)&tbuf.data[TPM_HEADER_SIZE]);
+ *handle = be32_to_cpup((__be32 *)&tbuf->data[TPM_HEADER_SIZE]);
*offset += body_size;
-
- tpm_buf_destroy(&tbuf);
return 0;
}
int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
unsigned int buf_size, unsigned int *offset)
{
- struct tpm_buf tbuf;
unsigned int body_size;
int rc;
- rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
- if (rc)
- return rc;
+ struct tpm_buf *tbuf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tbuf)
+ return -ENOMEM;
- tpm_buf_append_u32(&tbuf, handle);
+ tpm_buf_init(tbuf, TPM_BUFSIZE);
+ tpm_buf_reset(tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
+ tpm_buf_append_u32(tbuf, handle);
- rc = tpm_transmit_cmd(chip, &tbuf, 0, NULL);
+ rc = tpm_transmit_cmd(chip, tbuf, 0, NULL);
if (rc < 0) {
dev_warn(&chip->dev, "%s: failed with a system error %d\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
} else if (tpm2_rc_value(rc) == TPM2_RC_REFERENCE_H0) {
- tpm_buf_destroy(&tbuf);
return -ENOENT;
} else if (rc) {
dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
}
- body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
+ body_size = tpm_buf_length(tbuf) - TPM_HEADER_SIZE;
if ((*offset + body_size) > buf_size) {
dev_warn(&chip->dev, "%s: out of backing storage\n", __func__);
- tpm_buf_destroy(&tbuf);
return -ENOMEM;
}
- memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
+ memcpy(&buf[*offset], &tbuf->data[TPM_HEADER_SIZE], body_size);
*offset += body_size;
- tpm_buf_destroy(&tbuf);
return 0;
}
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 0818bb517805..682dfc93845d 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -395,40 +395,36 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip *chip, u8 status)
static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
{
- struct tpm_buf buf;
int rc;
const struct tpm_header *header;
struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
if (chip->flags & TPM_CHIP_FLAG_TPM2)
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
- TPM2_CC_SET_LOCALITY);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_SET_LOCALITY);
else
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
- TPM_ORD_SET_LOCALITY);
- if (rc)
- return rc;
- tpm_buf_append_u8(&buf, locality);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SET_LOCALITY);
+
+ tpm_buf_append_u8(buf, locality);
proxy_dev->state |= STATE_DRIVER_COMMAND;
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to set locality");
+ rc = tpm_transmit_cmd(chip, buf, 0, "attempting to set locality");
proxy_dev->state &= ~STATE_DRIVER_COMMAND;
- if (rc < 0) {
- locality = rc;
- goto out;
- }
+ if (rc < 0)
+ return rc;
- header = (const struct tpm_header *)buf.data;
+ header = (const struct tpm_header *)buf->data;
rc = be32_to_cpu(header->return_code);
if (rc)
locality = -1;
-out:
- tpm_buf_destroy(&buf);
-
return locality;
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 6d169c8976d8..e68995df8796 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -25,7 +25,8 @@
#include <crypto/hash_info.h>
#include <crypto/aes.h>
-#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
+#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
+#define TPM_BUFSIZE 4096
/*
* SHA-512 is, as of today, the largest digest in the TCG algorithm repository.
@@ -389,12 +390,14 @@ enum tpm_buf_flags {
};
/*
- * A string buffer type for constructing TPM commands.
+ * A buffer for constructing and parsing TPM commands, responses and sized
+ * (TPM2B) buffers.
*/
struct tpm_buf {
- u32 flags;
- u32 length;
- u8 *data;
+ u8 flags;
+ u16 length;
+ u16 capacity;
+ u8 data[];
};
enum tpm2_object_attributes {
@@ -425,12 +428,11 @@ struct tpm2_hash {
unsigned int tpm_id;
};
-int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal);
+void tpm_buf_init(struct tpm_buf *buf, u16 buf_size);
+void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size);
void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal);
-int tpm_buf_init_sized(struct tpm_buf *buf);
void tpm_buf_reset_sized(struct tpm_buf *buf);
-void tpm_buf_destroy(struct tpm_buf *buf);
-u32 tpm_buf_length(struct tpm_buf *buf);
+u16 tpm_buf_length(struct tpm_buf *buf);
void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length);
void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value);
void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value);
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 3d75bb6f9689..368e420c18af 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -320,9 +320,10 @@ static int TSS_checkhmac2(unsigned char *buffer,
* For key specific tpm requests, we will generate and send our
* own TPM command packets using the drivers send function.
*/
-static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
+static int trusted_tpm_send(void *cmd, size_t cmd_len)
{
- struct tpm_buf buf;
+ u8 buf_data[512];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
int rc;
if (!chip)
@@ -332,11 +333,10 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
if (rc)
return rc;
- buf.flags = 0;
- buf.length = buflen;
- buf.data = cmd;
+ tpm_buf_init(buf, sizeof(buf_data));
+ tpm_buf_append(buf, cmd, cmd_len);
dump_tpm_buf(cmd);
- rc = tpm_transmit_cmd(chip, &buf, 4, "sending data");
+ rc = tpm_transmit_cmd(chip, buf, 4, "sending data");
dump_tpm_buf(cmd);
if (rc > 0)
@@ -622,23 +622,23 @@ static int tpm_unseal(struct tpm_buf *tb,
static int key_seal(struct trusted_key_payload *p,
struct trusted_key_options *o)
{
- struct tpm_buf tb;
int ret;
- ret = tpm_buf_init(&tb, 0, 0);
- if (ret)
- return ret;
+ struct tpm_buf *tb __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tb)
+ return -ENOMEM;
+
+ tpm_buf_init(tb, TPM_BUFSIZE);
/* include migratable flag at end of sealed key */
p->key[p->key_len] = p->migratable;
- ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
+ ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth,
p->key, p->key_len + 1, p->blob, &p->blob_len,
o->blobauth, o->pcrinfo, o->pcrinfo_len);
if (ret < 0)
pr_info("srkseal failed (%d)\n", ret);
- tpm_buf_destroy(&tb);
return ret;
}
@@ -648,14 +648,15 @@ static int key_seal(struct trusted_key_payload *p,
static int key_unseal(struct trusted_key_payload *p,
struct trusted_key_options *o)
{
- struct tpm_buf tb;
int ret;
- ret = tpm_buf_init(&tb, 0, 0);
- if (ret)
- return ret;
+ struct tpm_buf *tb __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tb)
+ return -ENOMEM;
+
+ tpm_buf_init(tb, TPM_BUFSIZE);
- ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
+ ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
o->blobauth, p->key, &p->key_len);
if (ret < 0)
pr_info("srkunseal failed (%d)\n", ret);
@@ -663,7 +664,6 @@ static int key_unseal(struct trusted_key_payload *p,
/* pull migratable flag out of sealed key */
p->migratable = p->key[--p->key_len];
- tpm_buf_destroy(&tb);
return ret;
}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 6fcff1066873..4d8d8705cbbc 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -205,7 +205,6 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
{
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;
@@ -233,98 +232,100 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (rc)
goto out_put;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- if (rc) {
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf) {
+ rc = -ENOMEM;
tpm2_end_auth_session(chip);
goto out_put;
}
- rc = tpm_buf_init_sized(&sized);
- if (rc) {
- tpm_buf_destroy(&buf);
- tpm2_end_auth_session(chip);
- goto out_put;
- }
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
parent_name_size);
if (rc)
- goto out;
+ goto out_put;
- tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
+ tpm_buf_append_hmac_session(chip, buf, TPM2_SA_DECRYPT,
options->keyauth, TPM_DIGEST_SIZE);
+ struct tpm_buf *sized __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!sized) {
+ rc = -ENOMEM;
+ tpm2_end_auth_session(chip);
+ goto out_put;
+ }
+
/* sensitive */
- tpm_buf_append_u16(&sized, options->blobauth_len);
+ tpm_buf_init_sized(sized, TPM_BUFSIZE);
+ tpm_buf_append_u16(sized, options->blobauth_len);
if (options->blobauth_len)
- tpm_buf_append(&sized, options->blobauth, options->blobauth_len);
+ tpm_buf_append(sized, options->blobauth, options->blobauth_len);
- tpm_buf_append_u16(&sized, payload->key_len);
- tpm_buf_append(&sized, payload->key, payload->key_len);
- tpm_buf_append(&buf, sized.data, sized.length);
+ tpm_buf_append_u16(sized, payload->key_len);
+ tpm_buf_append(sized, payload->key, payload->key_len);
+ tpm_buf_append(buf, sized->data, sized->length);
/* public */
- tpm_buf_reset_sized(&sized);
- tpm_buf_append_u16(&sized, TPM_ALG_KEYEDHASH);
- tpm_buf_append_u16(&sized, hash);
+ tpm_buf_init_sized(sized, TPM_BUFSIZE);
+ tpm_buf_append_u16(sized, TPM_ALG_KEYEDHASH);
+ tpm_buf_append_u16(sized, hash);
/* key properties */
flags = 0;
flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT);
- tpm_buf_append_u32(&sized, flags);
+ tpm_buf_append_u32(sized, flags);
/* policy */
- tpm_buf_append_u16(&sized, options->policydigest_len);
+ tpm_buf_append_u16(sized, options->policydigest_len);
if (options->policydigest_len)
- tpm_buf_append(&sized, options->policydigest, options->policydigest_len);
+ tpm_buf_append(sized, options->policydigest, options->policydigest_len);
/* public parameters */
- tpm_buf_append_u16(&sized, TPM_ALG_NULL);
- tpm_buf_append_u16(&sized, 0);
+ tpm_buf_append_u16(sized, TPM_ALG_NULL);
+ tpm_buf_append_u16(sized, 0);
- tpm_buf_append(&buf, sized.data, sized.length);
+ tpm_buf_append(buf, sized->data, sized->length);
/* outside info */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* creation PCR */
- tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u32(buf, 0);
- if (buf.flags & TPM_BUF_INVALID) {
+ if (buf->flags & TPM_BUF_INVALID) {
rc = -E2BIG;
tpm2_end_auth_session(chip);
goto out;
}
- rc = tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, buf);
if (rc)
goto out;
- rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+ rc = tpm_transmit_cmd(chip, buf, 4, "sealing data");
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
if (rc)
goto out;
- blob_len = tpm_buf_read_u32(&buf, &offset);
- if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_INVALID) {
+ blob_len = tpm_buf_read_u32(buf, &offset);
+ if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_INVALID) {
rc = -E2BIG;
goto out;
}
- if (buf.length - offset < blob_len) {
+ if (buf->length - offset < blob_len) {
rc = -EFAULT;
goto out;
}
- blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+ blob_len = tpm2_key_encode(payload, options, &buf->data[offset], blob_len);
if (blob_len < 0)
rc = blob_len;
out:
- tpm_buf_destroy(&sized);
- tpm_buf_destroy(&buf);
-
if (!rc)
payload->blob_len = blob_len;
@@ -356,7 +357,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
u32 *blob_handle)
{
u8 *blob_ref __free(kfree) = NULL;
- struct tpm_buf buf;
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
@@ -396,40 +396,39 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
if (rc)
return rc;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
- if (rc) {
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf) {
tpm2_end_auth_session(chip);
- return rc;
+ return -ENOMEM;
}
- rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
parent_name_size);
if (rc)
- goto out;
+ return rc;
- tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
+ tpm_buf_append_hmac_session(chip, buf, 0, options->keyauth,
TPM_DIGEST_SIZE);
- tpm_buf_append(&buf, blob, blob_len);
+ tpm_buf_append(buf, blob, blob_len);
- if (buf.flags & TPM_BUF_INVALID) {
- rc = -E2BIG;
+ if (buf->flags & TPM_BUF_INVALID) {
tpm2_end_auth_session(chip);
- goto out;
+ return -E2BIG;
}
- rc = tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, buf);
if (rc)
- goto out;
+ return rc;
- rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+ rc = tpm_transmit_cmd(chip, buf, 4, "loading blob");
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
if (!rc)
*blob_handle = be32_to_cpup(
- (__be32 *) &buf.data[TPM_HEADER_SIZE]);
-
-out:
- tpm_buf_destroy(&buf);
+ (__be32 *)&buf->data[TPM_HEADER_SIZE]);
return tpm_ret_to_err(rc);
}
@@ -454,28 +453,28 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
u16 parent_name_size,
u32 blob_handle)
{
- struct tpm_buf buf;
u16 data_len;
u8 *data;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
rc = tpm2_start_auth_session(chip);
if (rc)
return rc;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- if (rc) {
- tpm2_end_auth_session(chip);
- return rc;
- }
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- rc = tpm_buf_append_name(chip, &buf, options->keyhandle, parent_name,
+ rc = tpm_buf_append_name(chip, buf, options->keyhandle, parent_name,
parent_name_size);
if (rc)
- goto out;
+ return rc;
if (!options->policyhandle) {
- tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
+ tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT,
options->blobauth,
options->blobauth_len);
} else {
@@ -490,37 +489,33 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
* could repeat our actions with the exfiltrated
* password.
*/
- 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);
+ 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);
+ tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT, NULL, 0);
}
- rc = tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, buf);
if (rc)
- goto out;
+ return rc;
- rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+ rc = tpm_transmit_cmd(chip, buf, 6, "unsealing");
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
if (!rc) {
data_len = be16_to_cpup(
- (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
- if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE) {
- rc = -EFAULT;
- goto out;
- }
+ (__be16 *)&buf->data[TPM_HEADER_SIZE + 4]);
+ if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE)
+ return -EFAULT;
- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
- rc = -EFAULT;
- goto out;
- }
- data = &buf.data[TPM_HEADER_SIZE + 6];
+ if (tpm_buf_length(buf) < TPM_HEADER_SIZE + 6 + data_len)
+ return -EFAULT;
+ data = &buf->data[TPM_HEADER_SIZE + 6];
if (payload->old_format) {
/* migratable flag is at the end of the key */
@@ -537,8 +532,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
}
}
-out:
- tpm_buf_destroy(&buf);
return tpm_ret_to_err(rc);
}
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2025-12-16 11:01 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CANpmjNNm-kbTw46Wh1BJudynHOeLn-Oxew8VuAnCppvV_WtyBw@mail.gmail.com>
On Mon, Dec 15, 2025 at 04:53PM +0100, Marco Elver wrote:
[..]
> > > So I think as is, we can start. But I really do want the cleanup thing
> > > sorted, even if just with that __release_on_cleanup mashup or so.
> >
> > Working on rebasing this to v6.19-rc1 and saw this new scoped seqlock
> > abstraction. For that one I was able to make it work like I thought we
> > could (below). Some awkwardness is required to make it work in
> > for-loops, which only let you define variables with the same type.
> >
> > For <linux/cleanup.h> it needs some more thought due to extra levels of
> > indirection.
>
> For cleanup.h, the problem is that to instantiate we use
> "guard(class)(args..)". If it had been designed as "guard(class,
> args...)", i.e. just use __VA_ARGS__ explicitly instead of the
> implicit 'args...', it might have been possible to add a second
> cleanup variable to do the same (with some additional magic to extract
> the first arg if one exists). Unfortunately, the use of the current
> guard()() idiom has become so pervasive that this is a bigger
> refactor. I'm going to leave cleanup.h as-is for now, if we think we
> want to give this a go in the current state.
Alright, this can work, but it's not that ergonomic as I'd hoped (see
below): we can redefine class_<name>_constructor to append another
cleanup variable. With enough documentation, this might be workable.
WDYT?
------ >8 ------
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 2f998bb42c4c..b47a1ba57e8e 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -518,7 +518,10 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T) _unlock;
#define DECLARE_LOCK_GUARD_1_ATTRS(_name, _lock, _unlock) \
static inline class_##_name##_t class_##_name##_constructor(lock_##_name##_t *_T) _lock;\
-static inline void class_##_name##_destructor(class_##_name##_t *_T) _unlock;
+static __always_inline void __class_##_name##_cleanup_ctx(class_##_name##_t **_T) \
+ __no_context_analysis _unlock {}
+#define WITH_LOCK_GUARD_1_ATTRS(_name, _T) class_##_name##_constructor(_T), \
+ *__UNIQUE_ID(cleanup_ctx) __cleanup(__class_##_name##_cleanup_ctx) = (void *)(_T)
#define DEFINE_LOCK_GUARD_1(_name, _type, _lock, _unlock, ...) \
__DEFINE_CLASS_IS_CONDITIONAL(_name, false); \
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index 8ed48d40007b..06c3f947ea49 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -255,9 +255,12 @@ DEFINE_LOCK_GUARD_1(mutex, struct mutex, mutex_lock(_T->lock), mutex_unlock(_T->
DEFINE_LOCK_GUARD_1_COND(mutex, _try, mutex_trylock(_T->lock))
DEFINE_LOCK_GUARD_1_COND(mutex, _intr, mutex_lock_interruptible(_T->lock), _RET == 0)
-DECLARE_LOCK_GUARD_1_ATTRS(mutex, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(mutex_try, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(mutex_intr, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(mutex, __acquires(_T), __releases(*(struct mutex **)_T))
+DECLARE_LOCK_GUARD_1_ATTRS(mutex_try, __acquires(_T), __releases(*(struct mutex **)_T))
+DECLARE_LOCK_GUARD_1_ATTRS(mutex_intr, __acquires(_T), __releases(*(struct mutex **)_T))
+#define class_mutex_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex, _T)
+#define class_mutex_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex_try, _T)
+#define class_mutex_intr_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex_intr, _T)
extern unsigned long mutex_get_owner(struct mutex *lock);
^ permalink raw reply related
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-16 11:08 UTC (permalink / raw)
To: Sumit Garg
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: <CAGptzHOOqLhBnAXDURAzkgckUvRr__UuF1S_7MLV0u-ZxYEdyA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2840 bytes --]
Hello,
On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> > On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > > 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.
Don't consider me in for that. But it sounds like a nice addition.
> > 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.
So only OP-TEE uses the bus for devices and the other *-TEE don't. Also
sounds like something worth to be fixed.
> > 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.
From reading that once I don't understand it. (But no need to explain
:-)
Still the file should better be added before device_add() is called.
> > - 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.
Without understanding the tee stuff, I'd say: Don't bother and only undo
the things that probe did before the failure.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Peter Zijlstra @ 2025-12-16 12:23 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <CANpmjNNm-kbTw46Wh1BJudynHOeLn-Oxew8VuAnCppvV_WtyBw@mail.gmail.com>
On Mon, Dec 15, 2025 at 04:53:18PM +0100, Marco Elver wrote:
> One observation from the rebase: Generally synchronization primitives
> do not change much and the annotations are relatively stable, but e.g.
> RCU & sched (latter is optional and depends on the sched-enablement
> patch) receive disproportionally more changes, and while new
> annotations required for v6.19-rc1 were trivial, it does require
> compiling with a Clang version that does produce the warnings to
> notice.
I have:
Debian clang version 22.0.0 (++20251023025710+3f47a7be1ae6-1~exp5)
I've not tried if that is new enough.
> While Clang 22-dev is being tested on CI, I doubt maintainers already
> use it, so it's possible we'll see some late warnings due to missing
> annotations when things hit -next. This might be an acceptable churn
> cost, if we think the outcome is worthwhile. Things should get better
> when Clang 22 is released properly, but until then things might be a
> little bumpy if there are large changes across the core
> synchronization primitives.
Yeah, we'll see how bad it gets, we can always disable it for
COMPILE_TEST or so for a while.
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Peter Zijlstra @ 2025-12-16 12:32 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <aUAPbFJSv0alh_ix@elver.google.com>
On Mon, Dec 15, 2025 at 02:38:52PM +0100, Marco Elver wrote:
> Working on rebasing this to v6.19-rc1 and saw this new scoped seqlock
> abstraction. For that one I was able to make it work like I thought we
> could (below). Some awkwardness is required to make it work in
> for-loops, which only let you define variables with the same type.
>
> diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
> index b5563dc83aba..5162962b4b26 100644
> --- a/include/linux/seqlock.h
> +++ b/include/linux/seqlock.h
> @@ -1249,6 +1249,7 @@ struct ss_tmp {
> };
>
> static __always_inline void __scoped_seqlock_cleanup(struct ss_tmp *sst)
> + __no_context_analysis
> {
> if (sst->lock)
> spin_unlock(sst->lock);
> @@ -1278,6 +1279,7 @@ extern void __scoped_seqlock_bug(void);
>
> static __always_inline void
> __scoped_seqlock_next(struct ss_tmp *sst, seqlock_t *lock, enum ss_state target)
> + __no_context_analysis
> {
> switch (sst->state) {
> case ss_done:
> @@ -1320,9 +1322,18 @@ __scoped_seqlock_next(struct ss_tmp *sst, seqlock_t *lock, enum ss_state target)
> }
> }
>
> +/*
> + * Context analysis helper to release seqlock at the end of the for-scope; the
> + * alias analysis of the compiler will recognize that the pointer @s is is an
> + * alias to @_seqlock passed to read_seqbegin(_seqlock) below.
> + */
> +static __always_inline void __scoped_seqlock_cleanup_ctx(struct ss_tmp **s)
> + __releases_shared(*((seqlock_t **)s)) __no_context_analysis {}
> +
> #define __scoped_seqlock_read(_seqlock, _target, _s) \
> for (struct ss_tmp _s __cleanup(__scoped_seqlock_cleanup) = \
> - { .state = ss_lockless, .data = read_seqbegin(_seqlock) }; \
> + { .state = ss_lockless, .data = read_seqbegin(_seqlock) }, \
> + *__UNIQUE_ID(ctx) __cleanup(__scoped_seqlock_cleanup_ctx) = (struct ss_tmp *)_seqlock; \
> _s.state != ss_done; \
> __scoped_seqlock_next(&_s, _seqlock, _target))
>
I am ever so confused.. where is the __acquire_shared(), in read_seqbegin() ?
Also, why do we need this second variable with cleanup; can't the
existing __scoped_seqlock_cleanup() get the __releases_shared()
attribute?
^ permalink raw reply
* Re: [PATCH V2 1/1] IMA event log trimming
From: Mimi Zohar @ 2025-12-16 12:50 UTC (permalink / raw)
To: steven chen, linux-integrity
Cc: roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet, serge,
paul, jmorris, linux-security-module, anirudhve, gregorylumen,
nramas, sushring, linux-doc
In-Reply-To: <d80958ec-f139-41e9-afa0-a5aca94221de@linux.microsoft.com>
Hi Steven,
As I previously said, "The main difference between this patch and Roberto's
version is the length of time needed for locking the measurement list, which
prevents new entries from being appended to the measurement list. In Roberto's
version, the list head is moved quickly and the lock released. Measuring the
total amount of time needed to trim the measurement list ignores the benefit of
his version. I plan on reviewing both this version and his (hopefully today)."
The other difference is "when" the IMA measurement list is read and saved,
before the trigger to trim the measurement list or after when the measurement
list is staged. In this case, the initial trigger trims the measurement list.
In the other case, the measurement list is staged and then deleted. When
reviewing Roberto's patch, I plan to discuss it.
After trimming the measurement list, existing verifiers, which walk the IMA
measurement list, will obviously fail to match the PCRs. Breaking existing
userspace applications is a problem and, unfortunately, requires yet another
Kconfig option. It needs to be at least mentioned here in the patch
description.
There are two places where it says, "the list never shrinks, so we don't need a
lock here". Either the code, the comment, or both need to be updated.
On Thu, 2025-12-11 at 10:41 -0800, steven chen wrote:
> On 12/10/2025 3:53 PM, steven chen wrote:
> > This patch is for trimming N entries of the IMA event logs. It will also
> > cleaning the hash table if ima_flush_htable is set.
Please refer to "Describe your changes in imperative mood" in the "Describe your
changes" section of Documentation/process/submitting-patches.rst.
> >
> > It provides a userspace interface ima_trim_log that can be used to input
> > number N to let kernel to trim N entries of IMA event logs. When read
There is only a single kernel measurement list or event log, not plural. There
are N number of "entries" or "records" in the IMA measurement list.
-> trim N records from the IMA measurement list.
> > this interface, it returns number of entries trimmed last time.
Please provide an example of how to initiate the trim.
After trimming the IMA measurement list, are the other securityfs files correct?
Are they correct after a kexec? Or are they reset without a way of resurrecting
them without the full measurement list?
> >
> > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > ---
> > .../admin-guide/kernel-parameters.txt | 4 +
> > security/integrity/ima/ima.h | 2 +
> > security/integrity/ima/ima_fs.c | 175 +++++++++++++++++-
> > security/integrity/ima/ima_queue.c | 64 +++++++
> > 4 files changed, 241 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index e92c0056e4e0..cd1a1d0bf0e2 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2197,6 +2197,10 @@
> > Use the canonical format for the binary runtime
> > measurements, instead of host native format.
> >
> > + ima_flush_htable [IMA]
> > + Flush the measurement list hash table when trim all
> > + or a part of it for deletion.
> > +
> > ima_hash= [IMA]
> > Format: { md5 | sha1 | rmd160 | sha256 | sha384
> > | sha512 | ... }
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index e3d71d8d56e3..ab0e30ee25ea 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
> >
> > #ifdef CONFIG_IMA_KEXEC
> > void ima_measure_kexec_event(const char *event_name);
> > +long ima_purge_event_log(long number_logs);
> > #else
> > static inline void ima_measure_kexec_event(const char *event_name) {}
> > +static inline long ima_purge_event_log(long number_logs) { return 0; }
> > #endif
> >
> > /*
> > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > index 87045b09f120..410f7d03c43f 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -21,6 +21,9 @@
> > #include <linux/rcupdate.h>
> > #include <linux/parser.h>
> > #include <linux/vmalloc.h>
> > +#include <linux/ktime.h>
> > +#include <linux/timekeeping.h>
> > +#include <linux/ima.h>
> >
> > #include "ima.h"
> >
> > @@ -38,6 +41,14 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
> >
> > static int valid_policy = 1;
> >
> > +#define IMA_LOG_TRIM_REQ_LENGTH 11
> > +#define IMA_LOG_TRIM_EVENT_LEN 256
> > +
> > +static long trimcount;
> > +/* mutex protects atomicity of trimming measurement list requests */
ima_measure_lock is taken for more than just synchronization of trimming the
measurement list. Please update comment.
> > +static DEFINE_MUTEX(ima_measure_lock);
> > +static long ima_measure_users;
> > +
> > static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> > loff_t *ppos, atomic_long_t *val)
> > {
> > @@ -202,16 +213,65 @@ static const struct seq_operations ima_measurments_seqops = {
> > .show = ima_measurements_show
> > };
> >
_ima_measurements_open() seems pretty fundamental to the locking scheme.
Preventing opening the IMA measurement list is new. There should at least be a
short, regular comment explaining what you're locking and why.
> > +static int _ima_measurements_open(struct inode *inode, struct file *file,
> > + const struct seq_operations *seq_ops)
> > +{
> > + bool write = !!(file->f_mode & FMODE_WRITE);
> > + int ret;
> > +
> > + if (write && !capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > +
> > + mutex_lock(&ima_measure_lock);
> > + if ((write && ima_measure_users != 0) ||
> > + (!write && ima_measure_users < 0)) {
> > + mutex_unlock(&ima_measure_lock);
> > + return -EBUSY;
> > + }
> > +
> > + ret = seq_open(file, seq_ops);
> > + if (ret < 0) {
> > + mutex_unlock(&ima_measure_lock);
> > + return ret;
> > + }
> > +
> > + if (write)
> > + ima_measure_users--;
> > + else
> > + ima_measure_users++;
> > +
> > + mutex_unlock(&ima_measure_lock);
> > + return ret;
> > +}
> > +
> > static int ima_measurements_open(struct inode *inode, struct file *file)
> > {
> > - return seq_open(file, &ima_measurments_seqops);
> > + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> > +}
> > +
> > +static int ima_measurements_release(struct inode *inode, struct file *file)
> > +{
> > + bool write = !!(file->f_mode & FMODE_WRITE);
> > + int ret;
> > +
> > + mutex_lock(&ima_measure_lock);
> > + ret = seq_release(inode, file);
> > + if (!ret) {
> > + if (write)
> > + ima_measure_users++;
> > + else
> > + ima_measure_users--;
> > + }
> > +
> > + mutex_unlock(&ima_measure_lock);
> > + return ret;
> > }
> >
> > static const struct file_operations ima_measurements_ops = {
> > .open = ima_measurements_open,
> > .read = seq_read,
> > .llseek = seq_lseek,
> > - .release = seq_release,
> > + .release = ima_measurements_release,
> > };
> >
> > void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
> > @@ -279,14 +339,111 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
> >
> > static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
> > {
> > - return seq_open(file, &ima_ascii_measurements_seqops);
> > + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
> > }
> >
> > static const struct file_operations ima_ascii_measurements_ops = {
> > .open = ima_ascii_measurements_open,
> > .read = seq_read,
> > .llseek = seq_lseek,
> > - .release = seq_release,
> > + .release = ima_measurements_release,
> > +};
> > +
> > +static void ima_measure_trim_event(const long number_logs)
> > +{
> > + char ima_log_trim_event[IMA_LOG_TRIM_EVENT_LEN];
> > + struct timespec64 ts;
> > + u64 time_ns;
> > + int n;
> > +
> > + ktime_get_real_ts64(&ts);
> > + time_ns = (u64)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
> > + n = scnprintf(ima_log_trim_event, IMA_LOG_TRIM_EVENT_LEN,
> > + "time=%llu; log trim this time=%lu;",
> > + time_ns, number_logs);
> > +
> > + ima_measure_critical_data("ima_log_trim", "trim ima event logs", ima_log_trim_event, n, false, NULL, 0);
> > +}
There's no mention of a new critical data record in the patch description. It
should be a separate patch with a full patch description describing what it is
and how to verify it.
> > +
> > +static int ima_log_trim_open(struct inode *inode, struct file *file)
> > +{
> > + bool write = !!(file->f_mode & FMODE_WRITE);
> > +
> > + if (!write && capable(CAP_SYS_ADMIN))
> > + return 0;
> > + else if (!capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > +
> > + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> > +}
> > +
> > +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
> > +{
> > + char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
> > + ssize_t len;
> > +
> > + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
> > + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
> > +}
> > +
> > +static ssize_t ima_log_trim_write(struct file *file,
> > + const char __user *buf, size_t datalen, loff_t *ppos)
> > +{
> > + unsigned char req[IMA_LOG_TRIM_REQ_LENGTH];
> > + long count, n;
> > + int ret;
> > +
> > + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + n = (int)datalen;
> > +
> > + ret = copy_from_user(req, buf, datalen);
> > + if (ret < 0)
> > + goto out;
> > +
> > + count = 0;
> > + for (int i = 0; i < n; ++i) {
> > + if (req[i] < '0' || req[i] > '9') {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + count = count * 10 + req[i] - '0';
> > + }
This code can be simplified by using the kstrto*_from_user() family of
functions. The patch description should include an example how to trim the
measurement list.
> > + ret = ima_purge_event_log(count);
The term "purge" is really strong wording. I hope you're not purging the
measurement list, but simply removing them from kernel memory.
> > +
> > + if (ret < 0)
> > + goto out;
> > +
> > + trimcount = ret;
> > +
> > + if (trimcount > 0)
> > + ima_measure_trim_event(trimcount);
> > +
> > + ret = datalen;
> > +out:
> > + return ret;
> > +}
> > +
> > +static int ima_log_trim_release(struct inode *inode, struct file *file)
> > +{
> > + bool write = !!(file->f_mode & FMODE_WRITE);
> > + if (!write && capable(CAP_SYS_ADMIN))
> > + return 0;
> > + else if (!capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > +
> > + return ima_measurements_release(inode, file);
> > +}
> > +
> > +static const struct file_operations ima_log_trim_ops = {
> > + .open = ima_log_trim_open,
> > + .read = ima_log_trim_read,
> > + .write = ima_log_trim_write,
> > + .llseek = generic_file_llseek,
> > + .release = ima_log_trim_release
> > };
> >
> > static ssize_t ima_read_policy(char *path)
> > @@ -528,6 +685,16 @@ int __init ima_fs_init(void)
> > goto out;
> > }
> >
> > + dentry = securityfs_create_file("ima_trim_log",
> > + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
> > + ima_dir, NULL, &ima_log_trim_ops);
> > + if (IS_ERR(dentry)) {
> > + ret = PTR_ERR(dentry);
> > + goto out;
> > + }
> > +
> > + trimcount = 0;
> > +
> > dentry = securityfs_create_file("runtime_measurements_count",
> > S_IRUSR | S_IRGRP, ima_dir, NULL,
> > &ima_measurements_count_ops);
> > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> > index 590637e81ad1..77ab52469727 100644
> > --- a/security/integrity/ima/ima_queue.c
> > +++ b/security/integrity/ima/ima_queue.c
> > @@ -22,6 +22,14 @@
> >
> > #define AUDIT_CAUSE_LEN_MAX 32
> >
> > +bool ima_flush_htable;
> > +static int __init ima_flush_htable_setup(char *str)
> > +{
> > + ima_flush_htable = true;
> > + return 1;
> > +}
> > +__setup("ima_flush_htable", ima_flush_htable_setup);
> > +
> > /* pre-allocated array of tpm_digest structures to extend a PCR */
> > static struct tpm_digest *digests;
> >
> > @@ -220,6 +228,62 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> > return result;
> > }
> >
> > +/* Delete the IMA event logs */
Do you mean delete the IMA event records/entries?
> > +long ima_purge_event_log(long number_logs)
If this function is not defined as static, then it requires a kernel-doc.
> > +{
> > + struct ima_queue_entry *qe, *qe_tmp;
> > + LIST_HEAD(ima_measurements_staged);
> > + unsigned int i;
> > + long cur = number_logs;
The variable name "number_logs" is confusing. As I mentioned in the patch
description, there is one measurement list with multiple records. There aren't
multiple logs in the kernel (other than the staged list).
> > +
> > + if (number_logs <= 0)
> > + return number_logs;
> > +
> > + mutex_lock(&ima_extend_list_mutex);
> > +
> > +
> > + list_for_each_entry(qe, &ima_measurements, later) {
> > + if (--number_logs == 0)
> > + break;
> > + }
> > +
> > + if (number_logs > 0) {
> > + mutex_unlock(&ima_extend_list_mutex);
> > + return -ENOENT;
> > + }
> > +
> > + __list_cut_position(&ima_measurements_staged, &ima_measurements,
> > + &qe->later);
> > + atomic_long_sub(cur, &ima_htable.len);
> > +
> > + if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable) {
> > + list_for_each_entry(qe, &ima_measurements_staged, later)
> > + /* It can race with ima_lookup_digest_entry(). */
> > + hlist_del_rcu(&qe->hnext);
> > + }
>
> If the h table can be staged during the locking period and deleted after
> unlocking, the time
> the list is held will be reduced.
>
> I will work on this, and any suggestions are greatly appreciated.
>
> Thanks,
>
> Steven
>
> > +
> > + mutex_unlock(&ima_extend_list_mutex);
> > +
> > +
> > + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
> > + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
> > + kfree(qe->entry->template_data[i].data);
> > + qe->entry->template_data[i].data = NULL;
> > + qe->entry->template_data[i].len = 0;
> > + }
> > +
> > + list_del(&qe->later);
> > +
> > + if (ima_flush_htable) {
> > + kfree(qe->entry->digests);
> > + kfree(qe->entry);
> > + kfree(qe);
> > + }
> > + }
To avoid code duplication, there's a similar function named
ima_free_template_entry(). Any changes needed to the function should be done as
a preparatory patch.
> > +
> > + return cur;
> > +}
> > +
> > int ima_restore_measurement_entry(struct ima_template_entry *entry)
> > {
> > int result = 0;
--
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2025-12-16 13:23 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251216123211.GT3707837@noisy.programming.kicks-ass.net>
On Tue, Dec 16, 2025 at 01:32PM +0100, Peter Zijlstra wrote:
> On Mon, Dec 15, 2025 at 02:38:52PM +0100, Marco Elver wrote:
>
> > Working on rebasing this to v6.19-rc1 and saw this new scoped seqlock
> > abstraction. For that one I was able to make it work like I thought we
> > could (below). Some awkwardness is required to make it work in
> > for-loops, which only let you define variables with the same type.
>
> >
> > diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
> > index b5563dc83aba..5162962b4b26 100644
> > --- a/include/linux/seqlock.h
> > +++ b/include/linux/seqlock.h
> > @@ -1249,6 +1249,7 @@ struct ss_tmp {
> > };
> >
> > static __always_inline void __scoped_seqlock_cleanup(struct ss_tmp *sst)
> > + __no_context_analysis
> > {
> > if (sst->lock)
> > spin_unlock(sst->lock);
> > @@ -1278,6 +1279,7 @@ extern void __scoped_seqlock_bug(void);
> >
> > static __always_inline void
> > __scoped_seqlock_next(struct ss_tmp *sst, seqlock_t *lock, enum ss_state target)
> > + __no_context_analysis
> > {
> > switch (sst->state) {
> > case ss_done:
> > @@ -1320,9 +1322,18 @@ __scoped_seqlock_next(struct ss_tmp *sst, seqlock_t *lock, enum ss_state target)
> > }
> > }
> >
> > +/*
> > + * Context analysis helper to release seqlock at the end of the for-scope; the
> > + * alias analysis of the compiler will recognize that the pointer @s is is an
> > + * alias to @_seqlock passed to read_seqbegin(_seqlock) below.
> > + */
> > +static __always_inline void __scoped_seqlock_cleanup_ctx(struct ss_tmp **s)
> > + __releases_shared(*((seqlock_t **)s)) __no_context_analysis {}
> > +
> > #define __scoped_seqlock_read(_seqlock, _target, _s) \
> > for (struct ss_tmp _s __cleanup(__scoped_seqlock_cleanup) = \
> > - { .state = ss_lockless, .data = read_seqbegin(_seqlock) }; \
> > + { .state = ss_lockless, .data = read_seqbegin(_seqlock) }, \
> > + *__UNIQUE_ID(ctx) __cleanup(__scoped_seqlock_cleanup_ctx) = (struct ss_tmp *)_seqlock; \
> > _s.state != ss_done; \
> > __scoped_seqlock_next(&_s, _seqlock, _target))
> >
>
> I am ever so confused.. where is the __acquire_shared(), in read_seqbegin() ?
Ah this is just a diff on top of this v4 series. The read_seqbegin()
already had it:
static inline unsigned read_seqbegin(const seqlock_t *sl)
__acquires_shared(sl) __no_context_analysis
{
> Also, why do we need this second variable with cleanup; can't the
> existing __scoped_seqlock_cleanup() get the __releases_shared()
> attribute?
The existing __scoped_seqlock_cleanup() receives &_s (struct ss_tmp *),
and we can't refer to the _seqlock from __scoped_seqlock_cleanup(). Even
if I create a member seqlock_t* ss_tmp::seqlock and initialize it with
_seqlock, the compiler can't track that the member would be an alias of
_seqlock. The function __scoped_seqlock_next() does receive _seqlock to
effectively release it executes for every loop, so there'd be a "lock
imbalance" in the compiler's eyes.
So having the direct alias (even if we cast it to make it work in the
single-statement multi-definition, the compiler doesn't care) is
required for it to work.
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2025-12-16 13:26 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251216122359.GS3707837@noisy.programming.kicks-ass.net>
On Tue, Dec 16, 2025 at 01:23PM +0100, Peter Zijlstra wrote:
> On Mon, Dec 15, 2025 at 04:53:18PM +0100, Marco Elver wrote:
> > One observation from the rebase: Generally synchronization primitives
> > do not change much and the annotations are relatively stable, but e.g.
> > RCU & sched (latter is optional and depends on the sched-enablement
> > patch) receive disproportionally more changes, and while new
> > annotations required for v6.19-rc1 were trivial, it does require
> > compiling with a Clang version that does produce the warnings to
> > notice.
>
> I have:
>
> Debian clang version 22.0.0 (++20251023025710+3f47a7be1ae6-1~exp5)
>
> I've not tried if that is new enough.
That's new enough - it's after
https://github.com/llvm/llvm-project/commit/7ccb5c08f0685d4787f12c3224a72f0650c5865e
which is the minimum required version.
> > While Clang 22-dev is being tested on CI, I doubt maintainers already
> > use it, so it's possible we'll see some late warnings due to missing
> > annotations when things hit -next. This might be an acceptable churn
> > cost, if we think the outcome is worthwhile. Things should get better
> > when Clang 22 is released properly, but until then things might be a
> > little bumpy if there are large changes across the core
> > synchronization primitives.
>
> Yeah, we'll see how bad it gets, we can always disable it for
> COMPILE_TEST or so for a while.
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Peter Zijlstra @ 2025-12-16 13:41 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <aUFdRzx1dxRx1Uqa@elver.google.com>
On Tue, Dec 16, 2025 at 02:23:19PM +0100, Marco Elver wrote:
> > Also, why do we need this second variable with cleanup; can't the
> > existing __scoped_seqlock_cleanup() get the __releases_shared()
> > attribute?
>
> The existing __scoped_seqlock_cleanup() receives &_s (struct ss_tmp *),
> and we can't refer to the _seqlock from __scoped_seqlock_cleanup(). Even
> if I create a member seqlock_t* ss_tmp::seqlock and initialize it with
> _seqlock, the compiler can't track that the member would be an alias of
> _seqlock. The function __scoped_seqlock_next() does receive _seqlock to
> effectively release it executes for every loop, so there'd be a "lock
> imbalance" in the compiler's eyes.
>
> So having the direct alias (even if we cast it to make it work in the
> single-statement multi-definition, the compiler doesn't care) is
> required for it to work.
Right -- it just clicked while I was walking outside. Without actual
inlining it cannot see through the constructor and track the variable :/
OK, let me stare at this more.
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2025-12-16 15:57 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <aUE77hgJa58waFOy@elver.google.com>
On Tue, Dec 16, 2025 at 12:01PM +0100, Marco Elver wrote:
> On Mon, Dec 15, 2025 at 04:53PM +0100, Marco Elver wrote:
> [..]
> > > > So I think as is, we can start. But I really do want the cleanup thing
> > > > sorted, even if just with that __release_on_cleanup mashup or so.
> > >
> > > Working on rebasing this to v6.19-rc1 and saw this new scoped seqlock
> > > abstraction. For that one I was able to make it work like I thought we
> > > could (below). Some awkwardness is required to make it work in
> > > for-loops, which only let you define variables with the same type.
> > >
> > > For <linux/cleanup.h> it needs some more thought due to extra levels of
> > > indirection.
> >
> > For cleanup.h, the problem is that to instantiate we use
> > "guard(class)(args..)". If it had been designed as "guard(class,
> > args...)", i.e. just use __VA_ARGS__ explicitly instead of the
> > implicit 'args...', it might have been possible to add a second
> > cleanup variable to do the same (with some additional magic to extract
> > the first arg if one exists). Unfortunately, the use of the current
> > guard()() idiom has become so pervasive that this is a bigger
> > refactor. I'm going to leave cleanup.h as-is for now, if we think we
> > want to give this a go in the current state.
>
> Alright, this can work, but it's not that ergonomic as I'd hoped (see
> below): we can redefine class_<name>_constructor to append another
> cleanup variable. With enough documentation, this might be workable.
Below is the preview of the complete changes to make the lock guards
work properly.
Thanks,
-- Marco
------ >8 ------
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 2f998bb42c4c..9fe3b0f816c6 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -516,9 +516,42 @@ static __always_inline class_##_name##_t class_##_name##_constructor(void) \
static inline class_##_name##_t class_##_name##_constructor(void) _lock;\
static inline void class_##_name##_destructor(class_##_name##_t *_T) _unlock;
+/*
+ * To support Context Analysis, we need to allow the compiler to see the
+ * acquisition and release of the context lock. However, the "cleanup" helpers
+ * wrap the lock in a struct passed through seperate helper functions, which
+ * hides the lock alias from the compiler (no inter-procedural analysis).
+ *
+ * To make it work, we introduce an explicit alias to the context lock instance
+ * that is "cleaned" up with a separate cleanup helper. This helper is a dummy
+ * function that does nothing at runtime, but has the "_unlock" attribute to
+ * tell the compiler what happens at the end of the scope.
+ *
+ * To generalize the pattern, the WITH_LOCK_GUARD_1_ATTRS() macro should be used
+ * to redefine the constructor, which then also creates the alias variable with
+ * the right "cleanup" attribute, *after* DECLARE_LOCK_GUARD_1_ATTRS() has been
+ * used.
+ *
+ * Example usage:
+ *
+ * DECLARE_LOCK_GUARD_1_ATTRS(mutex, __acquires(_T), __releases(*(struct mutex **)_T))
+ * #define class_mutex_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex, _T)
+ *
+ * Note: To support the for-loop based scoped helpers, the auxiliary variable
+ * must be a pointer to the "class" type because it is defined in the same
+ * statement as the guard variable. However, we initialize it with the lock
+ * pointer (despite the type mismatch, the compiler's alias analysis still works
+ * as expected). The "_unlock" attribute receives a pointer to the auxiliary
+ * variable (a double pointer to the class type), and must be cast and
+ * dereferenced appropriately.
+ */
#define DECLARE_LOCK_GUARD_1_ATTRS(_name, _lock, _unlock) \
static inline class_##_name##_t class_##_name##_constructor(lock_##_name##_t *_T) _lock;\
-static inline void class_##_name##_destructor(class_##_name##_t *_T) _unlock;
+static __always_inline void __class_##_name##_cleanup_ctx(class_##_name##_t **_T) \
+ __no_context_analysis _unlock { }
+#define WITH_LOCK_GUARD_1_ATTRS(_name, _T) \
+ class_##_name##_constructor(_T), \
+ *__UNIQUE_ID(unlock) __cleanup(__class_##_name##_cleanup_ctx) = (void *)(_T)
#define DEFINE_LOCK_GUARD_1(_name, _type, _lock, _unlock, ...) \
__DEFINE_CLASS_IS_CONDITIONAL(_name, false); \
diff --git a/include/linux/local_lock.h b/include/linux/local_lock.h
index bedcbb33b928..99c06e499375 100644
--- a/include/linux/local_lock.h
+++ b/include/linux/local_lock.h
@@ -104,9 +104,13 @@ DEFINE_LOCK_GUARD_1(local_lock_nested_bh, local_lock_t __percpu,
local_lock_nested_bh(_T->lock),
local_unlock_nested_bh(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(local_lock, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(local_lock_irq, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(local_lock_irqsave, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(local_lock_nested_bh, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(local_lock, __acquires(_T), __releases(*(local_lock_t __percpu **)_T))
+#define class_local_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(local_lock, _T)
+DECLARE_LOCK_GUARD_1_ATTRS(local_lock_irq, __acquires(_T), __releases(*(local_lock_t __percpu **)_T))
+#define class_local_lock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(local_lock_irq, _T)
+DECLARE_LOCK_GUARD_1_ATTRS(local_lock_irqsave, __acquires(_T), __releases(*(local_lock_t __percpu **)_T))
+#define class_local_lock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(local_lock_irqsave, _T)
+DECLARE_LOCK_GUARD_1_ATTRS(local_lock_nested_bh, __acquires(_T), __releases(*(local_lock_t __percpu **)_T))
+#define class_local_lock_nested_bh_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(local_lock_nested_bh, _T)
#endif
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index 8ed48d40007b..06c3f947ea49 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -255,9 +255,12 @@ DEFINE_LOCK_GUARD_1(mutex, struct mutex, mutex_lock(_T->lock), mutex_unlock(_T->
DEFINE_LOCK_GUARD_1_COND(mutex, _try, mutex_trylock(_T->lock))
DEFINE_LOCK_GUARD_1_COND(mutex, _intr, mutex_lock_interruptible(_T->lock), _RET == 0)
-DECLARE_LOCK_GUARD_1_ATTRS(mutex, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(mutex_try, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(mutex_intr, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(mutex, __acquires(_T), __releases(*(struct mutex **)_T))
+DECLARE_LOCK_GUARD_1_ATTRS(mutex_try, __acquires(_T), __releases(*(struct mutex **)_T))
+DECLARE_LOCK_GUARD_1_ATTRS(mutex_intr, __acquires(_T), __releases(*(struct mutex **)_T))
+#define class_mutex_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex, _T)
+#define class_mutex_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex_try, _T)
+#define class_mutex_intr_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(mutex_intr, _T)
extern unsigned long mutex_get_owner(struct mutex *lock);
diff --git a/include/linux/rwsem.h b/include/linux/rwsem.h
index 0e75e26e8813..8da14a08a4e1 100644
--- a/include/linux/rwsem.h
+++ b/include/linux/rwsem.h
@@ -262,17 +262,23 @@ DEFINE_LOCK_GUARD_1(rwsem_read, struct rw_semaphore, down_read(_T->lock), up_rea
DEFINE_LOCK_GUARD_1_COND(rwsem_read, _try, down_read_trylock(_T->lock))
DEFINE_LOCK_GUARD_1_COND(rwsem_read, _intr, down_read_interruptible(_T->lock), _RET == 0)
-DECLARE_LOCK_GUARD_1_ATTRS(rwsem_read, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(rwsem_read_try, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(rwsem_read_intr, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(rwsem_read, __acquires_shared(_T), __releases_shared(*(struct rw_semaphore **)_T))
+#define class_rwsem_read_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rwsem_read, _T)
+DECLARE_LOCK_GUARD_1_ATTRS(rwsem_read_try, __acquires_shared(_T), __releases_shared(*(struct rw_semaphore **)_T))
+#define class_rwsem_read_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rwsem_read_try, _T)
+DECLARE_LOCK_GUARD_1_ATTRS(rwsem_read_intr, __acquires_shared(_T), __releases_shared(*(struct rw_semaphore **)_T))
+#define class_rwsem_read_intr_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rwsem_read_intr, _T)
DEFINE_LOCK_GUARD_1(rwsem_write, struct rw_semaphore, down_write(_T->lock), up_write(_T->lock))
DEFINE_LOCK_GUARD_1_COND(rwsem_write, _try, down_write_trylock(_T->lock))
DEFINE_LOCK_GUARD_1_COND(rwsem_write, _kill, down_write_killable(_T->lock), _RET == 0)
-DECLARE_LOCK_GUARD_1_ATTRS(rwsem_write, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(rwsem_write_try, __assumes_ctx_lock(_T), /* */)
-DECLARE_LOCK_GUARD_1_ATTRS(rwsem_write_kill, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(rwsem_write, __acquires(_T), __releases(*(struct rw_semaphore **)_T))
+#define class_rwsem_write_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rwsem_write, _T)
+DECLARE_LOCK_GUARD_1_ATTRS(rwsem_write_try, __acquires(_T), __releases(*(struct rw_semaphore **)_T))
+#define class_rwsem_write_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rwsem_write_try, _T)
+DECLARE_LOCK_GUARD_1_ATTRS(rwsem_write_kill, __acquires(_T), __releases(*(struct rw_semaphore **)_T))
+#define class_rwsem_write_kill_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rwsem_write_kill, _T)
/*
* downgrade write lock to read lock
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 397f4753d10a..41ed884cffc9 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -226,6 +226,7 @@ static inline void task_unlock(struct task_struct *p)
}
DEFINE_LOCK_GUARD_1(task_lock, struct task_struct, task_lock(_T->lock), task_unlock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(task_lock, __assumes_ctx_lock(_T->alloc_lock), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(task_lock, __acquires(&_T->alloc_lock), __releases(&(*(struct task_struct **)_T)->alloc_lock))
+#define class_task_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(task_lock, _T)
#endif /* _LINUX_SCHED_TASK_H */
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 3a45b08ced43..396b8c5d6c1b 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -537,109 +537,132 @@ void free_bucket_spinlocks(spinlock_t *locks);
DEFINE_LOCK_GUARD_1(raw_spinlock, raw_spinlock_t,
raw_spin_lock(_T->lock),
raw_spin_unlock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock, _T)
DEFINE_LOCK_GUARD_1_COND(raw_spinlock, _try, raw_spin_trylock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_try, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_try, _T)
DEFINE_LOCK_GUARD_1(raw_spinlock_nested, raw_spinlock_t,
raw_spin_lock_nested(_T->lock, SINGLE_DEPTH_NESTING),
raw_spin_unlock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_nested, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_nested, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_nested_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_nested, _T)
DEFINE_LOCK_GUARD_1(raw_spinlock_irq, raw_spinlock_t,
raw_spin_lock_irq(_T->lock),
raw_spin_unlock_irq(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irq, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irq, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_irq, _T)
DEFINE_LOCK_GUARD_1_COND(raw_spinlock_irq, _try, raw_spin_trylock_irq(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irq_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irq_try, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_irq_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_irq_try, _T)
DEFINE_LOCK_GUARD_1(raw_spinlock_bh, raw_spinlock_t,
raw_spin_lock_bh(_T->lock),
raw_spin_unlock_bh(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_bh, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_bh, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_bh_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_bh, _T)
DEFINE_LOCK_GUARD_1_COND(raw_spinlock_bh, _try, raw_spin_trylock_bh(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_bh_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_bh_try, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_bh_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_bh_try, _T)
DEFINE_LOCK_GUARD_1(raw_spinlock_irqsave, raw_spinlock_t,
raw_spin_lock_irqsave(_T->lock, _T->flags),
raw_spin_unlock_irqrestore(_T->lock, _T->flags),
unsigned long flags)
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irqsave, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irqsave, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_irqsave, _T)
DEFINE_LOCK_GUARD_1_COND(raw_spinlock_irqsave, _try,
raw_spin_trylock_irqsave(_T->lock, _T->flags))
-DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irqsave_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(raw_spinlock_irqsave_try, __acquires(_T), __releases(*(raw_spinlock_t **)_T))
+#define class_raw_spinlock_irqsave_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(raw_spinlock_irqsave_try, _T)
DEFINE_LOCK_GUARD_1(spinlock, spinlock_t,
spin_lock(_T->lock),
spin_unlock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock, _T)
DEFINE_LOCK_GUARD_1_COND(spinlock, _try, spin_trylock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock_try, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock_try, _T)
DEFINE_LOCK_GUARD_1(spinlock_irq, spinlock_t,
spin_lock_irq(_T->lock),
spin_unlock_irq(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irq, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irq, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock_irq, _T)
DEFINE_LOCK_GUARD_1_COND(spinlock_irq, _try,
spin_trylock_irq(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irq_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irq_try, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_irq_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock_irq_try, _T)
DEFINE_LOCK_GUARD_1(spinlock_bh, spinlock_t,
spin_lock_bh(_T->lock),
spin_unlock_bh(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock_bh, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock_bh, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_bh_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock_bh, _T)
DEFINE_LOCK_GUARD_1_COND(spinlock_bh, _try,
spin_trylock_bh(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock_bh_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock_bh_try, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_bh_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock_bh_try, _T)
DEFINE_LOCK_GUARD_1(spinlock_irqsave, spinlock_t,
spin_lock_irqsave(_T->lock, _T->flags),
spin_unlock_irqrestore(_T->lock, _T->flags),
unsigned long flags)
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irqsave, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irqsave, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock_irqsave, _T)
DEFINE_LOCK_GUARD_1_COND(spinlock_irqsave, _try,
spin_trylock_irqsave(_T->lock, _T->flags))
-DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irqsave_try, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(spinlock_irqsave_try, __acquires(_T), __releases(*(spinlock_t **)_T))
+#define class_spinlock_irqsave_try_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(spinlock_irqsave_try, _T)
DEFINE_LOCK_GUARD_1(read_lock, rwlock_t,
read_lock(_T->lock),
read_unlock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(read_lock, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(read_lock, __acquires(_T), __releases(*(rwlock_t **)_T))
+#define class_read_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(read_lock, _T)
DEFINE_LOCK_GUARD_1(read_lock_irq, rwlock_t,
read_lock_irq(_T->lock),
read_unlock_irq(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(read_lock_irq, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(read_lock_irq, __acquires(_T), __releases(*(rwlock_t **)_T))
+#define class_read_lock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(read_lock_irq, _T)
DEFINE_LOCK_GUARD_1(read_lock_irqsave, rwlock_t,
read_lock_irqsave(_T->lock, _T->flags),
read_unlock_irqrestore(_T->lock, _T->flags),
unsigned long flags)
-DECLARE_LOCK_GUARD_1_ATTRS(read_lock_irqsave, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(read_lock_irqsave, __acquires(_T), __releases(*(rwlock_t **)_T))
+#define class_read_lock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(read_lock_irqsave, _T)
DEFINE_LOCK_GUARD_1(write_lock, rwlock_t,
write_lock(_T->lock),
write_unlock(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(write_lock, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(write_lock, __acquires(_T), __releases(*(rwlock_t **)_T))
+#define class_write_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(write_lock, _T)
DEFINE_LOCK_GUARD_1(write_lock_irq, rwlock_t,
write_lock_irq(_T->lock),
write_unlock_irq(_T->lock))
-DECLARE_LOCK_GUARD_1_ATTRS(write_lock_irq, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(write_lock_irq, __acquires(_T), __releases(*(rwlock_t **)_T))
+#define class_write_lock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(write_lock_irq, _T)
DEFINE_LOCK_GUARD_1(write_lock_irqsave, rwlock_t,
write_lock_irqsave(_T->lock, _T->flags),
write_unlock_irqrestore(_T->lock, _T->flags),
unsigned long flags)
-DECLARE_LOCK_GUARD_1_ATTRS(write_lock_irqsave, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(write_lock_irqsave, __acquires(_T), __releases(*(rwlock_t **)_T))
+#define class_write_lock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(write_lock_irqsave, _T)
#undef __LINUX_INSIDE_SPINLOCK_H
#endif /* __LINUX_SPINLOCK_H */
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 88f01d6fded8..bb44a0bd7696 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -621,16 +621,21 @@ DEFINE_LOCK_GUARD_1(srcu, struct srcu_struct,
_T->idx = srcu_read_lock(_T->lock),
srcu_read_unlock(_T->lock, _T->idx),
int idx)
-DECLARE_LOCK_GUARD_1_ATTRS(srcu, __assumes_ctx_lock(_T), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(srcu, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
+#define class_srcu_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu, _T)
DEFINE_LOCK_GUARD_1(srcu_fast, struct srcu_struct,
_T->scp = srcu_read_lock_fast(_T->lock),
srcu_read_unlock_fast(_T->lock, _T->scp),
struct srcu_ctr __percpu *scp)
+DECLARE_LOCK_GUARD_1_ATTRS(srcu_fast, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
+#define class_srcu_fast_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu_fast, _T)
DEFINE_LOCK_GUARD_1(srcu_fast_notrace, struct srcu_struct,
_T->scp = srcu_read_lock_fast_notrace(_T->lock),
srcu_read_unlock_fast_notrace(_T->lock, _T->scp),
struct srcu_ctr __percpu *scp)
+DECLARE_LOCK_GUARD_1_ATTRS(srcu_fast_notrace, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
+#define class_srcu_fast_notrace_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu_fast_notrace, _T)
#endif
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index fab6a19dda02..7c57a6d2f847 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1874,7 +1874,8 @@ DEFINE_LOCK_GUARD_1(task_rq_lock, struct task_struct,
_T->rq = task_rq_lock(_T->lock, &_T->rf),
task_rq_unlock(_T->rq, _T->lock, &_T->rf),
struct rq *rq; struct rq_flags rf)
-DECLARE_LOCK_GUARD_1_ATTRS(task_rq_lock, __assumes_ctx_lock(_T->pi_lock), /* */)
+DECLARE_LOCK_GUARD_1_ATTRS(task_rq_lock, __acquires(_T->pi_lock), __releases((*(struct task_struct **)_T)->pi_lock))
+#define class_task_rq_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(task_rq_lock, _T)
DEFINE_LOCK_GUARD_1(__task_rq_lock, struct task_struct,
_T->rq = __task_rq_lock(_T->lock, &_T->rf),
@@ -1928,21 +1929,24 @@ DEFINE_LOCK_GUARD_1(rq_lock, struct rq,
rq_unlock(_T->lock, &_T->rf),
struct rq_flags rf)
-DECLARE_LOCK_GUARD_1_ATTRS(rq_lock, __assumes_ctx_lock(__rq_lockp(_T)), /* */);
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T)));
+#define class_rq_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock, _T)
DEFINE_LOCK_GUARD_1(rq_lock_irq, struct rq,
rq_lock_irq(_T->lock, &_T->rf),
rq_unlock_irq(_T->lock, &_T->rf),
struct rq_flags rf)
-DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irq, __assumes_ctx_lock(__rq_lockp(_T)), /* */);
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irq, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T)));
+#define class_rq_lock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock_irq, _T)
DEFINE_LOCK_GUARD_1(rq_lock_irqsave, struct rq,
rq_lock_irqsave(_T->lock, &_T->rf),
rq_unlock_irqrestore(_T->lock, &_T->rf),
struct rq_flags rf)
-DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, __assumes_ctx_lock(__rq_lockp(_T)), /* */);
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T)));
+#define class_rq_lock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, _T)
#define this_rq_lock_irq(...) __acquire_ret(_this_rq_lock_irq(__VA_ARGS__), __rq_lockp(__ret))
static inline struct rq *_this_rq_lock_irq(struct rq_flags *rf) __acquires_ret
@@ -3075,10 +3079,17 @@ static inline class_##name##_t class_##name##_constructor(type *lock, type *lock
__no_context_analysis \
{ class_##name##_t _t = { .lock = lock, .lock2 = lock2 }, *_T = &_t; \
_lock; return _t; }
-#define DECLARE_LOCK_GUARD_2_ATTRS(_name, _lock, _unlock) \
+#define DECLARE_LOCK_GUARD_2_ATTRS(_name, _lock, _unlock1, _unlock2) \
static inline class_##_name##_t class_##_name##_constructor(lock_##_name##_t *_T1, \
lock_##_name##_t *_T2) _lock; \
-static inline void class_##_name##_destructor(class_##_name##_t *_T) _unlock
+static __always_inline void __class_##_name##_cleanup_ctx1(class_##_name##_t **_T1) \
+ __no_context_analysis _unlock1 { } \
+static __always_inline void __class_##_name##_cleanup_ctx2(class_##_name##_t **_T2) \
+ __no_context_analysis _unlock2 { }
+#define WITH_LOCK_GUARD_2_ATTRS(_name, _T1, _T2) \
+ class_##_name##_constructor(_T), \
+ *__UNIQUE_ID(unlock1) __cleanup(__class_##_name##_cleanup_ctx1) = (void *)(_T1),\
+ *__UNIQUE_ID(unlock2) __cleanup(__class_##_name##_cleanup_ctx2) = (void *)(_T2)
static inline bool rq_order_less(struct rq *rq1, struct rq *rq2)
{
@@ -3229,7 +3240,12 @@ DEFINE_LOCK_GUARD_2(double_raw_spinlock, raw_spinlock_t,
double_raw_lock(_T->lock, _T->lock2),
double_raw_unlock(_T->lock, _T->lock2))
-DECLARE_LOCK_GUARD_2_ATTRS(double_raw_spinlock, __assumes_ctx_lock(_T1) __assumes_ctx_lock(_T2), /* */);
+DECLARE_LOCK_GUARD_2_ATTRS(double_raw_spinlock,
+ __acquires(_T1, _T2),
+ __releases(*(raw_spinlock_t **)_T1),
+ __releases(*(raw_spinlock_t **)_T2));
+#define class_double_raw_spinlock_constructor(_T1, _T2) \
+ WITH_LOCK_GUARD_2_ATTRS(double_raw_spinlock, _T1, _T2)
/*
* double_rq_unlock - safely unlock two runqueues
diff --git a/lib/test_context-analysis.c b/lib/test_context-analysis.c
index 3f72b1ab2300..1c5a381461fc 100644
--- a/lib/test_context-analysis.c
+++ b/lib/test_context-analysis.c
@@ -459,8 +459,9 @@ static void __used test_srcu(struct test_srcu_data *d)
static void __used test_srcu_guard(struct test_srcu_data *d)
{
- guard(srcu)(&d->srcu);
- (void)srcu_dereference(d->data, &d->srcu);
+ { guard(srcu)(&d->srcu); (void)srcu_dereference(d->data, &d->srcu); }
+ { guard(srcu_fast)(&d->srcu); (void)srcu_dereference(d->data, &d->srcu); }
+ { guard(srcu_fast_notrace)(&d->srcu); (void)srcu_dereference(d->data, &d->srcu); }
}
struct test_local_lock_data {
^ permalink raw reply related
* Re: [PATCH V2 1/1] IMA event log trimming
From: steven chen @ 2025-12-16 19:59 UTC (permalink / raw)
To: Mimi Zohar, linux-integrity
Cc: roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet, serge,
paul, jmorris, linux-security-module, anirudhve, gregorylumen,
nramas, sushring, linux-doc, steven chen
In-Reply-To: <c93907cb0f08f9baa320488989aa87e7867ee9da.camel@linux.ibm.com>
On 12/16/2025 4:50 AM, Mimi Zohar wrote:
> Hi Steven,
>
> As I previously said, "The main difference between this patch and Roberto's
> version is the length of time needed for locking the measurement list, which
> prevents new entries from being appended to the measurement list. In Roberto's
> version, the list head is moved quickly and the lock released. Measuring the
> total amount of time needed to trim the measurement list ignores the benefit of
> his version. I plan on reviewing both this version and his (hopefully today)."
>
> The other difference is "when" the IMA measurement list is read and saved,
> before the trigger to trim the measurement list or after when the measurement
> list is staged. In this case, the initial trigger trims the measurement list.
> In the other case, the measurement list is staged and then deleted. When
> reviewing Roberto's patch, I plan to discuss it.
Hi Mimi,
Will update this. Thanks!
> After trimming the measurement list, existing verifiers, which walk the IMA
> measurement list, will obviously fail to match the PCRs. Breaking existing
> userspace applications is a problem and, unfortunately, requires yet another
> Kconfig option. It needs to be at least mentioned here in the patch
> description.
Will add Kconfig option. Thanks!
> There are two places where it says, "the list never shrinks, so we don't need a
> lock here". Either the code, the comment, or both need to be updated.
Will update.
>
> On Thu, 2025-12-11 at 10:41 -0800, steven chen wrote:
>> On 12/10/2025 3:53 PM, steven chen wrote:
>>> This patch is for trimming N entries of the IMA event logs. It will also
>>> cleaning the hash table if ima_flush_htable is set.
> Please refer to "Describe your changes in imperative mood" in the "Describe your
> changes" section of Documentation/process/submitting-patches.rst.
Will update. Thanks!
>>> It provides a userspace interface ima_trim_log that can be used to input
>>> number N to let kernel to trim N entries of IMA event logs. When read
> There is only a single kernel measurement list or event log, not plural. There
> are N number of "entries" or "records" in the IMA measurement list.
>
> -> trim N records from the IMA measurement list.
Will update. Thanks!
>>> this interface, it returns number of entries trimmed last time.
> Please provide an example of how to initiate the trim.
>
> After trimming the IMA measurement list, are the other securityfs files correct?
> Are they correct after a kexec? Or are they reset without a way of resurrecting
> them without the full measurement list?
Will update. Thanks!
>
>>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>>> ---
>>> .../admin-guide/kernel-parameters.txt | 4 +
>>> security/integrity/ima/ima.h | 2 +
>>> security/integrity/ima/ima_fs.c | 175 +++++++++++++++++-
>>> security/integrity/ima/ima_queue.c | 64 +++++++
>>> 4 files changed, 241 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>> index e92c0056e4e0..cd1a1d0bf0e2 100644
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -2197,6 +2197,10 @@
>>> Use the canonical format for the binary runtime
>>> measurements, instead of host native format.
>>>
>>> + ima_flush_htable [IMA]
>>> + Flush the measurement list hash table when trim all
>>> + or a part of it for deletion.
>>> +
>>> ima_hash= [IMA]
>>> Format: { md5 | sha1 | rmd160 | sha256 | sha384
>>> | sha512 | ... }
>>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>>> index e3d71d8d56e3..ab0e30ee25ea 100644
>>> --- a/security/integrity/ima/ima.h
>>> +++ b/security/integrity/ima/ima.h
>>> @@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
>>>
>>> #ifdef CONFIG_IMA_KEXEC
>>> void ima_measure_kexec_event(const char *event_name);
>>> +long ima_purge_event_log(long number_logs);
>>> #else
>>> static inline void ima_measure_kexec_event(const char *event_name) {}
>>> +static inline long ima_purge_event_log(long number_logs) { return 0; }
>>> #endif
>>>
>>> /*
>>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>>> index 87045b09f120..410f7d03c43f 100644
>>> --- a/security/integrity/ima/ima_fs.c
>>> +++ b/security/integrity/ima/ima_fs.c
>>> @@ -21,6 +21,9 @@
>>> #include <linux/rcupdate.h>
>>> #include <linux/parser.h>
>>> #include <linux/vmalloc.h>
>>> +#include <linux/ktime.h>
>>> +#include <linux/timekeeping.h>
>>> +#include <linux/ima.h>
>>>
>>> #include "ima.h"
>>>
>>> @@ -38,6 +41,14 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>>>
>>> static int valid_policy = 1;
>>>
>>> +#define IMA_LOG_TRIM_REQ_LENGTH 11
>>> +#define IMA_LOG_TRIM_EVENT_LEN 256
>>> +
>>> +static long trimcount;
>>> +/* mutex protects atomicity of trimming measurement list requests */
> ima_measure_lock is taken for more than just synchronization of trimming the
> measurement list. Please update comment.
Will update. Thanks!
>
>>> +static DEFINE_MUTEX(ima_measure_lock);
>>> +static long ima_measure_users;
>>> +
>>> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
>>> loff_t *ppos, atomic_long_t *val)
>>> {
>>> @@ -202,16 +213,65 @@ static const struct seq_operations ima_measurments_seqops = {
>>> .show = ima_measurements_show
>>> };
>>>
> _ima_measurements_open() seems pretty fundamental to the locking scheme.
> Preventing opening the IMA measurement list is new. There should at least be a
> short, regular comment explaining what you're locking and why.
Will update. Thanks!
>>> +static int _ima_measurements_open(struct inode *inode, struct file *file,
>>> + const struct seq_operations *seq_ops)
>>> +{
>>> + bool write = !!(file->f_mode & FMODE_WRITE);
>>> + int ret;
>>> +
>>> + if (write && !capable(CAP_SYS_ADMIN))
>>> + return -EPERM;
>>> +
>>> + mutex_lock(&ima_measure_lock);
>>> + if ((write && ima_measure_users != 0) ||
>>> + (!write && ima_measure_users < 0)) {
>>> + mutex_unlock(&ima_measure_lock);
>>> + return -EBUSY;
>>> + }
>>> +
>>> + ret = seq_open(file, seq_ops);
>>> + if (ret < 0) {
>>> + mutex_unlock(&ima_measure_lock);
>>> + return ret;
>>> + }
>>> +
>>> + if (write)
>>> + ima_measure_users--;
>>> + else
>>> + ima_measure_users++;
>>> +
>>> + mutex_unlock(&ima_measure_lock);
>>> + return ret;
>>> +}
>>> +
>>> static int ima_measurements_open(struct inode *inode, struct file *file)
>>> {
>>> - return seq_open(file, &ima_measurments_seqops);
>>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>>> +}
>>> +
>>> +static int ima_measurements_release(struct inode *inode, struct file *file)
>>> +{
>>> + bool write = !!(file->f_mode & FMODE_WRITE);
>>> + int ret;
>>> +
>>> + mutex_lock(&ima_measure_lock);
>>> + ret = seq_release(inode, file);
>>> + if (!ret) {
>>> + if (write)
>>> + ima_measure_users++;
>>> + else
>>> + ima_measure_users--;
>>> + }
>>> +
>>> + mutex_unlock(&ima_measure_lock);
>>> + return ret;
>>> }
>>>
>>> static const struct file_operations ima_measurements_ops = {
>>> .open = ima_measurements_open,
>>> .read = seq_read,
>>> .llseek = seq_lseek,
>>> - .release = seq_release,
>>> + .release = ima_measurements_release,
>>> };
>>>
>>> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
>>> @@ -279,14 +339,111 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>>>
>>> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
>>> {
>>> - return seq_open(file, &ima_ascii_measurements_seqops);
>>> + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
>>> }
>>>
>>> static const struct file_operations ima_ascii_measurements_ops = {
>>> .open = ima_ascii_measurements_open,
>>> .read = seq_read,
>>> .llseek = seq_lseek,
>>> - .release = seq_release,
>>> + .release = ima_measurements_release,
>>> +};
>>> +
>>> +static void ima_measure_trim_event(const long number_logs)
>>> +{
>>> + char ima_log_trim_event[IMA_LOG_TRIM_EVENT_LEN];
>>> + struct timespec64 ts;
>>> + u64 time_ns;
>>> + int n;
>>> +
>>> + ktime_get_real_ts64(&ts);
>>> + time_ns = (u64)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
>>> + n = scnprintf(ima_log_trim_event, IMA_LOG_TRIM_EVENT_LEN,
>>> + "time=%llu; log trim this time=%lu;",
>>> + time_ns, number_logs);
>>> +
>>> + ima_measure_critical_data("ima_log_trim", "trim ima event logs", ima_log_trim_event, n, false, NULL, 0);
>>> +}
> There's no mention of a new critical data record in the patch description. It
> should be a separate patch with a full patch description describing what it is
> and how to verify it.
Will update this. Thanks!
>>> +
>>> +static int ima_log_trim_open(struct inode *inode, struct file *file)
>>> +{
>>> + bool write = !!(file->f_mode & FMODE_WRITE);
>>> +
>>> + if (!write && capable(CAP_SYS_ADMIN))
>>> + return 0;
>>> + else if (!capable(CAP_SYS_ADMIN))
>>> + return -EPERM;
>>> +
>>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>>> +}
>>> +
>>> +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
>>> +{
>>> + char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
>>> + ssize_t len;
>>> +
>>> + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
>>> + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
>>> +}
>>> +
>>> +static ssize_t ima_log_trim_write(struct file *file,
>>> + const char __user *buf, size_t datalen, loff_t *ppos)
>>> +{
>>> + unsigned char req[IMA_LOG_TRIM_REQ_LENGTH];
>>> + long count, n;
>>> + int ret;
>>> +
>>> + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
>>> + ret = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> + n = (int)datalen;
>>> +
>>> + ret = copy_from_user(req, buf, datalen);
>>> + if (ret < 0)
>>> + goto out;
>>> +
>>> + count = 0;
>>> + for (int i = 0; i < n; ++i) {
>>> + if (req[i] < '0' || req[i] > '9') {
>>> + ret = -EINVAL;
>>> + goto out;
>>> + }
>>> + count = count * 10 + req[i] - '0';
>>> + }
> This code can be simplified by using the kstrto*_from_user() family of
> functions. The patch description should include an example how to trim the
> measurement list.
Will update. Thanks!
>>> + ret = ima_purge_event_log(count);
> The term "purge" is really strong wording. I hope you're not purging the
> measurement list, but simply removing them from kernel memory.
Will update. Thanks!
>
>>> +
>>> + if (ret < 0)
>>> + goto out;
>>> +
>>> + trimcount = ret;
>>> +
>>> + if (trimcount > 0)
>>> + ima_measure_trim_event(trimcount);
>>> +
>>> + ret = datalen;
>>> +out:
>>> + return ret;
>>> +}
>>> +
>>> +static int ima_log_trim_release(struct inode *inode, struct file *file)
>>> +{
>>> + bool write = !!(file->f_mode & FMODE_WRITE);
>>> + if (!write && capable(CAP_SYS_ADMIN))
>>> + return 0;
>>> + else if (!capable(CAP_SYS_ADMIN))
>>> + return -EPERM;
>>> +
>>> + return ima_measurements_release(inode, file);
>>> +}
>>> +
>>> +static const struct file_operations ima_log_trim_ops = {
>>> + .open = ima_log_trim_open,
>>> + .read = ima_log_trim_read,
>>> + .write = ima_log_trim_write,
>>> + .llseek = generic_file_llseek,
>>> + .release = ima_log_trim_release
>>> };
>>>
>>> static ssize_t ima_read_policy(char *path)
>>> @@ -528,6 +685,16 @@ int __init ima_fs_init(void)
>>> goto out;
>>> }
>>>
>>> + dentry = securityfs_create_file("ima_trim_log",
>>> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
>>> + ima_dir, NULL, &ima_log_trim_ops);
>>> + if (IS_ERR(dentry)) {
>>> + ret = PTR_ERR(dentry);
>>> + goto out;
>>> + }
>>> +
>>> + trimcount = 0;
>>> +
>>> dentry = securityfs_create_file("runtime_measurements_count",
>>> S_IRUSR | S_IRGRP, ima_dir, NULL,
>>> &ima_measurements_count_ops);
>>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>>> index 590637e81ad1..77ab52469727 100644
>>> --- a/security/integrity/ima/ima_queue.c
>>> +++ b/security/integrity/ima/ima_queue.c
>>> @@ -22,6 +22,14 @@
>>>
>>> #define AUDIT_CAUSE_LEN_MAX 32
>>>
>>> +bool ima_flush_htable;
>>> +static int __init ima_flush_htable_setup(char *str)
>>> +{
>>> + ima_flush_htable = true;
>>> + return 1;
>>> +}
>>> +__setup("ima_flush_htable", ima_flush_htable_setup);
>>> +
>>> /* pre-allocated array of tpm_digest structures to extend a PCR */
>>> static struct tpm_digest *digests;
>>>
>>> @@ -220,6 +228,62 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>>> return result;
>>> }
>>>
>>> +/* Delete the IMA event logs */
> Do you mean delete the IMA event records/entries?
Will update. Thanks!
>>> +long ima_purge_event_log(long number_logs)
> If this function is not defined as static, then it requires a kernel-doc.
Will update. Thanks!
>>> +{
>>> + struct ima_queue_entry *qe, *qe_tmp;
>>> + LIST_HEAD(ima_measurements_staged);
>>> + unsigned int i;
>>> + long cur = number_logs;
> The variable name "number_logs" is confusing. As I mentioned in the patch
> description, there is one measurement list with multiple records. There aren't
> multiple logs in the kernel (other than the staged list).
Will update it to "req_value". Thanks!
>>> +
>>> + if (number_logs <= 0)
>>> + return number_logs;
>>> +
>>> + mutex_lock(&ima_extend_list_mutex);
>>> +
>>> +
>>> + list_for_each_entry(qe, &ima_measurements, later) {
>>> + if (--number_logs == 0)
>>> + break;
>>> + }
>>> +
>>> + if (number_logs > 0) {
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + __list_cut_position(&ima_measurements_staged, &ima_measurements,
>>> + &qe->later);
>>> + atomic_long_sub(cur, &ima_htable.len);
>>> +
>>> + if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable) {
>>> + list_for_each_entry(qe, &ima_measurements_staged, later)
>>> + /* It can race with ima_lookup_digest_entry(). */
>>> + hlist_del_rcu(&qe->hnext);
>>> + }
>> If the h table can be staged during the locking period and deleted after
>> unlocking, the time
>> the list is held will be reduced.
>>
>> I will work on this, and any suggestions are greatly appreciated.
>>
>> Thanks,
>>
>> Steven
>>
>>> +
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> +
>>> +
>>> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
>>> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
>>> + kfree(qe->entry->template_data[i].data);
>>> + qe->entry->template_data[i].data = NULL;
>>> + qe->entry->template_data[i].len = 0;
>>> + }
>>> +
>>> + list_del(&qe->later);
>>> +
>>> + if (ima_flush_htable) {
>>> + kfree(qe->entry->digests);
>>> + kfree(qe->entry);
>>> + kfree(qe);
>>> + }
>>> + }
> To avoid code duplication, there's a similar function named
> ima_free_template_entry(). Any changes needed to the function should be done as
> a preparatory patch.
I will check and update.
Thanks,
Steven
>>> +
>>> + return cur;
>>> +}
>>> +
>>> int ima_restore_measurement_entry(struct ima_template_entry *entry)
>>> {
>>> int result = 0;
>
^ permalink raw reply
* Re: [PATCH RFC 8/11] security: Hornet LSM
From: Paul Moore @ 2025-12-16 21:00 UTC (permalink / raw)
To: Blaise Boscaccy, Blaise Boscaccy, Jonathan Corbet, James Morris,
Serge E. Hallyn, Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
linux-security-module, linux-doc, linux-kernel, bpf
In-Reply-To: <20251211021257.1208712-9-bboscaccy@linux.microsoft.com>
On Dec 10, 2025 Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
>
> This adds the Hornet Linux Security Module which provides enhanced
> signature verification and data validation for eBPF programs. This
> allows users to continue to maintain an invariant that all code
> running inside of the kernel has actually been signed and verified, by
> the kernel.
>
> This effort builds upon the currently excepted upstream solution. It
s/excepted/accepted/ ;)
> further hardens it by providing deterministic, in-kernel checking of
> map hashes to solidify auditing along with preventing TOCTOU attacks
> against lskel map hashes.
>
> Target map hashes are passed in via PKCS#7 signed attributes. Hornet
> determines the extent which the eBFP program is signed and defers to
> other LSMs for policy decisions.
>
> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> ---
> Documentation/admin-guide/LSM/Hornet.rst | 38 +++++
> Documentation/admin-guide/LSM/index.rst | 1 +
> MAINTAINERS | 9 +
> include/linux/oid_registry.h | 3 +
> include/uapi/linux/lsm.h | 1 +
> security/Kconfig | 3 +-
> security/Makefile | 1 +
> security/hornet/Kconfig | 11 ++
> security/hornet/Makefile | 7 +
> security/hornet/hornet.asn1 | 13 ++
> security/hornet/hornet_lsm.c | 201 +++++++++++++++++++++++
> 11 files changed, 287 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/admin-guide/LSM/Hornet.rst
> create mode 100644 security/hornet/Kconfig
> create mode 100644 security/hornet/Makefile
> create mode 100644 security/hornet/hornet.asn1
> create mode 100644 security/hornet/hornet_lsm.c
...
> diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst
> index b44ef68f6e4da..57f6e9fbe5fd1 100644
> --- a/Documentation/admin-guide/LSM/index.rst
> +++ b/Documentation/admin-guide/LSM/index.rst
> @@ -49,3 +49,4 @@ subdirectories.
> SafeSetID
> ipe
> landlock
> + Hornet
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3da2c26a796b8..64c9aaff6a219 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11399,6 +11399,15 @@ S: Maintained
> F: Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml
> F: drivers/iio/pressure/mprls0025pa*
>
> +HORNET SECURITY MODULE
> +M: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> +L: linux-security-module@vger.kernel.org
> +S: Supported
> +T: git https://github.com/blaiseboscaccy/hornet.git
> +F: Documentation/admin-guide/LSM/Hornet.rst
> +F: scripts/hornet/
> +F: security/hornet/
I know we talked about this last time Hornet was proposed, but as a
reminder, here are the guidelines for new LSMs:
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md
> diff --git a/security/hornet/Kconfig b/security/hornet/Kconfig
> new file mode 100644
> index 0000000000000..19406aa237ac6
> --- /dev/null
> +++ b/security/hornet/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config SECURITY_HORNET
> + bool "Hornet support"
> + depends on SECURITY
> + default n
> + help
> + This selects Hornet.
> + Further information can be found in
> + Documentation/admin-guide/LSM/Hornet.rst.
> +
> + If you are unsure how to answer this question, answer N.
Pointing people at the docs is good, but there really should be a couple
lines about Hornet here. At the very least explain that it does BPF
signature verification and allows for other LSMs to enforce policy on
this integrity verification checks.
> diff --git a/security/hornet/hornet_lsm.c b/security/hornet/hornet_lsm.c
> new file mode 100644
> index 0000000000000..a8499ee108ad3
> --- /dev/null
> +++ b/security/hornet/hornet_lsm.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Hornet Linux Security Module
> + *
> + * Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> + *
> + * Copyright (C) 2025 Microsoft Corporation
> + */
> +
> +#include <linux/lsm_hooks.h>
> +#include <uapi/linux/lsm.h>
> +#include <linux/bpf.h>
> +#include <linux/verification.h>
> +#include <crypto/public_key.h>
> +#include <linux/module_signature.h>
> +#include <crypto/pkcs7.h>
> +#include <linux/sort.h>
> +#include <linux/asn1_decoder.h>
> +#include <linux/oid_registry.h>
> +#include "hornet.asn1.h"
> +
> +#define MAX_USED_MAPS 64
> +
> +struct hornet_maps {
> + bpfptr_t fd_array;
> +};
> +
> +struct hornet_parse_context {
> + size_t indexes[MAX_USED_MAPS];
> + bool skips[MAX_USED_MAPS];
> + unsigned char hashes[SHA256_DIGEST_SIZE * MAX_USED_MAPS];
> + int hash_count;
> +};
> +
> +static int hornet_verify_hashes(struct hornet_maps *maps,
> + struct hornet_parse_context *ctx)
> +{
> + int map_fd;
> + u32 i;
> + struct bpf_map *map;
> + int err = 0;
> + unsigned char hash[SHA256_DIGEST_SIZE];
I'd suggest a comment block here explaining that the hash choice is
fixed to SHA256 to remain compatible with the existing BPF signature
mechanism.
> + for (i = 0; i < ctx->hash_count; i++) {
> + if (ctx->skips[i])
> + continue;
> +
> + err = copy_from_bpfptr_offset(&map_fd, maps->fd_array,
> + ctx->indexes[i] * sizeof(map_fd),
> + sizeof(map_fd));
> + if (err < 0)
> + return LSM_INT_VERDICT_BADSIG;
> +
> + CLASS(fd, f)(map_fd);
> + if (fd_empty(f))
> + return LSM_INT_VERDICT_BADSIG;
> + if (unlikely(fd_file(f)->f_op != &bpf_map_fops))
> + return LSM_INT_VERDICT_BADSIG;
> +
> + if (!map->frozen)
> + return LSM_INT_VERDICT_BADSIG;
> +
> + map = fd_file(f)->private_data;
> + map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
> +
> + err = (memcmp(hash, &ctx->hashes[ctx->indexes[i] * SHA256_DIGEST_SIZE],
> + SHA256_DIGEST_SIZE));
> + if (!err)
> + return LSM_INT_VERDICT_BADSIG;
> + }
> + return LSM_INT_VERDICT_OK;
> +}
> +
> +int hornet_next_map(void *context, size_t hdrlen,
> + unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)value;
> +
> + ctx->hash_count++;
> + return 0;
> +}
> +
> +
> +int hornet_map_index(void *context, size_t hdrlen,
> + unsigned char tag,
> + const void *value, size_t vlen)
> +{
> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)value;
> +
> + ctx->hashes[ctx->hash_count] = *(int *)value;
> + return 0;
> +}
> +
> +int hornet_map_hash(void *context, size_t hdrlen,
> + unsigned char tag,
> + const void *value, size_t vlen)
> +
> +{
> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)value;
> +
> + if (vlen != SHA256_DIGEST_SIZE && vlen != 0)
> + return -EINVAL;
It seems like one could incorporate the sanity checking into the
if-statement below.
> + if (vlen != 0) {
> + ctx->skips[ctx->hash_count] = false;
> + memcpy(&ctx->hashes[ctx->hash_count * SHA256_DIGEST_SIZE], value, vlen);
> + } else
> + ctx->skips[ctx->hash_count] = true;
> +
> + return 0;
> +}
> +
> +static int hornet_check_program(struct bpf_prog *prog, union bpf_attr *attr,
> + struct bpf_token *token, bool is_kernel)
> +{
> + struct hornet_maps maps = {0};
> + bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
> + struct pkcs7_message *msg;
> + struct hornet_parse_context *ctx;
> + void *sig;
> + int err;
> + const void *authattrs;
> + size_t authattrs_len;
> +
> + if (!attr->signature)
> + return LSM_INT_VERDICT_UNSIGNED;
Do we need to check the "is_kernel" flag so we don't signal UNSIGNED
to an enforcing LSM on the second half of a lskel program load where
the original BPF program is being loaded?
I'm also not a big fan of mixing int error codes and enums in the return
value, I'd rather see those split out into separate vars. Perhaps
still return the error codes, like the -ENOMEM below, but pass the
verdict info back as a pointer in the arg list? This applies
elsewhere in this file/patch too.
> + ctx = kzalloc(sizeof(struct hornet_parse_context), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + maps.fd_array = make_bpfptr(attr->fd_array, is_kernel);
> + sig = kzalloc(attr->signature_size, GFP_KERNEL);
> + if (!sig) {
> + err = -ENOMEM;
> + goto out;
> + }
> + err = copy_from_bpfptr(sig, usig, attr->signature_size);
> + if (err != 0)
> + goto out;
> +
> + msg = pkcs7_parse_message(sig, attr->signature_size);
> + if (IS_ERR(msg)) {
> + err = LSM_INT_VERDICT_BADSIG;
> + goto out;
> + }
> +
> + if (validate_pkcs7_trust(msg, VERIFY_USE_SECONDARY_KEYRING)) {
> + err = LSM_INT_VERDICT_PARTIALSIG;
> + goto out;
> + }
Personally, I'd be much happier only allowing the secondary keyring on
my systems, but we know that some people are not as concerned about
allowing user and session keys, so we should think about ways to allow
Hornet to support multiple keyrings so long as the keyring is factored
into Hornet's integrity verdict.
This is worth some discussion, but we could potentially encode this into
the integrity verdict (e.g. VERDICT_GOODSIG_TRUSTEDKEY, _GOODSIG_USRKEY,
__GOODSIG_GRPKEY, __GOODSIG_SESKEY, etc.). We could also leave that up
to the enforcement LSMs that care as they are passed the bpf_attr in
security_bpf_prog_load_post_integrity() already. Or something else?
I'm also not sure how much key type granularity we want. We definitely
want trusted key vs others, but do we care about user vs session keys?
Perhaps that is an argument for deferring to the enforcing LSM.
If all else fails, we could add some Hornet specific configuration knobs
for this, but considering the simplicity of Hornet, I think it would be
nice if we can keep the zero-config nature of Hornet.
> + if (pkcs7_get_authattr(msg, OID_hornet_data,
> + &authattrs, &authattrs_len) == -ENODATA) {
> + err = LSM_INT_VERDICT_PARTIALSIG;
> + goto out;
> + }
> +
> + err = asn1_ber_decoder(&hornet_decoder, ctx, authattrs, authattrs_len);
> + if (err < 0 || authattrs == NULL) {
> + err = LSM_INT_VERDICT_PARTIALSIG;
> + goto out;
> + }
> + err = hornet_verify_hashes(&maps, ctx);
> +out:
Style nitpick - a line of vertical space between the
hornet_verify_hashes() call and the jump label would be a nit nicer
visually.
While I'm talking about style, I know the old days of a strict 80-char
line length are no more, but I still prefer the 80-char limit for code
I'm maintaining. As you would be maintinaing Hornet that decisions is
up to you, but 80-char lines are nice ;)
> + kfree(ctx);
> + return err;
> +}
Some of this would be mitigated by the fact that Hornet verifies the
lskel loader and all the maps, but should we worry about nested loaders?
> +static const struct lsm_id hornet_lsmid = {
> + .name = "hornet",
> + .id = LSM_ID_HORNET,
> +};
> +
> +static int hornet_bpf_prog_load_integrity(struct bpf_prog *prog, union bpf_attr *attr,
> + struct bpf_token *token, bool is_kernel)
> +{
> + int result = hornet_check_program(prog, attr, token, is_kernel);
> +
> + if (result < 0)
> + return result;
> +
> + return security_bpf_prog_load_post_integrity(prog, attr, token, is_kernel,
> + &hornet_lsmid, result);
> +}
> +
> +static struct security_hook_list hornet_hooks[] __ro_after_init = {
> + LSM_HOOK_INIT(bpf_prog_load_integrity, hornet_bpf_prog_load_integrity),
> +};
> +
> +static int __init hornet_init(void)
> +{
> + pr_info("Hornet: eBPF signature verification enabled\n");
> + security_add_hooks(hornet_hooks, ARRAY_SIZE(hornet_hooks), &hornet_lsmid);
> + return 0;
> +}
> +
> +DEFINE_LSM(hornet) = {
> + .name = "hornet",
> + .init = hornet_init,
> +};
> --
> 2.52.0
--
paul-moore.com
^ permalink raw reply
* Re: [RFC 08/11] security: Hornet LSM
From: Blaise Boscaccy @ 2025-12-16 21:01 UTC (permalink / raw)
To: Fan Wu
Cc: Jonathan Corbet, Paul Moore, James Morris, Serge E. Hallyn,
Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
linux-security-module, linux-doc, linux-kernel, bpf
In-Reply-To: <CAKtyLkHNA_fyyK2WZrpA6o1nOzY6dOt+pfhFjDR-1H9UJOceAw@mail.gmail.com>
Fan Wu <wufan@kernel.org> writes:
> On Wed, Dec 10, 2025 at 6:18 PM Blaise Boscaccy
> <bboscaccy@linux.microsoft.com> wrote:
>>
>> This adds the Hornet Linux Security Module which provides enhanced
>> signature verification and data validation for eBPF programs. This
>> allows users to continue to maintain an invariant that all code
>> running inside of the kernel has actually been signed and verified, by
>> the kernel.
>>
>> This effort builds upon the currently excepted upstream solution. It
>> further hardens it by providing deterministic, in-kernel checking of
>> map hashes to solidify auditing along with preventing TOCTOU attacks
>> against lskel map hashes.
>>
>> Target map hashes are passed in via PKCS#7 signed attributes. Hornet
>> determines the extent which the eBFP program is signed and defers to
>> other LSMs for policy decisions.
>>
>> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
>> ---
> ...
>> +
>> +int hornet_next_map(void *context, size_t hdrlen,
>> + unsigned char tag,
>> + const void *value, size_t vlen)
>> +{
>> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)value;
>
> I think you wanted to cast context instead?
>
>> +
>> + ctx->hash_count++;
>> + return 0;
>> +}
>> +
>> +
>> +int hornet_map_index(void *context, size_t hdrlen,
>> + unsigned char tag,
>> + const void *value, size_t vlen)
>> +{
>> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)value;
>
> Same above.
>
>> +
>> + ctx->hashes[ctx->hash_count] = *(int *)value;
>> + return 0;
>> +}
>> +
>> +int hornet_map_hash(void *context, size_t hdrlen,
>> + unsigned char tag,
>> + const void *value, size_t vlen)
>> +
>> +{
>> + struct hornet_parse_context *ctx = (struct hornet_parse_context *)value;
>
> Same above.
>
> -Fan
>
Thanks Fan. Will get that fixed up.
-blaise
>> +
>> + if (vlen != SHA256_DIGEST_SIZE && vlen != 0)
>> + return -EINVAL;
>> +
>> + if (vlen != 0) {
>> + ctx->skips[ctx->hash_count] = false;
>> + memcpy(&ctx->hashes[ctx->hash_count * SHA256_DIGEST_SIZE], value, vlen);
>> + } else
>> + ctx->skips[ctx->hash_count] = true;
>> +
>> + return 0;
>> +}
>> +
^ permalink raw reply
* Re: [RFC 08/11] security: Hornet LSM
From: Blaise Boscaccy @ 2025-12-16 21:02 UTC (permalink / raw)
To: Randy Dunlap, Jonathan Corbet, Paul Moore, James Morris,
Serge E. Hallyn, Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
linux-security-module, linux-doc, linux-kernel, bpf
In-Reply-To: <f7e997ac-2312-4d18-96d7-d6abb190a5c3@infradead.org>
Randy Dunlap <rdunlap@infradead.org> writes:
> On 12/10/25 6:12 PM, Blaise Boscaccy wrote:
>> diff --git a/Documentation/admin-guide/LSM/Hornet.rst b/Documentation/admin-guide/LSM/Hornet.rst
>> new file mode 100644
>> index 0000000000000..0fb5920e9b68f
>> --- /dev/null
>> +++ b/Documentation/admin-guide/LSM/Hornet.rst
>> @@ -0,0 +1,38 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +======
>> +Hornet
>> +======
>> +
>> +Hornet is a Linux Security Module that provides extensible signature
>> +verification for eBPF programs. This is selectable at build-time with
>> +``CONFIG_SECURITY_HORNET``.
>> +
>> +Overview
>> +========
>> +
>> +Hornet addresses concerns from users who require strict audit
>> +trails and verification guarantees, especially in security-sensitive
>> +environments. Map hashes for extended verification are passed in via
>> +the existing PKCS#7 uapi and verifified by the crypto
>
> verified
> and preferably UAPI
>
>> +subsystem. Hornet then calculates the verification state of the
>> +program (full, partial, bad, etc) and then invokes a new downstream
>
> etc.)
>
Copy that. Thanks Randy.
-blaise
>> +LSM hook to delegate policy decisions.
>> +
>> +Tooling
>> +=======
>> +
>> +Some tooling is provided to aid with the development of signed eBPF
>> +light-skeletons.
>> +
>> +extract-skel.sh
>> +---------------
>> +
>> +This shell script extracts the instructions and map data used by the
>> +light skeleton from the autogenerated header file created by bpftool.
>> +
>> +gen_sig
>> +---------
>> +
>> +gen_sig creates a pkcs#7 signature of a data payload. Additionally it
>> +appends a signed attribute containing a set of hashes.
>
> --
> ~Randy
^ permalink raw reply
* [PATCH 1/3] landlock: Add missing ABI 7 case in documentation example
From: Samasth Norway Ananda @ 2025-12-16 21:02 UTC (permalink / raw)
To: mic, gnoack; +Cc: linux-security-module, linux-kernel
Add the missing case 6 and case 7 handling in the ABI version
compatibility example to properly handle LANDLOCK_RESTRICT_SELF_LOG_*
flags for kernels with ABI < 7.
This introduces the supported_restrict_flags variable which is
initialized with LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON, removed
in case 6 for ABI < 7, and passed to landlock_restrict_self() to
enable logging on supported kernels.
Also fix misleading description of the /usr rule which incorrectly
stated it "only allow[s] reading" when the code actually allows both
reading and executing (LANDLOCK_ACCESS_FS_EXECUTE is included in
allowed_access).
Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
Documentation/userspace-api/landlock.rst | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 1d0c2c15c22e..b8caac299056 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -97,6 +97,8 @@ version, and only use the available subset of access rights:
.. code-block:: c
int abi;
+ /* Tracks which landlock_restrict_self() flags are supported */
+ int supported_restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
if (abi < 0) {
@@ -127,6 +129,17 @@ version, and only use the available subset of access rights:
/* Removes LANDLOCK_SCOPE_* for ABI < 6 */
ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
LANDLOCK_SCOPE_SIGNAL);
+ __attribute__((fallthrough));
+ case 6:
+ /*
+ * Removes LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON for ABI < 7.
+ * Note: This modifies supported_restrict_flags, not ruleset_attr,
+ * because logging flags are passed to landlock_restrict_self().
+ */
+ supported_restrict_flags &= ~LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
+ __attribute__((fallthrough));
+ case 7:
+ break;
}
This enables the creation of an inclusive ruleset that will contain our rules.
@@ -142,8 +155,9 @@ This enables the creation of an inclusive ruleset that will contain our rules.
}
We can now add a new rule to this ruleset thanks to the returned file
-descriptor referring to this ruleset. The rule will only allow reading the
-file hierarchy ``/usr``. Without another rule, write actions would then be
+descriptor referring to this ruleset. The rule will allow reading and
+executing files in the ``/usr`` hierarchy. Without another rule, write actions
+and other operations (make_dir, remove_file, etc.) would then be
denied by the ruleset. To add ``/usr`` to the ruleset, we open it with the
``O_PATH`` flag and fill the &struct landlock_path_beneath_attr with this file
descriptor.
@@ -193,7 +207,7 @@ number for a specific action: HTTPS connections.
The next step is to restrict the current thread from gaining more privileges
(e.g. through a SUID binary). We now have a ruleset with the first rule
-allowing read access to ``/usr`` while denying all other handled accesses for
+allowing read and execute access to ``/usr`` while denying all other handled accesses for
the filesystem, and a second rule allowing HTTPS connections.
.. code-block:: c
@@ -208,7 +222,7 @@ The current thread is now ready to sandbox itself with the ruleset.
.. code-block:: c
- if (landlock_restrict_self(ruleset_fd, 0)) {
+ if (landlock_restrict_self(ruleset_fd, supported_restrict_flags)) {
perror("Failed to enforce ruleset");
close(ruleset_fd);
return 1;
--
2.50.1
^ permalink raw reply related
* [PATCH 2/3] landlock: Document Landlock errata mechanism
From: Samasth Norway Ananda @ 2025-12-16 21:02 UTC (permalink / raw)
To: mic, gnoack; +Cc: linux-security-module, linux-kernel
In-Reply-To: <20251216210248.4150777-1-samasth.norway.ananda@oracle.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 5927 bytes --]
Add comprehensive documentation for the Landlock errata mechanism,
including how to query errata using LANDLOCK_CREATE_RULESET_ERRATA
and detailed descriptions of all three existing errata.
Also update the code comment in syscalls.c to remind developers to
update errata documentation when applicable, and update the
documentation date to reflect this new content.
This addresses the gap where the kernel implements errata tracking
but provides no user-facing documentation on how to use it.
Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
Documentation/userspace-api/landlock.rst | 99 +++++++++++++++++++++++-
security/landlock/syscalls.c | 4 +-
2 files changed, 101 insertions(+), 2 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index b8caac299056..d1f7dd30395d 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -8,7 +8,7 @@ Landlock: unprivileged access control
=====================================
:Author: Mickaël Salaün
-:Date: March 2025
+:Date: December 2025
The goal of Landlock is to enable restriction of ambient rights (e.g. global
filesystem or network access) for a set of processes. Because Landlock
@@ -445,6 +445,103 @@ system call:
printf("Landlock supports LANDLOCK_ACCESS_FS_REFER.\n");
}
+Landlock Errata
+---------------
+
+In addition to ABI versions, Landlock provides an errata mechanism to track
+fixes for issues that may affect backwards compatibility or require userspace
+awareness. The errata bitmask can be queried using:
+
+.. code-block:: c
+
+ int errata;
+
+ errata = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_ERRATA);
+ if (errata < 0) {
+ /* Landlock not available or disabled */
+ return 0;
+ }
+
+The returned value is a bitmask where each bit represents a specific erratum.
+If bit N is set (``errata & (1 << (N - 1))``), then erratum N has been fixed
+in the running kernel.
+
+Known Errata
+~~~~~~~~~~~~
+
+**Erratum 1: TCP socket identification (ABI 4)**
+
+Fixed an issue where IPv4 and IPv6 stream sockets (e.g., SMC, MPTCP, or SCTP)
+were incorrectly restricted by TCP access rights during :manpage:`bind(2)` and
+:manpage:`connect(2)` operations.
+
+*Impact:* In kernels without this fix, using ``LANDLOCK_ACCESS_NET_BIND_TCP``
+or ``LANDLOCK_ACCESS_NET_CONNECT_TCP`` would incorrectly restrict non-TCP
+stream protocols.
+
+*How to check:*
+
+.. code-block:: c
+
+ if (errata & (1 << 0)) {
+ /* Erratum 1 is fixed - TCP restrictions only apply to TCP */
+ /* Safe to use non-TCP stream protocols */
+ }
+
+**Erratum 2: Scoped signal handling (ABI 6)**
+
+Fixed an issue where signal scoping (``LANDLOCK_SCOPE_SIGNAL``) was overly
+restrictive, preventing sandboxed threads from signaling other threads within
+the same process if they belonged to different Landlock domains.
+
+*Impact:* Without this fix, signal scoping could break multi-threaded
+applications that expect threads within the same process to freely signal
+each other, as documented in :manpage:`nptl(7)` and :manpage:`libpsx(3)`.
+
+*How to check:*
+
+.. code-block:: c
+
+ if (errata & (1 << 1)) {
+ /* Erratum 2 is fixed - threads can signal within same process */
+ /* Safe to use LANDLOCK_SCOPE_SIGNAL with multi-threaded apps */
+ }
+
+**Erratum 3: Disconnected directory handling (ABI 1)**
+
+Fixed an issue with disconnected directories that occur when a directory is
+moved outside the scope of a bind mount. The fix ensures that evaluated access
+rights include both those from the disconnected file hierarchy down to its
+filesystem root and those from the related mount point hierarchy.
+
+*Impact:* Without this fix, it was possible to widen access rights through
+rename or link actions involving disconnected directories, potentially
+bypassing ``LANDLOCK_ACCESS_FS_REFER`` restrictions.
+
+*How to check:*
+
+.. code-block:: c
+
+ if (errata & (1 << 2)) {
+ /* Erratum 3 is fixed - disconnected directories handled correctly */
+ /* LANDLOCK_ACCESS_FS_REFER restrictions cannot be bypassed */
+ }
+
+When to Check Errata
+
+Applications should check for specific errata when:
+
+1. Using features that were relaxed or had their behavior changed (like
+ erratum 2 with signal scoping in multi-threaded applications).
+2. Relying on specific security guarantees that may not have been fully
+ enforced in earlier implementations (like erratum 3 with refer restrictions).
+3. Using network restrictions and need to ensure other protocols aren't
+ incorrectly blocked (erratum 1).
+
+Most applications using Landlock's best-effort approach don't need to check
+errata, as the fixes generally make Landlock less restrictive or more correct,
+not more restrictive.
+
The following kernel interfaces are implicitly supported by the first ABI
version. Features only supported from a specific version are explicitly marked
as such.
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 0116e9f93ffe..cf5ba7715916 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -157,9 +157,11 @@ static const struct file_operations ruleset_fops = {
/*
* The Landlock ABI version should be incremented for each new Landlock-related
* user space visible change (e.g. Landlock syscalls). This version should
- * only be incremented once per Linux release, and the date in
+ * only be incremented once per Linux release. When incrementing, the date in
* Documentation/userspace-api/landlock.rst should be updated to reflect the
* UAPI change.
+ * If the change involves a fix that requires userspace awareness, also update
+ * the errata documentation in Documentation/userspace-api/landlock.rst.
*/
const int landlock_abi_version = 7;
--
2.50.1
^ permalink raw reply related
* [PATCH 3/3] landlock: Document audit blocker field format
From: Samasth Norway Ananda @ 2025-12-16 21:02 UTC (permalink / raw)
To: mic, gnoack; +Cc: linux-security-module, linux-kernel
In-Reply-To: <20251216210248.4150777-1-samasth.norway.ananda@oracle.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3175 bytes --]
Add comprehensive documentation for the ``blockers`` field format
in AUDIT_LANDLOCK_ACCESS records, including all possible prefixes
(fs., net., scope.) and their meanings.
Also fix a typo and update the documentation date to reflect these
changes.
Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
Documentation/admin-guide/LSM/landlock.rst | 34 ++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/LSM/landlock.rst b/Documentation/admin-guide/LSM/landlock.rst
index 9e61607def08..f1ea67cff9da 100644
--- a/Documentation/admin-guide/LSM/landlock.rst
+++ b/Documentation/admin-guide/LSM/landlock.rst
@@ -6,7 +6,7 @@ Landlock: system-wide management
================================
:Author: Mickaël Salaün
-:Date: March 2025
+:Date: December 2025
Landlock can leverage the audit framework to log events.
@@ -38,6 +38,36 @@ AUDIT_LANDLOCK_ACCESS
domain=195ba459b blockers=fs.refer path="/usr/bin" dev="vda2" ino=351
domain=195ba459b blockers=fs.make_reg,fs.refer path="/usr/local" dev="vda2" ino=365
+ The ``blockers`` field uses dot-separated prefixes to indicate the type of
+ restriction that caused the denial:
+
+ **fs.*** - Filesystem access rights (ABI 1+):
+ - fs.execute, fs.write_file, fs.read_file, fs.read_dir
+ - fs.remove_dir, fs.remove_file
+ - fs.make_char, fs.make_dir, fs.make_reg, fs.make_sock
+ - fs.make_fifo, fs.make_block, fs.make_sym
+ - fs.refer (ABI 2+)
+ - fs.truncate (ABI 3+)
+ - fs.ioctl_dev (ABI 5+)
+
+ **net.*** - Network access rights (ABI 4+):
+ - net.bind_tcp - TCP port binding was denied
+ - net.connect_tcp - TCP connection was denied
+
+ **scope.*** - IPC scoping restrictions (ABI 6+):
+ - scope.abstract_unix_socket - Abstract UNIX socket connection denied
+ - scope.signal - Signal sending denied
+
+ Multiple blockers can appear in a single event (comma-separated) when
+ multiple access rights are missing. For example, creating a regular file
+ in a directory that lacks both ``make_reg`` and ``refer`` rights would show
+ ``blockers=fs.make_reg,fs.refer``.
+
+ The object identification fields (path, dev, ino for filesystem; opid,
+ ocomm for signals) depend on the type of access being blocked and provide
+ context about what resource was involved in the denial.
+
+
AUDIT_LANDLOCK_DOMAIN
This record type describes the status of a Landlock domain. The ``status``
field can be either ``allocated`` or ``deallocated``.
@@ -86,7 +116,7 @@ This command generates two events, each identified with a unique serial
number following a timestamp (``msg=audit(1729738800.268:30)``). The first
event (serial ``30``) contains 4 records. The first record
(``type=LANDLOCK_ACCESS``) shows an access denied by the domain `1a6fdc66f`.
-The cause of this denial is signal scopping restriction
+The cause of this denial is signal scoping restriction
(``blockers=scope.signal``). The process that would have receive this signal
is the init process (``opid=1 ocomm="systemd"``).
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v8 04/12] tpm: Change tpm_get_random() opportunistic
From: Jarkko Sakkinen @ 2025-12-16 22:03 UTC (permalink / raw)
To: linux-integrity
Cc: David S . Miller, Herbert Xu, Eric Biggers, 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: <20251216092147.2326606-5-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 11:21:38AM +0200, Jarkko Sakkinen wrote:
> 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>
> Cc: Eric Biggers <ebiggers@kernel.org>
> 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.
> ---
IMHO, these first four patches that add sanity to tpm_get_random() usage
ought to be somewhat "dead obvious" category.
Once these are merged it is much easier to further reduce the frequency
of tpm_get_random() calls made, and test their effect on e.g. call
frequency (as tpm_get_random() is now much more tighly bound to hwrng
requirements).
> 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
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Paul Moore @ 2025-12-16 22:57 UTC (permalink / raw)
To: Ryan Foster, serge; +Cc: linux-security-module, linux-kernel
In-Reply-To: <20251204215610.148342-1-foster.ryan.r@gmail.com>
On Thu, Dec 4, 2025 at 4:56 PM Ryan Foster <foster.ryan.r@gmail.com> wrote:
>
> Add comprehensive KUnit tests for the namespace-related capability
> functions that Serge Hallyn refactored in commit 9891d2f79a9f
> ("Clarify the rootid_owns_currentns").
>
> The tests verify:
> - Basic functionality: UID 0 in init namespace, invalid vfsuid, non-zero UIDs
> - Actual namespace traversal: Creating user namespaces with different UID
> mappings where uid 0 maps to different kuids (e.g., 1000, 2000, 3000)
> - Hierarchy traversal: Testing multiple nested namespaces to verify
> correct namespace hierarchy traversal
>
> This addresses the feedback to "test the actual functionality" by creating
> real user namespaces with different values for the namespace's uid 0, rather
> than just basic input validation.
>
> The test file is included at the end of commoncap.c when
> CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, following the standard
> kernel pattern (e.g., scsi_lib.c, ext4/mballoc.c). This allows tests to
> access static functions in the same compilation unit without modifying
> production code based on test configuration.
>
> All 7 tests pass:
> - test_vfsuid_root_in_currentns_init_ns
> - test_vfsuid_root_in_currentns_invalid
> - test_vfsuid_root_in_currentns_nonzero
> - test_kuid_root_in_ns_init_ns_uid0
> - test_kuid_root_in_ns_init_ns_nonzero
> - test_kuid_root_in_ns_with_mapping
> - test_kuid_root_in_ns_with_different_mappings
> ---
> security/Kconfig | 17 +++
> security/commoncap.c | 4 +
> security/commoncap_test.c | 290 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 311 insertions(+)
> create mode 100644 security/commoncap_test.c
You'll need to sort this out with Serge, but I would suggest adding
security/commoncap_test.c to the CAPABILITIES entry in the MAINTAINERS
file so it has a proper home.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] nfs: unify security_inode_listsecurity() calls
From: Paul Moore @ 2025-12-16 23:03 UTC (permalink / raw)
To: Stephen Smalley, anna, trondmy
Cc: okorniev, linux-nfs, linux-security-module, selinux
In-Reply-To: <20251203195728.8592-1-stephen.smalley.work@gmail.com>
On Wed, Dec 3, 2025 at 2:58 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> commit 243fea134633 ("NFSv4.2: fix listxattr to return selinux
> security label") introduced a direct call to
> security_inode_listsecurity() in nfs4_listxattr(). However,
> nfs4_listxattr() already indirectly called
> security_inode_listsecurity() via nfs4_listxattr_nfs4_label() if
> CONFIG_NFS_V4_SECURITY_LABEL is enabled and the server has the
> NFS_CAP_SECURITY_LABEL capability enabled. This duplication was fixed
> by commit 9acb237deff7 ("NFSv4.2: another fix for listxattr") by
> making the second call conditional on NFS_CAP_SECURITY_LABEL not being
> set by the server. However, the combination of the two changes
> effectively makes one call to security_inode_listsecurity() in every
> case - which is the desired behavior since getxattr() always returns a
> security xattr even if it has to synthesize one. Further, the two
> different calls produce different xattr name ordering between
> security.* and user.* xattr names. Unify the two separate calls into a
> single call and get rid of nfs4_listxattr_nfs4_label() altogether.
>
> Link: https://lore.kernel.org/selinux/CAEjxPJ6e8z__=MP5NfdUxkOMQ=EnUFSjWFofP4YPwHqK=Ki5nw@mail.gmail.com/
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> fs/nfs/nfs4proc.c | 38 +++-----------------------------------
> 1 file changed, 3 insertions(+), 35 deletions(-)
NFS folks, any thoughts on this? We'd like to clean up the
security_inode_listsecurity() interface (see the Link: metadata
above), but we need to sort this out first.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC 00/11] Reintroduce Hornet LSM
From: Paul Moore @ 2025-12-17 2:27 UTC (permalink / raw)
To: ryan foster
Cc: bboscaccy, James.Bottomley, akpm, bpf, corbet, dhowells, gnoack,
jmorris, linux-doc, linux-kernel, linux-security-module, linux,
mic, serge
In-Reply-To: <CAHtS32-Zh3knxSdR=DUqQH4rX4QU8ewgu+KHGq6Af3qs9S0FAg@mail.gmail.com>
On Mon, Dec 15, 2025 at 12:26 PM ryan foster <foster.ryan.r@gmail.com> wrote:
>
> Hi all,
>
> I want to confirm I understand the current semantics, and specific issues this series is addressing.
I don't want to speak for Blaise (or James for that matter), but my
understanding is that Hornet is focused on ensuring BPF program
integrity at load time; similar to KP's signature scheme which has
recently found its way into Linus tree. Where KP's and Blaise's
scheme differ is in how they perform the integrity checks.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 5/6] keys/trusted_keys: establish PKWM as a trusted source
From: Srish Srinivasan @ 2025-12-17 5:18 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, keyrings, linuxppc-dev, maddy, mpe, npiggin,
christophe.leroy, James.Bottomley, zohar, nayna, rnsastry,
linux-kernel, linux-security-module
In-Reply-To: <aT286Y6UYCth0--4@kernel.org>
Hi Jarkko,
thank you for taking a look and providing feedback.
On 12/14/25 12:52 AM, Jarkko Sakkinen wrote:
> On Sat, Dec 13, 2025 at 10:56:17AM +0530, Srish Srinivasan wrote:
>> The wrapping key does not exist by default and is generated by the
>> hypervisor as a part of PKWM initialization. This key is then persisted by
>> the hypervisor and is used to wrap trusted keys. These are variable length
>> symmetric keys, which in the case of PowerVM Key Wrapping Module (PKWM) are
>> generated using the kernel RNG. PKWM can be used as a trust source through
>> the following example keyctl command
>>
>> keyctl add trusted my_trusted_key "new 32" @u
>>
>> Use the wrap_flags command option to set the secure boot requirement for
>> the wrapping request through the following keyctl commands
>>
>> case1: no secure boot requirement. (default)
>> keyctl usage: keyctl add trusted my_trusted_key "new 32" @u
>> OR
>> keyctl add trusted my_trusted_key "new 32 wrap_flags=0x00" @u
>>
>> case2: secure boot required to in either audit or enforce mode. set bit 0
>> keyctl usage: keyctl add trusted my_trusted_key "new 32 wrap_flags=0x01" @u
>>
>> case3: secure boot required to be in enforce mode. set bit 1
>> keyctl usage: keyctl add trusted my_trusted_key "new 32 wrap_flags=0x02" @u
>>
>> NOTE:
>> -> Setting the secure boot requirement is NOT a must.
>> -> Only either of the secure boot requirement options should be set. Not
>> both.
>> -> All the other bits are requied to be not set.
>> -> Set the kernel parameter trusted.source=pkwm to choose PKWM as the
>> backend for trusted keys implementation.
>> -> CONFIG_PSERIES_PLPKS must be enabled to build PKWM.
>>
>> Add PKWM, which is a combination of IBM PowerVM and Power LPAR Platform
>> KeyStore, as a new trust source for trusted keys.
>>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> ---
>> MAINTAINERS | 9 ++
>> include/keys/trusted-type.h | 7 +-
>> include/keys/trusted_pkwm.h | 30 ++++
>> security/keys/trusted-keys/Kconfig | 8 ++
>> security/keys/trusted-keys/Makefile | 2 +
>> security/keys/trusted-keys/trusted_core.c | 6 +-
>> security/keys/trusted-keys/trusted_pkwm.c | 168 ++++++++++++++++++++++
>> 7 files changed, 228 insertions(+), 2 deletions(-)
>> create mode 100644 include/keys/trusted_pkwm.h
>> create mode 100644 security/keys/trusted-keys/trusted_pkwm.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index aff3e162180d..bf78ab78a309 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13993,6 +13993,15 @@ S: Supported
>> F: include/keys/trusted_dcp.h
>> F: security/keys/trusted-keys/trusted_dcp.c
>>
>> +KEYS-TRUSTED-PLPKS
>> +M: Srish Srinivasan <ssrish@linux.ibm.com>
>> +M: Nayna Jain <nayna@linux.ibm.com>
>> +L: linux-integrity@vger.kernel.org
>> +L: keyrings@vger.kernel.org
>> +S: Supported
>> +F: include/keys/trusted_plpks.h
>> +F: security/keys/trusted-keys/trusted_pkwm.c
>> +
>> KEYS-TRUSTED-TEE
>> M: Sumit Garg <sumit.garg@kernel.org>
>> L: linux-integrity@vger.kernel.org
>> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
>> index 4eb64548a74f..45c6c538df22 100644
>> --- a/include/keys/trusted-type.h
>> +++ b/include/keys/trusted-type.h
>> @@ -19,7 +19,11 @@
>>
>> #define MIN_KEY_SIZE 32
>> #define MAX_KEY_SIZE 128
>> -#define MAX_BLOB_SIZE 512
>> +#if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
>> +#define MAX_BLOB_SIZE 1152
>> +#else
>> +#define MAX_BLOB_SIZE 512
>> +#endif
>> #define MAX_PCRINFO_SIZE 64
>> #define MAX_DIGEST_SIZE 64
>>
>> @@ -46,6 +50,7 @@ struct trusted_key_options {
>> uint32_t policydigest_len;
>> unsigned char policydigest[MAX_DIGEST_SIZE];
>> uint32_t policyhandle;
>> + uint16_t wrap_flags;
>> };
>>
>> struct trusted_key_ops {
>> diff --git a/include/keys/trusted_pkwm.h b/include/keys/trusted_pkwm.h
>> new file mode 100644
>> index 000000000000..736edfc1e1dd
>> --- /dev/null
>> +++ b/include/keys/trusted_pkwm.h
>> @@ -0,0 +1,30 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __PKWM_TRUSTED_KEY_H
>> +#define __PKWM_TRUSTED_KEY_H
>> +
>> +#include <keys/trusted-type.h>
>> +
>> +extern struct trusted_key_ops pkwm_trusted_key_ops;
>> +
>> +#define PKWM_DEBUG 0
>> +
>> +#if PKWM_DEBUG
>> +static inline void dump_options(struct trusted_key_options *o)
>> +{
>> + bool sb_audit_or_enforce_bit = o->policyhandle & BIT(0);
>> + bool sb_enforce_bit = o->policyhandle & BIT(1);
>> +
>> + if (sb_audit_or_enforce_bit)
>> + pr_info("secure boot mode: audit or enforce");
>> + else if (sb_enforce_bit)
>> + pr_info("secure boot mode: enforce");
>> + else
>> + pr_info("secure boot mode: disabled");
>> +}
>> +#else
>> +static inline void dump_options(struct trusted_key_options *o)
>> +{
>> +}
>> +#endif
> Please use pr_debug() instead of emulating this with 'PKWM_DEBUG'.
Sure, I will fix this.
>> +
>> +#endif
>> diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
>> index 204a68c1429d..9e00482d886a 100644
>> --- a/security/keys/trusted-keys/Kconfig
>> +++ b/security/keys/trusted-keys/Kconfig
>> @@ -46,6 +46,14 @@ config TRUSTED_KEYS_DCP
>> help
>> Enable use of NXP's DCP (Data Co-Processor) as trusted key backend.
>>
>> +config TRUSTED_KEYS_PKWM
>> + bool "PKWM-based trusted keys"
>> + depends on PSERIES_PLPKS >= TRUSTED_KEYS
>> + default y
>> + select HAVE_TRUSTED_KEYS
>> + help
>> + Enable use of IBM PowerVM Key Wrapping Module (PKWM) as a trusted key backend.
>> +
>> if !HAVE_TRUSTED_KEYS
>> comment "No trust source selected!"
>> endif
>> diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
>> index f0f3b27f688b..5fc053a21dad 100644
>> --- a/security/keys/trusted-keys/Makefile
>> +++ b/security/keys/trusted-keys/Makefile
>> @@ -16,3 +16,5 @@ trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
>> trusted-$(CONFIG_TRUSTED_KEYS_CAAM) += trusted_caam.o
>>
>> trusted-$(CONFIG_TRUSTED_KEYS_DCP) += trusted_dcp.o
>> +
>> +trusted-$(CONFIG_TRUSTED_KEYS_PKWM) += trusted_pkwm.o
>> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
>> index b1680ee53f86..2d328de170e8 100644
>> --- a/security/keys/trusted-keys/trusted_core.c
>> +++ b/security/keys/trusted-keys/trusted_core.c
>> @@ -12,6 +12,7 @@
>> #include <keys/trusted_caam.h>
>> #include <keys/trusted_dcp.h>
>> #include <keys/trusted_tpm.h>
>> +#include <keys/trusted_pkwm.h>
>> #include <linux/capability.h>
>> #include <linux/err.h>
>> #include <linux/init.h>
>> @@ -31,7 +32,7 @@ MODULE_PARM_DESC(rng, "Select trusted key RNG");
>>
>> static char *trusted_key_source;
>> module_param_named(source, trusted_key_source, charp, 0);
>> -MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam or dcp)");
>> +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam, dcp or pkwm)");
>>
>> static const struct trusted_key_source trusted_key_sources[] = {
>> #if defined(CONFIG_TRUSTED_KEYS_TPM)
>> @@ -46,6 +47,9 @@ static const struct trusted_key_source trusted_key_sources[] = {
>> #if defined(CONFIG_TRUSTED_KEYS_DCP)
>> { "dcp", &dcp_trusted_key_ops },
>> #endif
>> +#if defined(CONFIG_TRUSTED_KEYS_PKWM)
>> + { "pkwm", &pkwm_trusted_key_ops },
>> +#endif
>> };
>>
>> DEFINE_STATIC_CALL_NULL(trusted_key_seal, *trusted_key_sources[0].ops->seal);
>> diff --git a/security/keys/trusted-keys/trusted_pkwm.c b/security/keys/trusted-keys/trusted_pkwm.c
>> new file mode 100644
>> index 000000000000..7968601dcf42
>> --- /dev/null
>> +++ b/security/keys/trusted-keys/trusted_pkwm.c
>> @@ -0,0 +1,168 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
>> + */
>> +
>> +#include <keys/trusted_pkwm.h>
>> +#include <keys/trusted-type.h>
>> +#include <linux/build_bug.h>
>> +#include <linux/key-type.h>
>> +#include <linux/parser.h>
>> +#include <asm/plpks.h>
>> +
>> +enum {
>> + Opt_err,
>> + Opt_wrap_flags,
>> +};
>> +
>> +static const match_table_t key_tokens = {
>> + {Opt_wrap_flags, "wrap_flags=%s"},
>> + {Opt_err, NULL}
>> +};
>> +
>> +static int getoptions(char *datablob, struct trusted_key_options **opt)
>> +{
>> + substring_t args[MAX_OPT_ARGS];
>> + char *p = datablob;
>> + int token;
>> + int res;
>> + unsigned long wrap_flags;
>> + unsigned long token_mask = 0;
>> +
>> + if (!datablob)
>> + return 0;
>> +
>> + while ((p = strsep(&datablob, " \t"))) {
>> + if (*p == '\0' || *p == ' ' || *p == '\t')
>> + continue;
>> +
>> + token = match_token(p, key_tokens, args);
>> + if (test_and_set_bit(token, &token_mask))
>> + return -EINVAL;
>> +
>> + switch (token) {
>> + case Opt_wrap_flags:
>> + res = kstrtoul(args[0].from, 16, &wrap_flags);
>> + if (res < 0 || wrap_flags > 2)
>> + return -EINVAL;
>> + (*opt)->wrap_flags = wrap_flags;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +static struct trusted_key_options *trusted_options_alloc(void)
>> +{
>> + struct trusted_key_options *options;
>> +
>> + options = kzalloc(sizeof(*options), GFP_KERNEL);
>> + return options;
>> +}
>> +
>> +static int trusted_pkwm_seal(struct trusted_key_payload *p, char *datablob)
>> +{
>> + struct trusted_key_options *options = NULL;
>> + u8 *input_buf, *output_buf;
>> + u32 output_len, input_len;
>> + int rc;
>> +
>> + options = trusted_options_alloc();
>> + if (!options)
>> + return -ENOMEM;
>> +
>> + rc = getoptions(datablob, &options);
>> + if (rc < 0)
>> + goto out;
>> + dump_options(options);
>> +
>> + input_len = p->key_len;
>> + input_buf = kmalloc(ALIGN(input_len, 4096), GFP_KERNEL);
>> + if (!input_buf) {
>> + pr_err("Input buffer allocation failed. Returning -ENOMEM.");
>> + return -ENOMEM;
>> + }
>> +
>> + memcpy(input_buf, p->key, p->key_len);
>> +
>> + rc = plpks_wrap_object(&input_buf, input_len, options->wrap_flags,
>> + &output_buf, &output_len);
>> + if (!rc) {
>> + memcpy(p->blob, output_buf, output_len);
>> + p->blob_len = output_len;
>> + dump_payload(p);
>> + } else {
>> + pr_err("Invalid argument");
>> + }
>> +
>> + kfree(input_buf);
>> + kfree(output_buf);
>> +
>> +out:
>> + kfree_sensitive(options);
>> + return rc;
>> +}
>> +
>> +static int trusted_pkwm_unseal(struct trusted_key_payload *p, char *datablob)
>> +{
>> + u8 *input_buf, *output_buf;
>> + u32 input_len, output_len;
>> + int rc;
>> +
>> + input_len = p->blob_len;
>> + input_buf = kmalloc(ALIGN(input_len, 4096), GFP_KERNEL);
>> + if (!input_buf)
>> + return -ENOMEM;
>> +
>> + memcpy(input_buf, p->blob, p->blob_len);
>> +
>> + rc = plpks_unwrap_object(&input_buf, input_len, &output_buf,
>> + &output_len);
>> + if (!rc) {
>> + memcpy(p->key, output_buf, output_len);
>> + p->key_len = output_len;
>> + dump_payload(p);
>> + } else {
>> + pr_err("Invalid argument");
> I don't get this error message. What does this mean? pr_err() is used
> when you have actual malfunction.
Yes, the error message lacks clarity here.
It was meant to communicate that the unwrap operation failed.
I will provide a more meaningful error message with the error code
as that would be more helpful.
And thanks for pointing this out.
>
>> + }
>> +
>> + kfree(input_buf);
>> + kfree(output_buf);
>> +
>> + return rc;
>> +}
>> +
>> +static int trusted_pkwm_init(void)
>> +{
>> + int ret;
>> +
>> + if (!plpks_wrapping_is_supported()) {
>> + pr_err("H_PKS_WRAP_OBJECT interface not supported\n");
>> +
>> + return -ENODEV;
>> + }
>> +
>> + ret = plpks_gen_wrapping_key();
>> + if (ret) {
>> + pr_err("Failed to generate default wrapping key\n");
>> +
>> + return -EINVAL;
>> + }
>> +
>> + return register_key_type(&key_type_trusted);
>> +}
>> +
>> +static void trusted_pkwm_exit(void)
>> +{
>> + unregister_key_type(&key_type_trusted);
>> +}
>> +
>> +struct trusted_key_ops pkwm_trusted_key_ops = {
>> + .migratable = 0, /* non-migratable */
>> + .init = trusted_pkwm_init,
>> + .seal = trusted_pkwm_seal,
>> + .unseal = trusted_pkwm_unseal,
>> + .exit = trusted_pkwm_exit,
>> +};
>> --
>> 2.47.3
>>
> BR, Jarkko
>
thanks,
Srish.
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-17 7:55 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: <ayebinxqpcnl7hpa35ytrudiy7j75u5bdk3enlirkp5pevppeg@6mx6a5fwymwf>
On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > <u.kleine-koenig@baylibre.com> wrote:
> > > On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > > > 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.
>
> Don't consider me in for that. But it sounds like a nice addition.
>
No worries, the current cleanup is fine for now.
> > > 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.
>
> So only OP-TEE uses the bus for devices and the other *-TEE don't. Also
> sounds like something worth to be fixed.
The TEE bus is common for all the TEE implementation drivers which need
to support kernel TEE client drivers. I am aware there will be QTEE and
TS-TEE from past discussion that they will be exposing devices on the
TEE 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.
>
> From reading that once I don't understand it. (But no need to explain
> :-)
>
> Still the file should better be added before device_add() is called.
Noted, let me see if I can get to fix this until someone jumps in before
me.
>
> > > - 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.
>
> Without understanding the tee stuff, I'd say: Don't bother and only undo
> the things that probe did before the failure.
>
True, but this is special case where if there is any leftover device
registered from the TEE implementation then it is likely going to cause
the corresponding kernel client driver crash.
-Sumit
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-17 8:21 UTC (permalink / raw)
To: Sumit Garg
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: <aUJh--HGVeJWIilS@sumit-xelite>
[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]
Hello Sumit,
On Wed, Dec 17, 2025 at 01:25:39PM +0530, Sumit Garg wrote:
> On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> > On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > > <u.kleine-koenig@baylibre.com> wrote:
> > > > - 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.
> >
> > Without understanding the tee stuff, I'd say: Don't bother and only undo
> > the things that probe did before the failure.
>
> True, but this is special case where if there is any leftover device
> registered from the TEE implementation then it is likely going to cause
> the corresponding kernel client driver crash.
You are aware that this is racy? So if a driver crashes e.g. after
teedev_close_context() it might happen that it is registered just after
optee_unregister_devices() returns.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-17 9:02 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: <max5wxkcjjvnftwfwgymybwbnvf5s3ytwpy4oo5i74kfvnav4m@m2wasqyxsf4h>
On Wed, Dec 17, 2025 at 09:21:41AM +0100, Uwe Kleine-König wrote:
> Hello Sumit,
>
> On Wed, Dec 17, 2025 at 01:25:39PM +0530, Sumit Garg wrote:
> > On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> > > On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > > > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > > - 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.
> > >
> > > Without understanding the tee stuff, I'd say: Don't bother and only undo
> > > the things that probe did before the failure.
> >
> > True, but this is special case where if there is any leftover device
> > registered from the TEE implementation then it is likely going to cause
> > the corresponding kernel client driver crash.
>
> You are aware that this is racy? So if a driver crashes e.g. after
> teedev_close_context() it might happen that it is registered just after
> optee_unregister_devices() returns.
>
I see your point about the unavoidable race. Maybe it's better to not
try anything and let the kernel client driver fail.
-Sumit
^ 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