* [PATCH 4/4] tpm2-sessions: Open code tpm_buf_append_hmac_session()
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jarkko Sakkinen,
Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251201224554.1717104-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Open code 'tpm_buf_append_hmac_session_opt' to the call site, as it only
masks a call sequence and does otherwise nothing particularly useful.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
---
drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++---
include/linux/tpm.h | 23 -----------------------
security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
3 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index f1e9c35f13a2..94cb887cb4a9 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -270,9 +270,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
do {
tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
- | TPM2_SA_CONTINUE_SESSION,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf,
+ TPM2_SA_ENCRYPT |
+ TPM2_SA_CONTINUE_SESSION,
+ NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
tpm_buf_append_u16(&buf, num_bytes);
err = tpm_buf_fill_hmac_session(chip, &buf);
if (err) {
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index e3a6e2fb41a7..a93df0efebe4 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -535,29 +535,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
u8 *passphrase, int passphraselen);
-static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
- struct tpm_buf *buf,
- u8 attributes,
- u8 *passphrase,
- int passphraselen)
-{
- struct tpm_header *head;
- int offset;
-
- if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
- } else {
- offset = buf->handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf->data;
-
- /*
- * If the only sessions are optional, the command tag must change to
- * TPM2_ST_NO_SESSIONS.
- */
- if (tpm_buf_length(buf) == offset)
- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
- }
-}
#ifdef CONFIG_TCG_TPM2_HMAC
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 7672a4376dad..e6b95111ac7d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -494,8 +494,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle)
{
+ struct tpm_header *head;
struct tpm_buf buf;
u16 data_len;
+ int offset;
u8 *data;
int rc;
@@ -532,8 +534,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm2_buf_append_auth(&buf, options->policyhandle,
NULL /* nonce */, 0, 0,
options->blobauth, options->blobauth_len);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
}
rc = tpm_buf_fill_hmac_session(chip, &buf);
--
2.52.0
^ permalink raw reply related
* [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-01 22:45 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, stable, Peter Huewe, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, Ard Biesheuvel, open list,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251201224554.1717104-1-jarkko@kernel.org>
'name_size' does not have any range checks, and it just directly indexes
with TPM_ALG_ID, which could lead into memory corruption at worst.
Address the issue by only processing known values and returning -EINVAL for
unrecognized values.
Make also 'tpm_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible so
that errors are detected before causing any spurious TPM traffic.
End also the authorization session on failure in both of the functions, as
the session state would be then by definition corrupted.
Cc: stable@vger.kernel.org # v6.10+
Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
- Add two missing 'tpm2_end_auth_session' calls to the fallback paths of
'tpm_buf_fill_hmac_session'.
- Rewrote the commit message.
- End authorization session on failure in 'tpm2_buf_append_name' and
'tpm_buf_fill_hmac_session'.
v2:
There was spurious extra field added to tpm2_hash by mistake.
---
drivers/char/tpm/tpm2-cmd.c | 23 +++-
drivers/char/tpm/tpm2-sessions.c | 131 +++++++++++++++-------
include/linux/tpm.h | 6 +-
security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
4 files changed, 136 insertions(+), 53 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 5b6ccf901623..4473b81122e8 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -187,7 +187,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
}
if (!disable_pcr_integrity) {
- tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+ rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return rc;
+ }
tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
} else {
tpm_buf_append_handle(chip, &buf, pcr_idx);
@@ -202,8 +206,14 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
chip->allocated_banks[i].digest_size);
}
- if (!disable_pcr_integrity)
- tpm_buf_fill_hmac_session(chip, &buf);
+ if (!disable_pcr_integrity) {
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return rc;
+ }
+ }
+
rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
if (!disable_pcr_integrity)
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
@@ -261,7 +271,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
| TPM2_SA_CONTINUE_SESSION,
NULL, 0);
tpm_buf_append_u16(&buf, num_bytes);
- tpm_buf_fill_hmac_session(chip, &buf);
+ err = tpm_buf_fill_hmac_session(chip, &buf);
+ if (err) {
+ tpm_buf_destroy(&buf);
+ return err;
+ }
+
err = tpm_transmit_cmd(chip, &buf,
offsetof(struct tpm2_get_random_out,
buffer),
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 6d03c224e6b2..33ad0d668e1a 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -144,16 +144,24 @@ struct tpm2_auth {
/*
* Name Size based on TPM algorithm (assumes no hash bigger than 255)
*/
-static u8 name_size(const u8 *name)
+static int name_size(const u8 *name)
{
- static u8 size_map[] = {
- [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
- [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
- [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
- [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
- };
- u16 alg = get_unaligned_be16(name);
- return size_map[alg] + 2;
+ u16 hash_alg = get_unaligned_be16(name);
+
+ switch (hash_alg) {
+ case TPM_ALG_SHA1:
+ return SHA1_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA256:
+ return SHA256_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA384:
+ return SHA384_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA512:
+ return SHA512_DIGEST_SIZE + 2;
+ case TPM_ALG_SM3_256:
+ return SM3256_DIGEST_SIZE + 2;
+ }
+
+ return -EINVAL;
}
static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
@@ -161,6 +169,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
struct tpm_header *head = (struct tpm_header *)buf->data;
off_t offset = TPM_HEADER_SIZE;
u32 tot_len = be32_to_cpu(head->length);
+ int ret;
u32 val;
/* we're starting after the header so adjust the length */
@@ -172,9 +181,15 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
return -EINVAL;
offset += val;
/* name */
+
val = tpm_buf_read_u16(buf, &offset);
- if (val != name_size(&buf->data[offset]))
+ ret = name_size(&buf->data[offset]);
+ if (ret < 0)
+ return ret;
+
+ if (val != ret)
return -EINVAL;
+
memcpy(name, &buf->data[offset], val);
/* forget the rest */
return 0;
@@ -221,46 +236,70 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
* As with most tpm_buf operations, success is assumed because failure
* will be caused by an incorrect programming model and indicated by a
* kernel message.
+ *
+ * Ends the authorization session on failure.
*/
-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name)
+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
+ u32 handle, u8 *name)
{
#ifdef CONFIG_TCG_TPM2_HMAC
enum tpm2_mso_type mso = tpm2_handle_mso(handle);
struct tpm2_auth *auth;
int slot;
+ int ret;
#endif
if (!tpm2_chip_auth(chip)) {
tpm_buf_append_handle(chip, buf, handle);
- return;
+ return 0;
}
#ifdef CONFIG_TCG_TPM2_HMAC
slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
if (slot >= AUTH_MAX_NAMES) {
- dev_err(&chip->dev, "TPM: too many handles\n");
- return;
+ dev_err(&chip->dev, "too many handles\n");
+ ret = -EIO;
+ goto err;
}
auth = chip->auth;
- WARN(auth->session != tpm_buf_length(buf),
- "name added in wrong place\n");
+ if (auth->session != tpm_buf_length(buf)) {
+ dev_err(&chip->dev, "session state malformed");
+ ret = -EIO;
+ goto err;
+ }
tpm_buf_append_u32(buf, handle);
auth->session += 4;
if (mso == TPM2_MSO_PERSISTENT ||
mso == TPM2_MSO_VOLATILE ||
mso == TPM2_MSO_NVRAM) {
- if (!name)
- tpm2_read_public(chip, handle, auth->name[slot]);
+ if (!name) {
+ ret = tpm2_read_public(chip, handle, auth->name[slot]);
+ if (ret)
+ goto err;
+ }
} else {
- if (name)
- dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
+ if (name) {
+ ret = -EIO;
+ goto err;
+ }
}
auth->name_h[slot] = handle;
- if (name)
- memcpy(auth->name[slot], name, name_size(name));
+ if (name) {
+ ret = name_size(name);
+ if (ret < 0)
+ goto err;
+
+ memcpy(auth->name[slot], name, ret);
+ }
+#endif
+ return 0;
+
+#ifdef CONFIG_TCG_TPM2_HMAC
+err:
+ tpm2_end_auth_session(chip);
+ return tpm_ret_to_err(ret);
#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -533,11 +572,9 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
* encryption key and encrypts the first parameter of the command
* buffer with it.
*
- * As with most tpm_buf operations, success is assumed because failure
- * will be caused by an incorrect programming model and indicated by a
- * kernel message.
+ * Ends the authorization session on failure.
*/
-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
{
u32 cc, handles, val;
struct tpm2_auth *auth = chip->auth;
@@ -549,9 +586,12 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
u8 cphash[SHA256_DIGEST_SIZE];
struct sha256_ctx sctx;
struct hmac_sha256_ctx hctx;
+ int ret;
- if (!auth)
- return;
+ if (!auth) {
+ ret = -EINVAL;
+ goto err;
+ }
/* save the command code in BE format */
auth->ordinal = head->ordinal;
@@ -560,9 +600,10 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
i = tpm2_find_cc(chip, cc);
if (i < 0) {
- dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
- return;
+ ret = -EINVAL;
+ goto err;
}
+
attrs = chip->cc_attrs_tbl[i];
handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
@@ -576,9 +617,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
u32 handle = tpm_buf_read_u32(buf, &offset_s);
if (auth->name_h[i] != handle) {
- dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
- i);
- return;
+ dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
+ ret = -EINVAL;
+ goto err;
}
}
/* point offset_s to the start of the sessions */
@@ -609,12 +650,14 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
offset_s += len;
}
if (offset_s != offset_p) {
- dev_err(&chip->dev, "TPM session length is incorrect\n");
- return;
+ dev_err(&chip->dev, "session length is incorrect\n");
+ ret = -EINVAL;
+ goto err;
}
if (!hmac) {
- dev_err(&chip->dev, "TPM could not find HMAC session\n");
- return;
+ dev_err(&chip->dev, "could not find HMAC session\n");
+ ret = -EINVAL;
+ goto err;
}
/* encrypt before HMAC */
@@ -646,8 +689,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
if (mso == TPM2_MSO_PERSISTENT ||
mso == TPM2_MSO_VOLATILE ||
mso == TPM2_MSO_NVRAM) {
- sha256_update(&sctx, auth->name[i],
- name_size(auth->name[i]));
+ ret = name_size(auth->name[i]);
+ if (ret < 0)
+ goto err;
+
+ sha256_update(&sctx, auth->name[i], ret);
} else {
__be32 h = cpu_to_be32(auth->name_h[i]);
@@ -668,6 +714,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
hmac_sha256_update(&hctx, &auth->attrs, 1);
hmac_sha256_final(&hctx, hmac);
+ return 0;
+
+err:
+ tpm2_end_auth_session(chip);
+ return ret;
}
EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 0e9e043f728c..1a59f0190eb3 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -528,8 +528,8 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
#endif
}
-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name);
+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
+ u32 handle, u8 *name);
void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
@@ -562,7 +562,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
#ifdef CONFIG_TCG_TPM2_HMAC
int tpm2_start_auth_session(struct tpm_chip *chip);
-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
int rc);
void tpm2_end_auth_session(struct tpm_chip *chip);
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index e165b117bbca..7672a4376dad 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -283,7 +283,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out_put;
}
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
+
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
options->keyauth, TPM_DIGEST_SIZE);
@@ -331,7 +334,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (rc)
@@ -438,7 +444,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
return rc;
}
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
+
tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
TPM_DIGEST_SIZE);
@@ -450,7 +459,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
goto out;
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (!rc)
@@ -497,7 +509,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
return rc;
}
- tpm_buf_append_name(chip, &buf, blob_handle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
if (!options->policyhandle) {
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
@@ -522,7 +536,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
NULL, 0);
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
--
2.52.0
^ permalink raw reply related
* [GIT PULL] lsm/lsm-pr-20251201
From: Paul Moore @ 2025-12-02 2:00 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-kernel
Linus,
A larger pull request than usual for the LSM framework, however, all but
one of the commits are tied to one effort:
- Rework the LSM initialization code
What started as a "quick" patch to enable a notification event once
all of the individual LSMs were initialized, snowballed a bit into a
30+ patch patchset when everything was done. Most of the patches,
and diffstat, is due to splitting out the initialization code into
security/lsm_init.c and cleaning up some of the mess that was there.
While not strictly necessary, it does cleanup the code signficantly,
and hopefully makes the upkeep a bit easier in the future.
Aside from the new LSM_STARTED_ALL notification, these changes also
ensure that individual LSM initcalls are only called when the LSM is
enabled at boot time. There should be a minor reduction in boot times
for those who build multiple LSMs into their kernels, but only enable
a subset at boot.
It is worth mentioning that nothing at present makes use of the
LSM_STARTED_ALL notification, but there is work in progress which is
dependent upon LSM_STARTED_ALL.
- Make better use of the seq_put*() helpers in device_cgroup
Paul
--
The following changes since commit 211ddde0823f1442e4ad052a2f30f050145ccada:
Linux 6.18-rc2 (2025-10-19 15:19:16 -1000)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
tags/lsm-pr-20251201
for you to fetch changes up to 9a948eefad594c42717f29824dd40d6dc0b7aa13:
lsm: use unrcu_pointer() for current->cred in security_init()
(2025-11-19 10:32:06 -0500)
----------------------------------------------------------------
lsm/stable-6.19 PR 20251201
----------------------------------------------------------------
Paul Moore (34):
lsm: split the notifier code out into lsm_notifier.c
lsm: split the init code out into lsm_init.c
lsm: consolidate lsm_allowed() and prepare_lsm() into
lsm_prepare()
lsm: introduce looping macros for the initialization code
lsm: integrate report_lsm_order() code into caller
lsm: integrate lsm_early_cred() and lsm_early_task() into caller
lsm: rename ordered_lsm_init() to lsm_init_ordered()
lsm: replace the name field with a pointer to the lsm_id struct
lsm: rename the lsm order variables for consistency
lsm: rework lsm_active_cnt and lsm_idlist[]
lsm: get rid of the lsm_names list and do some cleanup
lsm: rework the LSM enable/disable setter/getter functions
lsm: rename exists_ordered_lsm() to lsm_order_exists()
lsm: rename/rework append_ordered_lsm() into lsm_order_append()
lsm: rename/rework ordered_lsm_parse() to lsm_order_parse()
lsm: cleanup the LSM blob size code
lsm: cleanup initialize_lsm() and rename to lsm_init_single()
lsm: fold lsm_init_ordered() into security_init()
lsm: add/tweak function header comment blocks in lsm_init.c
lsm: cleanup the debug and console output in lsm_init.c
lsm: output available LSMs when debugging
lsm: group lsm_order_parse() with the other lsm_order_*()
functions
lsm: introduce an initcall mechanism into the LSM framework
loadpin: move initcalls to the LSM framework
ipe: move initcalls to the LSM framework
smack: move initcalls to the LSM framework
tomoyo: move initcalls to the LSM framework
safesetid: move initcalls to the LSM framework
apparmor: move initcalls to the LSM framework
lockdown: move initcalls to the LSM framework
selinux: move initcalls to the LSM framework
lsm: consolidate all of the LSM framework initcalls
lsm: add a LSM_STARTED_ALL notification event
lsm: use unrcu_pointer() for current->cred in security_init()
Roberto Sassu (1):
ima,evm: move initcalls to the LSM framework
Thorsten Blum (1):
device_cgroup: Refactor devcgroup_seq_show to use seq_put* helpers
include/linux/lsm_hooks.h | 73 +-
include/linux/security.h | 3
security/Makefile | 2
security/apparmor/apparmorfs.c | 4
security/apparmor/crypto.c | 3
security/apparmor/include/apparmorfs.h | 2
security/apparmor/include/crypto.h | 1
security/apparmor/lsm.c | 11
security/bpf/hooks.c | 2
security/commoncap.c | 2
security/device_cgroup.c | 56 +-
security/inode.c | 46 +
security/integrity/evm/evm_main.c | 5
security/integrity/evm/evm_secfs.c | 11
security/integrity/iint.c | 14
security/integrity/ima/ima_fs.c | 11
security/integrity/ima/ima_main.c | 6
security/integrity/integrity.h | 2
security/ipe/fs.c | 4
security/ipe/ipe.c | 3
security/ipe/ipe.h | 2
security/landlock/setup.c | 2
security/loadpin/loadpin.c | 15
security/lockdown/lockdown.c | 5
security/lsm.h | 58 ++
security/lsm_init.c | 564 ++++++++++++++++++++++
security/lsm_notifier.c | 31 +
security/lsm_syscalls.c | 2
security/min_addr.c | 5
security/safesetid/lsm.c | 3
security/safesetid/lsm.h | 2
security/safesetid/securityfs.c | 3
security/security.c | 623 +------------------------
security/selinux/Makefile | 2
security/selinux/hooks.c | 11
security/selinux/ibpkey.c | 5
security/selinux/include/audit.h | 9
security/selinux/include/initcalls.h | 19
security/selinux/initcalls.c | 52 ++
security/selinux/netif.c | 5
security/selinux/netlink.c | 5
security/selinux/netnode.c | 5
security/selinux/netport.c | 5
security/selinux/selinuxfs.c | 5
security/selinux/ss/services.c | 26 -
security/smack/smack.h | 14
security/smack/smack_lsm.c | 11
security/smack/smack_netfilter.c | 4
security/smack/smackfs.c | 4
security/tomoyo/common.h | 2
security/tomoyo/securityfs_if.c | 4
security/tomoyo/tomoyo.c | 3
security/yama/yama_lsm.c | 2
53 files changed, 1026 insertions(+), 743 deletions(-)
--
paul-moore.com
^ permalink raw reply
* [GIT PULL] selinux/selinux-pr-20251201
From: Paul Moore @ 2025-12-02 2:00 UTC (permalink / raw)
To: Linus Torvalds; +Cc: selinux, linux-security-module, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3465 bytes --]
Linus,
A handful of SELinux changes for v6.19, the summary is below, but I
wanted to call out a merge conflict caused by the recent task/cred
changes sent up during the v6.18-rcX cycle. The conflict does require
some manual intervention to resolve, but it is limited to changing
the "SECCLASS_FILE" parameter in two avc_has_perm() calls to
"isec->sclass" in selinux_bprm_creds_for_exec(). The
selinux-pr-20251201.merge tag contains an example fixup of the merge
conflict.
Here is the pull request summary:
- Improve the granularity of SELinux labeling for memfd files
Currently when creating a memfd file, SELinux treats it the same as
any other tmpfs, or hugetlbfs, file. While simple, the drawback is
that it is not possible to differentiate between memfd and tmpfs
files. This pull request adds a call to the
security_inode_init_security_anon() LSM hook and wires up SELinux to
provide a set of memfd specific access controls, including the ability
to control the execution of memfds.
As usual, the commit message has more information.
- Improve the SELinux AVC lookup performance
Adopt the MurmurHash3 for the SELinux AVC hash function instead of
the custom hash function currently used. MurmurHash3 is already
used for the SELinux access vector table so the impact to the code
is minimal, and performance tests have shown improvements in both
hash distribution and latency.
See the commit message for the performance measurments.
- Introduce a Kconfig option for the SELinux AVC bucket/slot size
While we have the ability to grow the number of AVC hash buckets
today, the size of the buckets (slot size) is fixed at 512. This
pull request makes that slot size configurable at build time through
a new Kconfig knob, CONFIG_SECURITY_SELINUX_AVC_HASH_BITS.
Paul
--
The following changes since commit 211ddde0823f1442e4ad052a2f30f050145ccada:
Linux 6.18-rc2 (2025-10-19 15:19:16 -1000)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
tags/selinux-pr-20251201
for you to fetch changes up to 20d387d7ceab95aade436c363927b3ab81b0be36:
selinux: improve bucket distribution uniformity of avc_hash()
(2025-10-23 18:24:30 -0400)
----------------------------------------------------------------
selinux/stable-6.19 PR 20251201
----------------------------------------------------------------
Hongru Zhang (3):
selinux: Introduce a new config to make avc cache slot size
adjustable
selinux: Move avtab_hash() to a shared location for future reuse
selinux: improve bucket distribution uniformity of avc_hash()
Thiébaud Weksteen (1):
memfd,selinux: call security_inode_init_security_anon()
include/linux/memfd.h | 2
mm/memfd.c | 14 +++++-
security/selinux/Kconfig | 11 ++++
security/selinux/avc.c | 9 ++--
security/selinux/hooks.c | 26 +++++++++--
security/selinux/include/classmap.h | 2
security/selinux/include/hash.h | 47 +++++++++++++++++++++
security/selinux/include/policycap.h | 1
security/selinux/include/policycap_names.h | 1
security/selinux/include/security.h | 5 ++
security/selinux/ss/avtab.c | 39 +----------------
11 files changed, 110 insertions(+), 47 deletions(-)
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] selftests/landlock: Remove invalid unix socket bind()
From: Günther Noack @ 2025-12-02 8:37 UTC (permalink / raw)
To: Matthieu Buffet; +Cc: Mickaël Salaün, linux-security-module
In-Reply-To: <20251201003631.190817-1-matthieu@buffet.re>
Hello!
On Mon, Dec 01, 2025 at 01:36:31AM +0100, Matthieu Buffet wrote:
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index eee814e09dd7..7d378bdf3bce 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -4391,9 +4391,6 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
> cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> ASSERT_LE(0, cli_fd);
>
> - size = offsetof(struct sockaddr_un, sun_path) + strlen(cli_un.sun_path);
> - ASSERT_EQ(0, bind(cli_fd, (struct sockaddr *)&cli_un, size));
> -
> bzero(&cli_un, sizeof(cli_un));
> cli_un.sun_family = AF_UNIX;
> strncpy(cli_un.sun_path, path, sizeof(cli_un.sun_path));
>
> base-commit: 54f9baf537b0a091adad860ec92e3e18e0a0754c
> --
> 2.47.3
>
Reviewed-by: Günther Noack <gnoack@google.com>
It looks like I must have fumbled with the copy&paste in that test,
this bind() call does not make sense in the place where it is and is
not necessary for the test. Apologies for that and thank you for
spotting this!
Optional: In hindsight, I think it would be nice to simplify the way
that we calculate the address length here. I probably mis-read
unix(7) at the time, where a similar formula is used (but with a
"+1"). Re-reading it, it seems that just passing sizeof(cli_un) as
the address length would have been the simpler and less error prone
solution:
From unix(7) (emphasis mine):
The addrlen argument that describes the enclosing sockaddr_un
structure should have a value of at least:
offsetof(struct sockaddr_un, sun_path)+strlen(addr.sun_path)+1
or, more simply, addrlen **can be specified as sizeof(struct
sockaddr_un).**
So, I believe that all the places where we calculate the size in this
function can just disappear and we can directly use sizeof(cli_un) and
sizeof(srv_un) in the bind() and connect() calls instead.
(Let me know whether you want to do it in this change, otherwise I am
also happy to send a follow-up fix.)
Thanks,
—Günther
^ permalink raw reply
* Re: [PATCH] fuse: fix conversion of fuse_reverse_inval_entry() to start_removing()
From: Miklos Szeredi @ 2025-12-02 8:46 UTC (permalink / raw)
To: Al Viro
Cc: Amir Goldstein, NeilBrown, Christian Brauner, Val Packett,
Jan Kara, linux-fsdevel, Jeff Layton, Chris Mason, David Sterba,
David Howells, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Tyler Hicks, Chuck Lever, Olga Kornievskaia,
Dai Ngo, Namjae Jeon, Steve French, Sergey Senozhatsky,
Carlos Maiolino, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Mateusz Guzik,
Lorenzo Stoakes, Stefan Berger, Darrick J. Wong, linux-kernel,
netfs, ecryptfs, linux-nfs, linux-unionfs, linux-cifs, linux-xfs,
linux-security-module, selinux
In-Reply-To: <20251201170813.GH3538@ZenIV>
On Mon, 1 Dec 2025 at 18:08, Al Viro <viro@zeniv.linux.org.uk> wrote:
> Then as far as VFS is concerned, it's an equivalent of "we'd done
> a dcache lookup and revalidate told us to bugger off", which does
> *not* need locking the parent - the same sequence can very well
> happen without touching any inode locks.
Okay.
> IOW, from the point of view of locking protocol changes that's not
> a removal at all.
>
> Or do you need them serialized for fuse-internal purposes?
Not as far as I can see. As to any fuse filesystem being reliant on
this behavior, I think that's unlikely, though it's sort of documented
in the libfuse APIs as:
* To avoid a deadlock this function must not be called in the
* execution path of a related filesystem operation or within any code
* that could hold a lock that could be needed to execute such an
* operation. As of kernel 4.18, a "related operation" is a lookup(),
* symlink(), mknod(), mkdir(), unlink(), rename(), link() or create()
* request for the parent, and a setattr(), unlink(), rmdir(),
* rename(), setxattr(), removexattr(), readdir() or readdirplus()
* request for the inode itself.
Why the locking was added in the first place? Oversight, probably.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jonathan McDowell @ 2025-12-02 16:26 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Stefano Garzarella, stable, Peter Huewe,
Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ard Biesheuvel,
linux-kernel, keyrings, linux-security-module
In-Reply-To: <20251201193958.896358-1-jarkko@kernel.org>
On Mon, Dec 01, 2025 at 09:39:58PM +0200, Jarkko Sakkinen wrote:
>'name_size' does not have any range checks, and it just directly indexes
>with TPM_ALG_ID, which could lead into memory corruption at worst.
>
>Address the issue by only processing known values and returning -EINVAL for
>unrecognized values.
>
>Make also 'tpm_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible so
>that errors are detected before causing any spurious TPM traffic.
>
>End also the authorization session on failure in both of the functions, as
>the session state would be then by definition corrupted.
>
>Cc: stable@vger.kernel.org # v6.10+
>Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
>v3:
>- Add two missing 'tpm2_end_auth_session' calls to the fallback paths of
> 'tpm_buf_fill_hmac_session'.
>- Rewrote the commit message.
>- End authorization session on failure in 'tpm2_buf_append_name' and
> 'tpm_buf_fill_hmac_session'.
>v2:
>There was spurious extra field added to tpm2_hash by mistake.
>---
> drivers/char/tpm/tpm2-cmd.c | 23 +++-
> drivers/char/tpm/tpm2-sessions.c | 131 +++++++++++++++-------
> include/linux/tpm.h | 6 +-
> security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
> 4 files changed, 136 insertions(+), 53 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>index 5b6ccf901623..4473b81122e8 100644
>--- a/drivers/char/tpm/tpm2-cmd.c
>+++ b/drivers/char/tpm/tpm2-cmd.c
>@@ -187,7 +187,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> }
>
> if (!disable_pcr_integrity) {
>- tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
>+ rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
>+ if (rc) {
>+ tpm_buf_destroy(&buf);
>+ return rc;
>+ }
> tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> } else {
> tpm_buf_append_handle(chip, &buf, pcr_idx);
>@@ -202,8 +206,14 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> chip->allocated_banks[i].digest_size);
> }
>
>- if (!disable_pcr_integrity)
>- tpm_buf_fill_hmac_session(chip, &buf);
>+ if (!disable_pcr_integrity) {
>+ rc = tpm_buf_fill_hmac_session(chip, &buf);
>+ if (rc) {
>+ tpm_buf_destroy(&buf);
>+ return rc;
>+ }
>+ }
>+
> rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
> if (!disable_pcr_integrity)
> rc = tpm_buf_check_hmac_response(chip, &buf, rc);
>@@ -261,7 +271,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> | TPM2_SA_CONTINUE_SESSION,
> NULL, 0);
> tpm_buf_append_u16(&buf, num_bytes);
>- tpm_buf_fill_hmac_session(chip, &buf);
>+ err = tpm_buf_fill_hmac_session(chip, &buf);
>+ if (err) {
>+ tpm_buf_destroy(&buf);
>+ return err;
>+ }
>+
> err = tpm_transmit_cmd(chip, &buf,
> offsetof(struct tpm2_get_random_out,
> buffer),
>diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
>index 6d03c224e6b2..33ad0d668e1a 100644
>--- a/drivers/char/tpm/tpm2-sessions.c
>+++ b/drivers/char/tpm/tpm2-sessions.c
>@@ -144,16 +144,24 @@ struct tpm2_auth {
> /*
> * Name Size based on TPM algorithm (assumes no hash bigger than 255)
> */
>-static u8 name_size(const u8 *name)
>+static int name_size(const u8 *name)
> {
>- static u8 size_map[] = {
>- [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
>- [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
>- [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
>- [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
>- };
>- u16 alg = get_unaligned_be16(name);
>- return size_map[alg] + 2;
>+ u16 hash_alg = get_unaligned_be16(name);
>+
>+ switch (hash_alg) {
>+ case TPM_ALG_SHA1:
>+ return SHA1_DIGEST_SIZE + 2;
>+ case TPM_ALG_SHA256:
>+ return SHA256_DIGEST_SIZE + 2;
>+ case TPM_ALG_SHA384:
>+ return SHA384_DIGEST_SIZE + 2;
>+ case TPM_ALG_SHA512:
>+ return SHA512_DIGEST_SIZE + 2;
>+ case TPM_ALG_SM3_256:
>+ return SM3256_DIGEST_SIZE + 2;
>+ }
>+
Can we/should we perhaps print a warning here if we don't know the
algorithm?
>+ return -EINVAL;
> }
>
> static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
>@@ -161,6 +169,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> struct tpm_header *head = (struct tpm_header *)buf->data;
> off_t offset = TPM_HEADER_SIZE;
> u32 tot_len = be32_to_cpu(head->length);
>+ int ret;
> u32 val;
>
> /* we're starting after the header so adjust the length */
>@@ -172,9 +181,15 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> return -EINVAL;
> offset += val;
> /* name */
>+
> val = tpm_buf_read_u16(buf, &offset);
>- if (val != name_size(&buf->data[offset]))
>+ ret = name_size(&buf->data[offset]);
>+ if (ret < 0)
>+ return ret;
>+
>+ if (val != ret)
> return -EINVAL;
>+
> memcpy(name, &buf->data[offset], val);
> /* forget the rest */
> return 0;
>@@ -221,46 +236,70 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
> * As with most tpm_buf operations, success is assumed because failure
> * will be caused by an incorrect programming model and indicated by a
> * kernel message.
>+ *
>+ * Ends the authorization session on failure.
> */
>-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
>- u32 handle, u8 *name)
>+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
>+ u32 handle, u8 *name)
> {
> #ifdef CONFIG_TCG_TPM2_HMAC
> enum tpm2_mso_type mso = tpm2_handle_mso(handle);
> struct tpm2_auth *auth;
> int slot;
>+ int ret;
> #endif
>
> if (!tpm2_chip_auth(chip)) {
> tpm_buf_append_handle(chip, buf, handle);
>- return;
>+ return 0;
> }
>
> #ifdef CONFIG_TCG_TPM2_HMAC
> slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
> if (slot >= AUTH_MAX_NAMES) {
>- dev_err(&chip->dev, "TPM: too many handles\n");
>- return;
>+ dev_err(&chip->dev, "too many handles\n");
>+ ret = -EIO;
>+ goto err;
> }
> auth = chip->auth;
>- WARN(auth->session != tpm_buf_length(buf),
>- "name added in wrong place\n");
>+ if (auth->session != tpm_buf_length(buf)) {
>+ dev_err(&chip->dev, "session state malformed");
>+ ret = -EIO;
>+ goto err;
>+ }
> tpm_buf_append_u32(buf, handle);
> auth->session += 4;
>
> if (mso == TPM2_MSO_PERSISTENT ||
> mso == TPM2_MSO_VOLATILE ||
> mso == TPM2_MSO_NVRAM) {
>- if (!name)
>- tpm2_read_public(chip, handle, auth->name[slot]);
>+ if (!name) {
>+ ret = tpm2_read_public(chip, handle, auth->name[slot]);
>+ if (ret)
>+ goto err;
>+ }
> } else {
>- if (name)
>- dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
We're dropping the error message here; is there a reason for that?
>+ if (name) {
>+ ret = -EIO;
>+ goto err;
>+ }
> }
>
> auth->name_h[slot] = handle;
>- if (name)
>- memcpy(auth->name[slot], name, name_size(name));
>+ if (name) {
>+ ret = name_size(name);
>+ if (ret < 0)
>+ goto err;
>+
>+ memcpy(auth->name[slot], name, ret);
>+ }
>+#endif
>+ return 0;
>+
>+#ifdef CONFIG_TCG_TPM2_HMAC
>+err:
>+ tpm2_end_auth_session(chip);
>+ return tpm_ret_to_err(ret);
> #endif
> }
> EXPORT_SYMBOL_GPL(tpm_buf_append_name);
>@@ -533,11 +572,9 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
> * encryption key and encrypts the first parameter of the command
> * buffer with it.
> *
>- * As with most tpm_buf operations, success is assumed because failure
>- * will be caused by an incorrect programming model and indicated by a
>- * kernel message.
>+ * Ends the authorization session on failure.
> */
>-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
>+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> {
> u32 cc, handles, val;
> struct tpm2_auth *auth = chip->auth;
>@@ -549,9 +586,12 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> u8 cphash[SHA256_DIGEST_SIZE];
> struct sha256_ctx sctx;
> struct hmac_sha256_ctx hctx;
>+ int ret;
>
>- if (!auth)
>- return;
>+ if (!auth) {
>+ ret = -EINVAL;
>+ goto err;
>+ }
>
> /* save the command code in BE format */
> auth->ordinal = head->ordinal;
>@@ -560,9 +600,10 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
>
> i = tpm2_find_cc(chip, cc);
> if (i < 0) {
>- dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
Again, I think it's generally helpful to have the error message given
that the return (EINVAL) does not help narrow down which value is bad.
>- return;
>+ ret = -EINVAL;
>+ goto err;
> }
>+
> attrs = chip->cc_attrs_tbl[i];
>
> handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
>@@ -576,9 +617,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> u32 handle = tpm_buf_read_u32(buf, &offset_s);
>
> if (auth->name_h[i] != handle) {
>- dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
>- i);
>- return;
>+ dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
>+ ret = -EINVAL;
>+ goto err;
> }
> }
> /* point offset_s to the start of the sessions */
>@@ -609,12 +650,14 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> offset_s += len;
> }
> if (offset_s != offset_p) {
>- dev_err(&chip->dev, "TPM session length is incorrect\n");
>- return;
>+ dev_err(&chip->dev, "session length is incorrect\n");
>+ ret = -EINVAL;
>+ goto err;
> }
> if (!hmac) {
>- dev_err(&chip->dev, "TPM could not find HMAC session\n");
>- return;
>+ dev_err(&chip->dev, "could not find HMAC session\n");
>+ ret = -EINVAL;
>+ goto err;
> }
>
> /* encrypt before HMAC */
>@@ -646,8 +689,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> if (mso == TPM2_MSO_PERSISTENT ||
> mso == TPM2_MSO_VOLATILE ||
> mso == TPM2_MSO_NVRAM) {
>- sha256_update(&sctx, auth->name[i],
>- name_size(auth->name[i]));
>+ ret = name_size(auth->name[i]);
>+ if (ret < 0)
>+ goto err;
>+
>+ sha256_update(&sctx, auth->name[i], ret);
> } else {
> __be32 h = cpu_to_be32(auth->name_h[i]);
>
>@@ -668,6 +714,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> hmac_sha256_update(&hctx, &auth->attrs, 1);
> hmac_sha256_final(&hctx, hmac);
>+ return 0;
>+
>+err:
>+ tpm2_end_auth_session(chip);
>+ return ret;
> }
> EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
>
>diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>index 0e9e043f728c..1a59f0190eb3 100644
>--- a/include/linux/tpm.h
>+++ b/include/linux/tpm.h
>@@ -528,8 +528,8 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
> #endif
> }
>
>-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
>- u32 handle, u8 *name);
>+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
>+ u32 handle, u8 *name);
> void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
> u8 attributes, u8 *passphrase,
> int passphraselen);
>@@ -562,7 +562,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
> #ifdef CONFIG_TCG_TPM2_HMAC
>
> int tpm2_start_auth_session(struct tpm_chip *chip);
>-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
>+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
> int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
> int rc);
> void tpm2_end_auth_session(struct tpm_chip *chip);
>diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>index e165b117bbca..7672a4376dad 100644
>--- a/security/keys/trusted-keys/trusted_tpm2.c
>+++ b/security/keys/trusted-keys/trusted_tpm2.c
>@@ -283,7 +283,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> goto out_put;
> }
>
>- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
>+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
>+ if (rc)
>+ goto out;
>+
> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> options->keyauth, TPM_DIGEST_SIZE);
>
>@@ -331,7 +334,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> goto out;
> }
>
>- tpm_buf_fill_hmac_session(chip, &buf);
>+ rc = tpm_buf_fill_hmac_session(chip, &buf);
>+ if (rc)
>+ goto out;
>+
> rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
> rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> if (rc)
>@@ -438,7 +444,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> return rc;
> }
>
>- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
>+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
>+ if (rc)
>+ goto out;
>+
> tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
> TPM_DIGEST_SIZE);
>
>@@ -450,7 +459,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> goto out;
> }
>
>- tpm_buf_fill_hmac_session(chip, &buf);
>+ rc = tpm_buf_fill_hmac_session(chip, &buf);
>+ if (rc)
>+ goto out;
>+
> rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
> rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> if (!rc)
>@@ -497,7 +509,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> return rc;
> }
>
>- tpm_buf_append_name(chip, &buf, blob_handle, NULL);
>+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
>+ if (rc)
>+ goto out;
>
> if (!options->policyhandle) {
> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
>@@ -522,7 +536,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> NULL, 0);
> }
>
>- tpm_buf_fill_hmac_session(chip, &buf);
>+ rc = tpm_buf_fill_hmac_session(chip, &buf);
>+ if (rc)
>+ goto out;
>+
> rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
> rc = tpm_buf_check_hmac_response(chip, &buf, rc);
>
>--
>2.52.0
>
J.
--
Be Ye Not Lost Among Precepts of Order
^ permalink raw reply
* Re: [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature
From: Mimi Zohar @ 2025-12-02 17:10 UTC (permalink / raw)
To: Coiby Xu, linux-integrity
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list,
open list:SECURITY SUBSYSTEM
In-Reply-To: <7jzx432acnp7mrma7om5ccvrc3ucvm5psscst62bcl5t5yklh7@yxhcvmrgbgli>
On Mon, 2025-12-01 at 11:15 +0800, Coiby Xu wrote:
> Hi Mimi,
>
> I think this patch set just fell off the radar. Can you take a look at
> it when time permits? Thanks! Btw, the patch set is still applicable to
> current next-integrity tree Linus and main tree.
Hi Coiby. I haven't forgotten about this patch, but would prefer upstreaming it
together with another EVM change. I'm really sorry about the delay.
--
thanks,
Mimi
^ permalink raw reply
* [PATCH 00/46] Allow inlining C helpers into Rust when using LTO
From: Alice Ryhl @ 2025-12-02 19:37 UTC (permalink / raw)
To: rust-for-linux
Cc: linux-kernel, Alice Ryhl, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Peter Zijlstra, Boqun Feng,
Elle Rhumsaa, Carlos Llamas, Yury Norov, Andreas Hindborg,
linux-block, FUJITA Tomonori, Miguel Ojeda, Michael Turquette,
Stephen Boyd, linux-clk, Benno Lossin, Danilo Krummrich,
Thomas Gleixner, Rafael J. Wysocki, Viresh Kumar, linux-pm,
Paul Moore, Serge Hallyn, linux-security-module, Daniel Almeida,
Abdiel Janulgue, Robin Murphy, Lyude Paul, Alexander Viro,
Christian Brauner, Jan Kara, linux-fsdevel, Josh Poimboeuf,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Brendan Higgins,
David Gow, Rae Moar, linux-kselftest, Andrew Morton,
Liam R. Howlett, Andrew Ballance, maple-tree, linux-mm,
Lorenzo Stoakes, Uladzislau Rezki, Vitaly Wool, Rob Herring,
Saravana Kannan, devicetree, Bjorn Helgaas,
Krzysztof Wilczyński, linux-pci, Remo Senekowitsch,
Paul E. McKenney, rcu, Will Deacon, Fiona Behrens, Gary Guo,
Liam Girdwood, Mark Brown, Alexandre Courbot, Vlastimil Babka,
Christoph Lameter, David Rientjes, Ingo Molnar, Waiman Long,
Mitchell Levy, Frederic Weisbecker, Anna-Maria Behnsen,
John Stultz, linux-usb, Tejun Heo, Lai Jiangshan, Matthew Wilcox,
Tamir Duberstein
This patch series adds __rust_helper to every single rust helper. The
patches do not depend on each other, so maintainers please go ahead and
pick up any patches relevant to your subsystem! Or provide your Acked-by
so that Miguel can pick them up.
These changes were generated by adding __rust_helper and running
ClangFormat. Unrelated formatting changes were removed manually.
Why is __rust_helper needed?
============================
Currently, C helpers cannot be inlined into Rust even when using LTO
because LLVM detects slightly different options on the codegen units.
* LLVM doesn't want to inline functions compiled with
`-fno-delete-null-pointer-checks` with code compiled without. The C
CGUs all have this enabled and Rust CGUs don't. Inlining is okay since
this is one of the hardening features that does not change the ABI,
and we shouldn't have null pointer dereferences in these helpers.
* LLVM doesn't want to inline functions with different list of builtins. C
side has `-fno-builtin-wcslen`; `wcslen` is not a Rust builtin, so
they should be compatible, but LLVM does not perform inlining due to
attributes mismatch.
* clang and Rust doesn't have the exact target string. Clang generates
`+cmov,+cx8,+fxsr` but Rust doesn't enable them (in fact, Rust will
complain if `-Ctarget-feature=+cmov,+cx8,+fxsr` is used). x86-64
always enable these features, so they are in fact the same target
string, but LLVM doesn't understand this and so inlining is inhibited.
This can be bypassed with `--ignore-tti-inline-compatible`, but this
is a hidden option.
(This analysis was written by Gary Guo.)
How is this fixed?
==================
To fix this we need to add __always_inline to all helpers when compiling
with LTO. However, it should not be added when running bindgen as
bindgen will ignore functions marked inline. To achieve this, we are
using a #define called __rust_helper that is defined differently
depending on whether bindgen is running or not.
Note that __rust_helper is currently always #defined to nothing.
Changing it to __always_inline will happen separately in another patch
series.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (46):
rust: auxiliary: add __rust_helper to helpers
rust: barrier: add __rust_helper to helpers
rust: binder: add __rust_helper to helpers
rust: bitmap: add __rust_helper to helpers
rust: bitops: add __rust_helper to helpers
rust: blk: add __rust_helper to helpers
rust: bug: add __rust_helper to helpers
rust: clk: add __rust_helper to helpers
rust: completion: add __rust_helper to helpers
rust: cpu: add __rust_helper to helpers
rust: cpufreq: add __rust_helper to helpers
rust: cpumask: add __rust_helper to helpers
rust: cred: add __rust_helper to helpers
rust: device: add __rust_helper to helpers
rust: dma: add __rust_helper to helpers
rust: drm: add __rust_helper to helpers
rust: err: add __rust_helper to helpers
rust: fs: add __rust_helper to helpers
rust: io: add __rust_helper to helpers
rust: irq: add __rust_helper to helpers
rust: jump_label: add __rust_helper to helpers
rust: kunit: add __rust_helper to helpers
rust: maple_tree: add __rust_helper to helpers
rust: mm: add __rust_helper to helpers
rust: of: add __rust_helper to helpers
rust: pci: add __rust_helper to helpers
rust: pid_namespace: add __rust_helper to helpers
rust: platform: add __rust_helper to helpers
rust: poll: add __rust_helper to helpers
rust: processor: add __rust_helper to helpers
rust: property: add __rust_helper to helpers
rust: rbtree: add __rust_helper to helpers
rust: rcu: add __rust_helper to helpers
rust: refcount: add __rust_helper to helpers
rust: regulator: add __rust_helper to helpers
rust: scatterlist: add __rust_helper to helpers
rust: security: add __rust_helper to helpers
rust: slab: add __rust_helper to helpers
rust: sync: add __rust_helper to helpers
rust: task: add __rust_helper to helpers
rust: time: add __rust_helper to helpers
rust: uaccess: add __rust_helper to helpers
rust: usb: add __rust_helper to helpers
rust: wait: add __rust_helper to helpers
rust: workqueue: add __rust_helper to helpers
rust: xarray: add __rust_helper to helpers
rust/helpers/auxiliary.c | 6 +++--
rust/helpers/barrier.c | 6 ++---
rust/helpers/binder.c | 13 ++++-----
rust/helpers/bitmap.c | 6 +++--
rust/helpers/bitops.c | 11 +++++---
rust/helpers/blk.c | 4 +--
rust/helpers/bug.c | 4 +--
rust/helpers/build_bug.c | 2 +-
rust/helpers/clk.c | 24 +++++++++--------
rust/helpers/completion.c | 2 +-
rust/helpers/cpu.c | 2 +-
rust/helpers/cpufreq.c | 3 ++-
rust/helpers/cpumask.c | 32 +++++++++++++---------
rust/helpers/cred.c | 4 +--
rust/helpers/device.c | 16 +++++------
rust/helpers/dma.c | 15 ++++++-----
rust/helpers/drm.c | 7 ++---
rust/helpers/err.c | 6 ++---
rust/helpers/fs.c | 2 +-
rust/helpers/io.c | 64 +++++++++++++++++++++++---------------------
rust/helpers/irq.c | 6 +++--
rust/helpers/jump_label.c | 2 +-
rust/helpers/kunit.c | 2 +-
rust/helpers/maple_tree.c | 3 ++-
rust/helpers/mm.c | 20 +++++++-------
rust/helpers/mutex.c | 13 ++++-----
rust/helpers/of.c | 2 +-
rust/helpers/page.c | 9 ++++---
rust/helpers/pci.c | 13 +++++----
rust/helpers/pid_namespace.c | 8 +++---
rust/helpers/platform.c | 2 +-
rust/helpers/poll.c | 5 ++--
rust/helpers/processor.c | 2 +-
rust/helpers/property.c | 2 +-
rust/helpers/rbtree.c | 5 ++--
rust/helpers/rcu.c | 4 +--
rust/helpers/refcount.c | 10 +++----
rust/helpers/regulator.c | 24 ++++++++++-------
rust/helpers/scatterlist.c | 12 +++++----
rust/helpers/security.c | 26 ++++++++++--------
rust/helpers/signal.c | 2 +-
rust/helpers/slab.c | 14 +++++-----
rust/helpers/spinlock.c | 13 ++++-----
rust/helpers/sync.c | 4 +--
rust/helpers/task.c | 24 ++++++++---------
rust/helpers/time.c | 12 ++++-----
rust/helpers/uaccess.c | 8 +++---
rust/helpers/usb.c | 3 ++-
rust/helpers/vmalloc.c | 7 ++---
rust/helpers/wait.c | 2 +-
rust/helpers/workqueue.c | 8 +++---
rust/helpers/xarray.c | 10 +++----
52 files changed, 280 insertions(+), 226 deletions(-)
---
base-commit: 54e3eae855629702c566bd2e130d9f40e7f35bde
change-id: 20251202-define-rust-helper-f7b531813007
Best regards,
--
Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply
* [PATCH 13/46] rust: cred: add __rust_helper to helpers
From: Alice Ryhl @ 2025-12-02 19:37 UTC (permalink / raw)
To: rust-for-linux
Cc: linux-kernel, Alice Ryhl, Paul Moore, Serge Hallyn,
linux-security-module
In-Reply-To: <20251202-define-rust-helper-v1-0-a2e13cbc17a6@google.com>
This is needed to inline these helpers into Rust code.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <sergeh@kernel.org>
Cc: linux-security-module@vger.kernel.org
---
rust/helpers/cred.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/rust/helpers/cred.c b/rust/helpers/cred.c
index fde7ae20cdd19f04ac5f28808586c65de6b72f09..a56a7b7536232733dcdc640f09f8f2537e69d75e 100644
--- a/rust/helpers/cred.c
+++ b/rust/helpers/cred.c
@@ -2,12 +2,12 @@
#include <linux/cred.h>
-const struct cred *rust_helper_get_cred(const struct cred *cred)
+__rust_helper const struct cred *rust_helper_get_cred(const struct cred *cred)
{
return get_cred(cred);
}
-void rust_helper_put_cred(const struct cred *cred)
+__rust_helper void rust_helper_put_cred(const struct cred *cred)
{
put_cred(cred);
}
--
2.52.0.158.g65b55ccf14-goog
^ permalink raw reply related
* [PATCH 37/46] rust: security: add __rust_helper to helpers
From: Alice Ryhl @ 2025-12-02 19:38 UTC (permalink / raw)
To: rust-for-linux
Cc: linux-kernel, Alice Ryhl, Paul Moore, Greg Kroah-Hartman,
linux-security-module
In-Reply-To: <20251202-define-rust-helper-v1-0-a2e13cbc17a6@google.com>
This is needed to inline these helpers into Rust code.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Cc: Paul Moore <paul@paul-moore.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-security-module@vger.kernel.org
---
rust/helpers/security.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/rust/helpers/security.c b/rust/helpers/security.c
index ca22da09548dfed95a83168ed09263e75cf08fd2..8d0a25fcf931ac6540a986aa10187c345614fc9e 100644
--- a/rust/helpers/security.c
+++ b/rust/helpers/security.c
@@ -3,41 +3,45 @@
#include <linux/security.h>
#ifndef CONFIG_SECURITY
-void rust_helper_security_cred_getsecid(const struct cred *c, u32 *secid)
+__rust_helper void rust_helper_security_cred_getsecid(const struct cred *c,
+ u32 *secid)
{
security_cred_getsecid(c, secid);
}
-int rust_helper_security_secid_to_secctx(u32 secid, struct lsm_context *cp)
+__rust_helper int rust_helper_security_secid_to_secctx(u32 secid,
+ struct lsm_context *cp)
{
return security_secid_to_secctx(secid, cp);
}
-void rust_helper_security_release_secctx(struct lsm_context *cp)
+__rust_helper void rust_helper_security_release_secctx(struct lsm_context *cp)
{
security_release_secctx(cp);
}
-int rust_helper_security_binder_set_context_mgr(const struct cred *mgr)
+__rust_helper int
+rust_helper_security_binder_set_context_mgr(const struct cred *mgr)
{
return security_binder_set_context_mgr(mgr);
}
-int rust_helper_security_binder_transaction(const struct cred *from,
- const struct cred *to)
+__rust_helper int
+rust_helper_security_binder_transaction(const struct cred *from,
+ const struct cred *to)
{
return security_binder_transaction(from, to);
}
-int rust_helper_security_binder_transfer_binder(const struct cred *from,
- const struct cred *to)
+__rust_helper int
+rust_helper_security_binder_transfer_binder(const struct cred *from,
+ const struct cred *to)
{
return security_binder_transfer_binder(from, to);
}
-int rust_helper_security_binder_transfer_file(const struct cred *from,
- const struct cred *to,
- const struct file *file)
+__rust_helper int rust_helper_security_binder_transfer_file(
+ const struct cred *from, const struct cred *to, const struct file *file)
{
return security_binder_transfer_file(from, to, file);
}
--
2.52.0.158.g65b55ccf14-goog
^ permalink raw reply related
* Re: [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-02 19:53 UTC (permalink / raw)
To: Jonathan McDowell
Cc: linux-integrity, Stefano Garzarella, stable, Peter Huewe,
Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ard Biesheuvel,
linux-kernel, keyrings, linux-security-module
In-Reply-To: <aS8TIeviaippVAha@earth.li>
On Tue, Dec 02, 2025 at 04:26:09PM +0000, Jonathan McDowell wrote:
> On Mon, Dec 01, 2025 at 09:39:58PM +0200, Jarkko Sakkinen wrote:
> > 'name_size' does not have any range checks, and it just directly indexes
> > with TPM_ALG_ID, which could lead into memory corruption at worst.
> >
> > Address the issue by only processing known values and returning -EINVAL for
> > unrecognized values.
> >
> > Make also 'tpm_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible so
> > that errors are detected before causing any spurious TPM traffic.
> >
> > End also the authorization session on failure in both of the functions, as
> > the session state would be then by definition corrupted.
> >
> > Cc: stable@vger.kernel.org # v6.10+
> > Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > v3:
> > - Add two missing 'tpm2_end_auth_session' calls to the fallback paths of
> > 'tpm_buf_fill_hmac_session'.
> > - Rewrote the commit message.
> > - End authorization session on failure in 'tpm2_buf_append_name' and
> > 'tpm_buf_fill_hmac_session'.
> > v2:
> > There was spurious extra field added to tpm2_hash by mistake.
> > ---
> > drivers/char/tpm/tpm2-cmd.c | 23 +++-
> > drivers/char/tpm/tpm2-sessions.c | 131 +++++++++++++++-------
> > include/linux/tpm.h | 6 +-
> > security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
> > 4 files changed, 136 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 5b6ccf901623..4473b81122e8 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -187,7 +187,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > }
> >
> > if (!disable_pcr_integrity) {
> > - tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > + rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > + if (rc) {
> > + tpm_buf_destroy(&buf);
> > + return rc;
> > + }
> > tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > } else {
> > tpm_buf_append_handle(chip, &buf, pcr_idx);
> > @@ -202,8 +206,14 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > chip->allocated_banks[i].digest_size);
> > }
> >
> > - if (!disable_pcr_integrity)
> > - tpm_buf_fill_hmac_session(chip, &buf);
> > + if (!disable_pcr_integrity) {
> > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > + if (rc) {
> > + tpm_buf_destroy(&buf);
> > + return rc;
> > + }
> > + }
> > +
> > rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
> > if (!disable_pcr_integrity)
> > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > @@ -261,7 +271,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> > | TPM2_SA_CONTINUE_SESSION,
> > NULL, 0);
> > tpm_buf_append_u16(&buf, num_bytes);
> > - tpm_buf_fill_hmac_session(chip, &buf);
> > + err = tpm_buf_fill_hmac_session(chip, &buf);
> > + if (err) {
> > + tpm_buf_destroy(&buf);
> > + return err;
> > + }
> > +
> > err = tpm_transmit_cmd(chip, &buf,
> > offsetof(struct tpm2_get_random_out,
> > buffer),
> > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> > index 6d03c224e6b2..33ad0d668e1a 100644
> > --- a/drivers/char/tpm/tpm2-sessions.c
> > +++ b/drivers/char/tpm/tpm2-sessions.c
> > @@ -144,16 +144,24 @@ struct tpm2_auth {
> > /*
> > * Name Size based on TPM algorithm (assumes no hash bigger than 255)
> > */
> > -static u8 name_size(const u8 *name)
> > +static int name_size(const u8 *name)
> > {
> > - static u8 size_map[] = {
> > - [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
> > - [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
> > - [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
> > - [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
> > - };
> > - u16 alg = get_unaligned_be16(name);
> > - return size_map[alg] + 2;
> > + u16 hash_alg = get_unaligned_be16(name);
> > +
> > + switch (hash_alg) {
> > + case TPM_ALG_SHA1:
> > + return SHA1_DIGEST_SIZE + 2;
> > + case TPM_ALG_SHA256:
> > + return SHA256_DIGEST_SIZE + 2;
> > + case TPM_ALG_SHA384:
> > + return SHA384_DIGEST_SIZE + 2;
> > + case TPM_ALG_SHA512:
> > + return SHA512_DIGEST_SIZE + 2;
> > + case TPM_ALG_SM3_256:
> > + return SM3256_DIGEST_SIZE + 2;
> > + }
> > +
>
> Can we/should we perhaps print a warning here if we don't know the
> algorithm?
I think it is a good idea to do that right now.
Also, it'd be better to not have SM3 label as SM2/SM3 is not supported
at this point. I'm working simulatenously on an improved feature and
that slipped from that work:
My big picture roadmap for this feature, and how to make it useful is:
1. tpm.integrity_mode=disabled/permissive/enforced. Permissive means
here that the feature is conditionally enabled if algorithms that are
required to enable the HMAC pipe are available.
2. tpm.integrity_handle=0x00000000/0x81??????.
>
> > + return -EINVAL;
> > }
> >
> > static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > @@ -161,6 +169,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > struct tpm_header *head = (struct tpm_header *)buf->data;
> > off_t offset = TPM_HEADER_SIZE;
> > u32 tot_len = be32_to_cpu(head->length);
> > + int ret;
> > u32 val;
> >
> > /* we're starting after the header so adjust the length */
> > @@ -172,9 +181,15 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > return -EINVAL;
> > offset += val;
> > /* name */
> > +
> > val = tpm_buf_read_u16(buf, &offset);
> > - if (val != name_size(&buf->data[offset]))
> > + ret = name_size(&buf->data[offset]);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (val != ret)
> > return -EINVAL;
> > +
> > memcpy(name, &buf->data[offset], val);
> > /* forget the rest */
> > return 0;
> > @@ -221,46 +236,70 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
> > * As with most tpm_buf operations, success is assumed because failure
> > * will be caused by an incorrect programming model and indicated by a
> > * kernel message.
> > + *
> > + * Ends the authorization session on failure.
> > */
> > -void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > - u32 handle, u8 *name)
> > +int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > + u32 handle, u8 *name)
> > {
> > #ifdef CONFIG_TCG_TPM2_HMAC
> > enum tpm2_mso_type mso = tpm2_handle_mso(handle);
> > struct tpm2_auth *auth;
> > int slot;
> > + int ret;
> > #endif
> >
> > if (!tpm2_chip_auth(chip)) {
> > tpm_buf_append_handle(chip, buf, handle);
> > - return;
> > + return 0;
> > }
> >
> > #ifdef CONFIG_TCG_TPM2_HMAC
> > slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
> > if (slot >= AUTH_MAX_NAMES) {
> > - dev_err(&chip->dev, "TPM: too many handles\n");
> > - return;
> > + dev_err(&chip->dev, "too many handles\n");
> > + ret = -EIO;
> > + goto err;
> > }
> > auth = chip->auth;
> > - WARN(auth->session != tpm_buf_length(buf),
> > - "name added in wrong place\n");
> > + if (auth->session != tpm_buf_length(buf)) {
> > + dev_err(&chip->dev, "session state malformed");
> > + ret = -EIO;
> > + goto err;
> > + }
> > tpm_buf_append_u32(buf, handle);
> > auth->session += 4;
> >
> > if (mso == TPM2_MSO_PERSISTENT ||
> > mso == TPM2_MSO_VOLATILE ||
> > mso == TPM2_MSO_NVRAM) {
> > - if (!name)
> > - tpm2_read_public(chip, handle, auth->name[slot]);
> > + if (!name) {
> > + ret = tpm2_read_public(chip, handle, auth->name[slot]);
> > + if (ret)
> > + goto err;
> > + }
> > } else {
> > - if (name)
> > - dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
>
> We're dropping the error message here; is there a reason for that?
Thanks, I'll add it back.
>
> > + if (name) {
> > + ret = -EIO;
> > + goto err;
> > + }
> > }
> >
> > auth->name_h[slot] = handle;
> > - if (name)
> > - memcpy(auth->name[slot], name, name_size(name));
> > + if (name) {
> > + ret = name_size(name);
> > + if (ret < 0)
> > + goto err;
> > +
> > + memcpy(auth->name[slot], name, ret);
> > + }
> > +#endif
> > + return 0;
> > +
> > +#ifdef CONFIG_TCG_TPM2_HMAC
> > +err:
> > + tpm2_end_auth_session(chip);
> > + return tpm_ret_to_err(ret);
> > #endif
> > }
> > EXPORT_SYMBOL_GPL(tpm_buf_append_name);
> > @@ -533,11 +572,9 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
> > * encryption key and encrypts the first parameter of the command
> > * buffer with it.
> > *
> > - * As with most tpm_buf operations, success is assumed because failure
> > - * will be caused by an incorrect programming model and indicated by a
> > - * kernel message.
> > + * Ends the authorization session on failure.
> > */
> > -void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > +int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > {
> > u32 cc, handles, val;
> > struct tpm2_auth *auth = chip->auth;
> > @@ -549,9 +586,12 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > u8 cphash[SHA256_DIGEST_SIZE];
> > struct sha256_ctx sctx;
> > struct hmac_sha256_ctx hctx;
> > + int ret;
> >
> > - if (!auth)
> > - return;
> > + if (!auth) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
> >
> > /* save the command code in BE format */
> > auth->ordinal = head->ordinal;
> > @@ -560,9 +600,10 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> >
> > i = tpm2_find_cc(chip, cc);
> > if (i < 0) {
> > - dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
>
> Again, I think it's generally helpful to have the error message given that
> the return (EINVAL) does not help narrow down which value is bad.
Agreed.
>
> > - return;
> > + ret = -EINVAL;
> > + goto err;
> > }
> > +
> > attrs = chip->cc_attrs_tbl[i];
> >
> > handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
> > @@ -576,9 +617,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > u32 handle = tpm_buf_read_u32(buf, &offset_s);
> >
> > if (auth->name_h[i] != handle) {
> > - dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
> > - i);
> > - return;
> > + dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
> > + ret = -EINVAL;
> > + goto err;
> > }
> > }
> > /* point offset_s to the start of the sessions */
> > @@ -609,12 +650,14 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > offset_s += len;
> > }
> > if (offset_s != offset_p) {
> > - dev_err(&chip->dev, "TPM session length is incorrect\n");
> > - return;
> > + dev_err(&chip->dev, "session length is incorrect\n");
> > + ret = -EINVAL;
> > + goto err;
> > }
> > if (!hmac) {
> > - dev_err(&chip->dev, "TPM could not find HMAC session\n");
> > - return;
> > + dev_err(&chip->dev, "could not find HMAC session\n");
> > + ret = -EINVAL;
> > + goto err;
> > }
> >
> > /* encrypt before HMAC */
> > @@ -646,8 +689,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > if (mso == TPM2_MSO_PERSISTENT ||
> > mso == TPM2_MSO_VOLATILE ||
> > mso == TPM2_MSO_NVRAM) {
> > - sha256_update(&sctx, auth->name[i],
> > - name_size(auth->name[i]));
> > + ret = name_size(auth->name[i]);
> > + if (ret < 0)
> > + goto err;
> > +
> > + sha256_update(&sctx, auth->name[i], ret);
> > } else {
> > __be32 h = cpu_to_be32(auth->name_h[i]);
> >
> > @@ -668,6 +714,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> > hmac_sha256_update(&hctx, &auth->attrs, 1);
> > hmac_sha256_final(&hctx, hmac);
> > + return 0;
> > +
> > +err:
> > + tpm2_end_auth_session(chip);
> > + return ret;
> > }
> > EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
> >
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index 0e9e043f728c..1a59f0190eb3 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -528,8 +528,8 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
> > #endif
> > }
> >
> > -void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > - u32 handle, u8 *name);
> > +int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > + u32 handle, u8 *name);
> > void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
> > u8 attributes, u8 *passphrase,
> > int passphraselen);
> > @@ -562,7 +562,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
> > #ifdef CONFIG_TCG_TPM2_HMAC
> >
> > int tpm2_start_auth_session(struct tpm_chip *chip);
> > -void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
> > +int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
> > int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
> > int rc);
> > void tpm2_end_auth_session(struct tpm_chip *chip);
> > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> > index e165b117bbca..7672a4376dad 100644
> > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > @@ -283,7 +283,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> > goto out_put;
> > }
> >
> > - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > + if (rc)
> > + goto out;
> > +
> > tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> > options->keyauth, TPM_DIGEST_SIZE);
> >
> > @@ -331,7 +334,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> > goto out;
> > }
> >
> > - tpm_buf_fill_hmac_session(chip, &buf);
> > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > + if (rc)
> > + goto out;
> > +
> > rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
> > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > if (rc)
> > @@ -438,7 +444,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > return rc;
> > }
> >
> > - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > + if (rc)
> > + goto out;
> > +
> > tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
> > TPM_DIGEST_SIZE);
> >
> > @@ -450,7 +459,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > goto out;
> > }
> >
> > - tpm_buf_fill_hmac_session(chip, &buf);
> > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > + if (rc)
> > + goto out;
> > +
> > rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
> > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > if (!rc)
> > @@ -497,7 +509,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> > return rc;
> > }
> >
> > - tpm_buf_append_name(chip, &buf, blob_handle, NULL);
> > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > + if (rc)
> > + goto out;
> >
> > if (!options->policyhandle) {
> > tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
> > @@ -522,7 +536,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> > NULL, 0);
> > }
> >
> > - tpm_buf_fill_hmac_session(chip, &buf);
> > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > + if (rc)
> > + goto out;
> > +
> > rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
> > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> >
> > --
> > 2.52.0
> >
>
> J.
>
> --
> Be Ye Not Lost Among Precepts of Order
BR, Jarkko
^ permalink raw reply
* [PATCH v2 1/4] tpm2-sessions: fix out of range indexing in name_size
From: Jarkko Sakkinen @ 2025-12-02 20:26 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, stable, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ard Biesheuvel,
open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251202202643.107108-1-jarkko@kernel.org>
'name_size' does not have any range checks, and it just directly indexes
with TPM_ALG_ID, which could lead into memory corruption at worst.
Address the issue by only processing known values and returning -EINVAL for
unrecognized values.
Make also 'tpm_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible so
that errors are detected before causing any spurious TPM traffic.
End also the authorization session on failure in both of the functions, as
the session state would be then by definition corrupted.
Cc: stable@vger.kernel.org # v6.10+
Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
- Wrote a better short summary.
- Addressed remarks in https://lore.kernel.org/linux-integrity/aS8TIeviaippVAha@earth.li/
- Use -EIO consistently in tpm2_fill_hmac_session. These are not input value
errors. They could only spun from malformed (kernel) state.
- name_size did not have a proper default-case. Reorganize the
fallback into that.
---
drivers/char/tpm/tpm2-cmd.c | 23 +++-
drivers/char/tpm/tpm2-sessions.c | 133 +++++++++++++++-------
include/linux/tpm.h | 6 +-
security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
4 files changed, 138 insertions(+), 53 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 5b6ccf901623..4473b81122e8 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -187,7 +187,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
}
if (!disable_pcr_integrity) {
- tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+ rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return rc;
+ }
tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
} else {
tpm_buf_append_handle(chip, &buf, pcr_idx);
@@ -202,8 +206,14 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
chip->allocated_banks[i].digest_size);
}
- if (!disable_pcr_integrity)
- tpm_buf_fill_hmac_session(chip, &buf);
+ if (!disable_pcr_integrity) {
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc) {
+ tpm_buf_destroy(&buf);
+ return rc;
+ }
+ }
+
rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
if (!disable_pcr_integrity)
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
@@ -261,7 +271,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
| TPM2_SA_CONTINUE_SESSION,
NULL, 0);
tpm_buf_append_u16(&buf, num_bytes);
- tpm_buf_fill_hmac_session(chip, &buf);
+ err = tpm_buf_fill_hmac_session(chip, &buf);
+ if (err) {
+ tpm_buf_destroy(&buf);
+ return err;
+ }
+
err = tpm_transmit_cmd(chip, &buf,
offsetof(struct tpm2_get_random_out,
buffer),
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 6d03c224e6b2..dc6bee7e1ef6 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -144,16 +144,23 @@ struct tpm2_auth {
/*
* Name Size based on TPM algorithm (assumes no hash bigger than 255)
*/
-static u8 name_size(const u8 *name)
+static int name_size(const u8 *name)
{
- static u8 size_map[] = {
- [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
- [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
- [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
- [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
- };
- u16 alg = get_unaligned_be16(name);
- return size_map[alg] + 2;
+ u16 hash_alg = get_unaligned_be16(name);
+
+ switch (hash_alg) {
+ case TPM_ALG_SHA1:
+ return SHA1_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA256:
+ return SHA256_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA384:
+ return SHA384_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA512:
+ return SHA512_DIGEST_SIZE + 2;
+ default:
+ pr_warn("tpm: invalid name algorithm: 0x%04\n", hash_alg);
+ return -EINVAL;
+ }
}
static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
@@ -161,6 +168,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
struct tpm_header *head = (struct tpm_header *)buf->data;
off_t offset = TPM_HEADER_SIZE;
u32 tot_len = be32_to_cpu(head->length);
+ int ret;
u32 val;
/* we're starting after the header so adjust the length */
@@ -172,9 +180,15 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
return -EINVAL;
offset += val;
/* name */
+
val = tpm_buf_read_u16(buf, &offset);
- if (val != name_size(&buf->data[offset]))
+ ret = name_size(&buf->data[offset]);
+ if (ret < 0)
+ return ret;
+
+ if (val != ret)
return -EINVAL;
+
memcpy(name, &buf->data[offset], val);
/* forget the rest */
return 0;
@@ -221,46 +235,72 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
* As with most tpm_buf operations, success is assumed because failure
* will be caused by an incorrect programming model and indicated by a
* kernel message.
+ *
+ * Ends the authorization session on failure.
*/
-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name)
+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
+ u32 handle, u8 *name)
{
#ifdef CONFIG_TCG_TPM2_HMAC
enum tpm2_mso_type mso = tpm2_handle_mso(handle);
struct tpm2_auth *auth;
int slot;
+ int ret;
#endif
if (!tpm2_chip_auth(chip)) {
tpm_buf_append_handle(chip, buf, handle);
- return;
+ return 0;
}
#ifdef CONFIG_TCG_TPM2_HMAC
slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
if (slot >= AUTH_MAX_NAMES) {
- dev_err(&chip->dev, "TPM: too many handles\n");
- return;
+ dev_err(&chip->dev, "too many handles\n");
+ ret = -EIO;
+ goto err;
}
auth = chip->auth;
- WARN(auth->session != tpm_buf_length(buf),
- "name added in wrong place\n");
+ if (auth->session != tpm_buf_length(buf)) {
+ dev_err(&chip->dev, "session state malformed");
+ ret = -EIO;
+ goto err;
+ }
tpm_buf_append_u32(buf, handle);
auth->session += 4;
if (mso == TPM2_MSO_PERSISTENT ||
mso == TPM2_MSO_VOLATILE ||
mso == TPM2_MSO_NVRAM) {
- if (!name)
- tpm2_read_public(chip, handle, auth->name[slot]);
+ if (!name) {
+ ret = tpm2_read_public(chip, handle, auth->name[slot]);
+ if (ret)
+ goto err;
+ }
} else {
- if (name)
- dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
+ if (name) {
+ dev_err(&chip->dev, "handle 0x%08x does not use a name\n",
+ handle);
+ ret = -EIO;
+ goto err;
+ }
}
auth->name_h[slot] = handle;
- if (name)
- memcpy(auth->name[slot], name, name_size(name));
+ if (name) {
+ ret = name_size(name);
+ if (ret < 0)
+ goto err;
+
+ memcpy(auth->name[slot], name, ret);
+ }
+#endif
+ return 0;
+
+#ifdef CONFIG_TCG_TPM2_HMAC
+err:
+ tpm2_end_auth_session(chip);
+ return tpm_ret_to_err(ret);
#endif
}
EXPORT_SYMBOL_GPL(tpm_buf_append_name);
@@ -533,11 +573,9 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
* encryption key and encrypts the first parameter of the command
* buffer with it.
*
- * As with most tpm_buf operations, success is assumed because failure
- * will be caused by an incorrect programming model and indicated by a
- * kernel message.
+ * Ends the authorization session on failure.
*/
-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
{
u32 cc, handles, val;
struct tpm2_auth *auth = chip->auth;
@@ -549,9 +587,12 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
u8 cphash[SHA256_DIGEST_SIZE];
struct sha256_ctx sctx;
struct hmac_sha256_ctx hctx;
+ int ret;
- if (!auth)
- return;
+ if (!auth) {
+ ret = -EIO;
+ goto err;
+ }
/* save the command code in BE format */
auth->ordinal = head->ordinal;
@@ -560,9 +601,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
i = tpm2_find_cc(chip, cc);
if (i < 0) {
- dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
- return;
+ dev_err(&chip->dev, "command 0x%08x not found\n", cc);
+ ret = -EIO;
+ goto err;
}
+
attrs = chip->cc_attrs_tbl[i];
handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
@@ -576,9 +619,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
u32 handle = tpm_buf_read_u32(buf, &offset_s);
if (auth->name_h[i] != handle) {
- dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
- i);
- return;
+ dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
+ ret = -EIO;
+ goto err;
}
}
/* point offset_s to the start of the sessions */
@@ -609,12 +652,14 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
offset_s += len;
}
if (offset_s != offset_p) {
- dev_err(&chip->dev, "TPM session length is incorrect\n");
- return;
+ dev_err(&chip->dev, "session length is incorrect\n");
+ ret = -EIO;
+ goto err;
}
if (!hmac) {
- dev_err(&chip->dev, "TPM could not find HMAC session\n");
- return;
+ dev_err(&chip->dev, "could not find HMAC session\n");
+ ret = -EIO;
+ goto err;
}
/* encrypt before HMAC */
@@ -646,8 +691,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
if (mso == TPM2_MSO_PERSISTENT ||
mso == TPM2_MSO_VOLATILE ||
mso == TPM2_MSO_NVRAM) {
- sha256_update(&sctx, auth->name[i],
- name_size(auth->name[i]));
+ ret = name_size(auth->name[i]);
+ if (ret < 0)
+ goto err;
+
+ sha256_update(&sctx, auth->name[i], ret);
} else {
__be32 h = cpu_to_be32(auth->name_h[i]);
@@ -668,6 +716,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
hmac_sha256_update(&hctx, &auth->attrs, 1);
hmac_sha256_final(&hctx, hmac);
+ return 0;
+
+err:
+ tpm2_end_auth_session(chip);
+ return ret;
}
EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 0e9e043f728c..1a59f0190eb3 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -528,8 +528,8 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
#endif
}
-void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
- u32 handle, u8 *name);
+int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
+ u32 handle, u8 *name);
void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
u8 attributes, u8 *passphrase,
int passphraselen);
@@ -562,7 +562,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
#ifdef CONFIG_TCG_TPM2_HMAC
int tpm2_start_auth_session(struct tpm_chip *chip);
-void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
+int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
int rc);
void tpm2_end_auth_session(struct tpm_chip *chip);
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index e165b117bbca..7672a4376dad 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -283,7 +283,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out_put;
}
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
+
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
options->keyauth, TPM_DIGEST_SIZE);
@@ -331,7 +334,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (rc)
@@ -438,7 +444,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
return rc;
}
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
+
tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
TPM_DIGEST_SIZE);
@@ -450,7 +459,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
goto out;
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (!rc)
@@ -497,7 +509,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
return rc;
}
- tpm_buf_append_name(chip, &buf, blob_handle, NULL);
+ rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+ if (rc)
+ goto out;
if (!options->policyhandle) {
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
@@ -522,7 +536,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
NULL, 0);
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
--
2.52.0
^ permalink raw reply related
* [PATCH v2 4/4] tpm2-sessions: Open code tpm_buf_append_hmac_session()
From: Jarkko Sakkinen @ 2025-12-02 20:26 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, Jonathan McDowell, Peter Huewe, Jason Gunthorpe,
open list, Jarkko Sakkinen, Jonathan McDowell, James Bottomley,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251202202643.107108-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Open code 'tpm_buf_append_hmac_session_opt' to the call site, as it only
masks a call sequence and does otherwise nothing particularly useful.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
---
drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++---
include/linux/tpm.h | 23 -----------------------
security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
3 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index f1e9c35f13a2..94cb887cb4a9 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -270,9 +270,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
do {
tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
- | TPM2_SA_CONTINUE_SESSION,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf,
+ TPM2_SA_ENCRYPT |
+ TPM2_SA_CONTINUE_SESSION,
+ NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
tpm_buf_append_u16(&buf, num_bytes);
err = tpm_buf_fill_hmac_session(chip, &buf);
if (err) {
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index e3a6e2fb41a7..a93df0efebe4 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -535,29 +535,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
u8 *passphrase, int passphraselen);
-static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
- struct tpm_buf *buf,
- u8 attributes,
- u8 *passphrase,
- int passphraselen)
-{
- struct tpm_header *head;
- int offset;
-
- if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
- } else {
- offset = buf->handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf->data;
-
- /*
- * If the only sessions are optional, the command tag must change to
- * TPM2_ST_NO_SESSIONS.
- */
- if (tpm_buf_length(buf) == offset)
- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
- }
-}
#ifdef CONFIG_TCG_TPM2_HMAC
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 7672a4376dad..e6b95111ac7d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -494,8 +494,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle)
{
+ struct tpm_header *head;
struct tpm_buf buf;
u16 data_len;
+ int offset;
u8 *data;
int rc;
@@ -532,8 +534,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm2_buf_append_auth(&buf, options->policyhandle,
NULL /* nonce */, 0, 0,
options->blobauth, options->blobauth_len);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
}
rc = tpm_buf_fill_hmac_session(chip, &buf);
--
2.52.0
^ permalink raw reply related
* Re: [PATCH] selftests/landlock: Remove invalid unix socket bind()
From: Matthieu Buffet @ 2025-12-02 21:46 UTC (permalink / raw)
To: Günther Noack; +Cc: Mickaël Salaün, linux-security-module
In-Reply-To: <aS6lMPTlUo9bWYEG@google.com>
Hi Günther,
On 12/2/2025 9:37 AM, Günther Noack wrote:
> Optional: In hindsight, I think it would be nice to simplify the way
> that we calculate the address length here. I probably mis-read
> unix(7) at the time, where a similar formula is used (but with a
> "+1"). Re-reading it, it seems that just passing sizeof(cli_un) as
> the address length would have been the simpler and less error prone
> solution:
You're right, the rest of the code uses non-NULL-terminated pathnames.
Not a problem per se on Linux, but these computations make tests less
readable. I will send a patch based on this fix. Thanks!
Matthieu
^ permalink raw reply
* [PATCH] selftests/landlock: NULL-terminate unix pathname addresses
From: Matthieu Buffet @ 2025-12-02 21:51 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Matthieu Buffet
In-Reply-To: <c2780073-9e74-4303-9e07-6b825963148e@buffet.re>
The size of Unix pathname addresses is computed in selftests using
offsetof(struct sockaddr_un, sun_path) + strlen(xxx). It should have
been that +1, which makes addresses passed to the libc and kernel
non-NULL-terminated. unix_mkname_bsd() fixes that in Linux so there is
no harm, but just using sizeof(the address struct) should improve
readability.
Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
Based on commit https://lore.kernel.org/linux-security-module/20251201003631.190817-1-matthieu@buffet.re/
tools/testing/selftests/landlock/fs_test.c | 24 +++++++++----------
.../landlock/scoped_abstract_unix_test.c | 21 +++++++---------
2 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 7d378bdf3bce..76491ba54dce 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -4362,22 +4362,24 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
{
const char *const path = file1_s1d1;
int srv_fd, cli_fd, ruleset_fd;
- socklen_t size;
- struct sockaddr_un srv_un, cli_un;
+ struct sockaddr_un srv_un = {
+ .sun_family = AF_UNIX,
+ };
+ struct sockaddr_un cli_un = {
+ .sun_family = AF_UNIX,
+ };
const struct landlock_ruleset_attr attr = {
.handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV,
};
/* Sets up a server */
- srv_un.sun_family = AF_UNIX;
- strncpy(srv_un.sun_path, path, sizeof(srv_un.sun_path));
-
ASSERT_EQ(0, unlink(path));
srv_fd = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, srv_fd);
- size = offsetof(struct sockaddr_un, sun_path) + strlen(srv_un.sun_path);
- ASSERT_EQ(0, bind(srv_fd, (struct sockaddr *)&srv_un, size));
+ strncpy(srv_un.sun_path, path, sizeof(srv_un.sun_path));
+ ASSERT_EQ(0, bind(srv_fd, (struct sockaddr *)&srv_un, sizeof(srv_un)));
+
ASSERT_EQ(0, listen(srv_fd, 10 /* qlen */));
/* Enables Landlock. */
@@ -4387,16 +4389,12 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
ASSERT_EQ(0, close(ruleset_fd));
/* Sets up a client connection to it */
- cli_un.sun_family = AF_UNIX;
cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, cli_fd);
- bzero(&cli_un, sizeof(cli_un));
- cli_un.sun_family = AF_UNIX;
strncpy(cli_un.sun_path, path, sizeof(cli_un.sun_path));
- size = offsetof(struct sockaddr_un, sun_path) + strlen(cli_un.sun_path);
-
- ASSERT_EQ(0, connect(cli_fd, (struct sockaddr *)&cli_un, size));
+ ASSERT_EQ(0,
+ connect(cli_fd, (struct sockaddr *)&cli_un, sizeof(cli_un)));
/* FIONREAD and other IOCTLs should not be forbidden. */
EXPECT_EQ(0, test_fionread_ioctl(cli_fd));
diff --git a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
index 6825082c079c..2cdf1ba07016 100644
--- a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
+++ b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
@@ -779,7 +779,6 @@ FIXTURE_TEARDOWN(various_address_sockets)
TEST_F(various_address_sockets, scoped_pathname_sockets)
{
- socklen_t size_stream, size_dgram;
pid_t child;
int status;
char buf_child, buf_parent;
@@ -798,12 +797,8 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
/* Pathname address. */
snprintf(stream_pathname_addr.sun_path,
sizeof(stream_pathname_addr.sun_path), "%s", stream_path);
- size_stream = offsetof(struct sockaddr_un, sun_path) +
- strlen(stream_pathname_addr.sun_path);
snprintf(dgram_pathname_addr.sun_path,
sizeof(dgram_pathname_addr.sun_path), "%s", dgram_path);
- size_dgram = offsetof(struct sockaddr_un, sun_path) +
- strlen(dgram_pathname_addr.sun_path);
/* Abstract address. */
memset(&stream_abstract_addr, 0, sizeof(stream_abstract_addr));
@@ -841,8 +836,9 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
/* Connects with pathname sockets. */
stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_pathname_socket);
- ASSERT_EQ(0, connect(stream_pathname_socket,
- &stream_pathname_addr, size_stream));
+ ASSERT_EQ(0,
+ connect(stream_pathname_socket, &stream_pathname_addr,
+ sizeof(stream_pathname_addr)));
ASSERT_EQ(1, write(stream_pathname_socket, "b", 1));
EXPECT_EQ(0, close(stream_pathname_socket));
@@ -850,12 +846,13 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_pathname_socket);
err = sendto(dgram_pathname_socket, "c", 1, 0,
- &dgram_pathname_addr, size_dgram);
+ &dgram_pathname_addr, sizeof(dgram_pathname_addr));
EXPECT_EQ(1, err);
/* Sends with connection. */
- ASSERT_EQ(0, connect(dgram_pathname_socket,
- &dgram_pathname_addr, size_dgram));
+ ASSERT_EQ(0,
+ connect(dgram_pathname_socket, &dgram_pathname_addr,
+ sizeof(dgram_pathname_addr)));
ASSERT_EQ(1, write(dgram_pathname_socket, "d", 1));
EXPECT_EQ(0, close(dgram_pathname_socket));
@@ -910,13 +907,13 @@ TEST_F(various_address_sockets, scoped_pathname_sockets)
stream_pathname_socket = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, stream_pathname_socket);
ASSERT_EQ(0, bind(stream_pathname_socket, &stream_pathname_addr,
- size_stream));
+ sizeof(stream_pathname_addr)));
ASSERT_EQ(0, listen(stream_pathname_socket, backlog));
dgram_pathname_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
ASSERT_LE(0, dgram_pathname_socket);
ASSERT_EQ(0, bind(dgram_pathname_socket, &dgram_pathname_addr,
- size_dgram));
+ sizeof(dgram_pathname_addr)));
/* Sets up abstract servers. */
stream_abstract_socket = socket(AF_UNIX, SOCK_STREAM, 0);
base-commit: c07e22414d18559b5c0fd5bc2c2f68f2903f1738
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v3] tpm2-sessions: address out-of-range indexing
From: Jarkko Sakkinen @ 2025-12-02 22:07 UTC (permalink / raw)
To: Jonathan McDowell
Cc: linux-integrity, Stefano Garzarella, stable, Peter Huewe,
Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ard Biesheuvel,
linux-kernel, keyrings, linux-security-module
In-Reply-To: <aS9Dv5CKGNOzpsN7@kernel.org>
On Tue, Dec 02, 2025 at 09:53:39PM +0200, Jarkko Sakkinen wrote:
> On Tue, Dec 02, 2025 at 04:26:09PM +0000, Jonathan McDowell wrote:
> > On Mon, Dec 01, 2025 at 09:39:58PM +0200, Jarkko Sakkinen wrote:
> > > 'name_size' does not have any range checks, and it just directly indexes
> > > with TPM_ALG_ID, which could lead into memory corruption at worst.
> > >
> > > Address the issue by only processing known values and returning -EINVAL for
> > > unrecognized values.
> > >
> > > Make also 'tpm_buf_append_name' and 'tpm_buf_fill_hmac_session' fallible so
> > > that errors are detected before causing any spurious TPM traffic.
> > >
> > > End also the authorization session on failure in both of the functions, as
> > > the session state would be then by definition corrupted.
> > >
> > > Cc: stable@vger.kernel.org # v6.10+
> > > Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > > v3:
> > > - Add two missing 'tpm2_end_auth_session' calls to the fallback paths of
> > > 'tpm_buf_fill_hmac_session'.
> > > - Rewrote the commit message.
> > > - End authorization session on failure in 'tpm2_buf_append_name' and
> > > 'tpm_buf_fill_hmac_session'.
> > > v2:
> > > There was spurious extra field added to tpm2_hash by mistake.
> > > ---
> > > drivers/char/tpm/tpm2-cmd.c | 23 +++-
> > > drivers/char/tpm/tpm2-sessions.c | 131 +++++++++++++++-------
> > > include/linux/tpm.h | 6 +-
> > > security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
> > > 4 files changed, 136 insertions(+), 53 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 5b6ccf901623..4473b81122e8 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -187,7 +187,11 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > > }
> > >
> > > if (!disable_pcr_integrity) {
> > > - tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > + if (rc) {
> > > + tpm_buf_destroy(&buf);
> > > + return rc;
> > > + }
> > > tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > } else {
> > > tpm_buf_append_handle(chip, &buf, pcr_idx);
> > > @@ -202,8 +206,14 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > > chip->allocated_banks[i].digest_size);
> > > }
> > >
> > > - if (!disable_pcr_integrity)
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (!disable_pcr_integrity) {
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc) {
> > > + tpm_buf_destroy(&buf);
> > > + return rc;
> > > + }
> > > + }
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
> > > if (!disable_pcr_integrity)
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > > @@ -261,7 +271,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> > > | TPM2_SA_CONTINUE_SESSION,
> > > NULL, 0);
> > > tpm_buf_append_u16(&buf, num_bytes);
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + err = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (err) {
> > > + tpm_buf_destroy(&buf);
> > > + return err;
> > > + }
> > > +
> > > err = tpm_transmit_cmd(chip, &buf,
> > > offsetof(struct tpm2_get_random_out,
> > > buffer),
> > > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> > > index 6d03c224e6b2..33ad0d668e1a 100644
> > > --- a/drivers/char/tpm/tpm2-sessions.c
> > > +++ b/drivers/char/tpm/tpm2-sessions.c
> > > @@ -144,16 +144,24 @@ struct tpm2_auth {
> > > /*
> > > * Name Size based on TPM algorithm (assumes no hash bigger than 255)
> > > */
> > > -static u8 name_size(const u8 *name)
> > > +static int name_size(const u8 *name)
> > > {
> > > - static u8 size_map[] = {
> > > - [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
> > > - [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
> > > - [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
> > > - [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
> > > - };
> > > - u16 alg = get_unaligned_be16(name);
> > > - return size_map[alg] + 2;
> > > + u16 hash_alg = get_unaligned_be16(name);
> > > +
> > > + switch (hash_alg) {
> > > + case TPM_ALG_SHA1:
> > > + return SHA1_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SHA256:
> > > + return SHA256_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SHA384:
> > > + return SHA384_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SHA512:
> > > + return SHA512_DIGEST_SIZE + 2;
> > > + case TPM_ALG_SM3_256:
> > > + return SM3256_DIGEST_SIZE + 2;
> > > + }
> > > +
> >
> > Can we/should we perhaps print a warning here if we don't know the
> > algorithm?
>
> I think it is a good idea to do that right now.
>
> Also, it'd be better to not have SM3 label as SM2/SM3 is not supported
> at this point. I'm working simulatenously on an improved feature and
> that slipped from that work:
>
> My big picture roadmap for this feature, and how to make it useful is:
>
> 1. tpm.integrity_mode=disabled/permissive/enforced. Permissive means
> here that the feature is conditionally enabled if algorithms that are
> required to enable the HMAC pipe are available.
> 2. tpm.integrity_handle=0x00000000/0x81??????.
>
> >
> > > + return -EINVAL;
> > > }
> > >
> > > static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > > @@ -161,6 +169,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > > struct tpm_header *head = (struct tpm_header *)buf->data;
> > > off_t offset = TPM_HEADER_SIZE;
> > > u32 tot_len = be32_to_cpu(head->length);
> > > + int ret;
> > > u32 val;
> > >
> > > /* we're starting after the header so adjust the length */
> > > @@ -172,9 +181,15 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
> > > return -EINVAL;
> > > offset += val;
> > > /* name */
> > > +
> > > val = tpm_buf_read_u16(buf, &offset);
> > > - if (val != name_size(&buf->data[offset]))
> > > + ret = name_size(&buf->data[offset]);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (val != ret)
> > > return -EINVAL;
> > > +
> > > memcpy(name, &buf->data[offset], val);
> > > /* forget the rest */
> > > return 0;
> > > @@ -221,46 +236,70 @@ static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
> > > * As with most tpm_buf operations, success is assumed because failure
> > > * will be caused by an incorrect programming model and indicated by a
> > > * kernel message.
> > > + *
> > > + * Ends the authorization session on failure.
> > > */
> > > -void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > - u32 handle, u8 *name)
> > > +int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > + u32 handle, u8 *name)
> > > {
> > > #ifdef CONFIG_TCG_TPM2_HMAC
> > > enum tpm2_mso_type mso = tpm2_handle_mso(handle);
> > > struct tpm2_auth *auth;
> > > int slot;
> > > + int ret;
> > > #endif
> > >
> > > if (!tpm2_chip_auth(chip)) {
> > > tpm_buf_append_handle(chip, buf, handle);
> > > - return;
> > > + return 0;
> > > }
> > >
> > > #ifdef CONFIG_TCG_TPM2_HMAC
> > > slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
> > > if (slot >= AUTH_MAX_NAMES) {
> > > - dev_err(&chip->dev, "TPM: too many handles\n");
> > > - return;
> > > + dev_err(&chip->dev, "too many handles\n");
> > > + ret = -EIO;
> > > + goto err;
> > > }
> > > auth = chip->auth;
> > > - WARN(auth->session != tpm_buf_length(buf),
> > > - "name added in wrong place\n");
> > > + if (auth->session != tpm_buf_length(buf)) {
> > > + dev_err(&chip->dev, "session state malformed");
> > > + ret = -EIO;
> > > + goto err;
> > > + }
> > > tpm_buf_append_u32(buf, handle);
> > > auth->session += 4;
> > >
> > > if (mso == TPM2_MSO_PERSISTENT ||
> > > mso == TPM2_MSO_VOLATILE ||
> > > mso == TPM2_MSO_NVRAM) {
> > > - if (!name)
> > > - tpm2_read_public(chip, handle, auth->name[slot]);
> > > + if (!name) {
> > > + ret = tpm2_read_public(chip, handle, auth->name[slot]);
> > > + if (ret)
> > > + goto err;
> > > + }
> > > } else {
> > > - if (name)
> > > - dev_err(&chip->dev, "TPM: Handle does not require name but one is specified\n");
> >
> > We're dropping the error message here; is there a reason for that?
>
> Thanks, I'll add it back.
>
> >
> > > + if (name) {
> > > + ret = -EIO;
> > > + goto err;
> > > + }
> > > }
> > >
> > > auth->name_h[slot] = handle;
> > > - if (name)
> > > - memcpy(auth->name[slot], name, name_size(name));
> > > + if (name) {
> > > + ret = name_size(name);
> > > + if (ret < 0)
> > > + goto err;
> > > +
> > > + memcpy(auth->name[slot], name, ret);
> > > + }
> > > +#endif
> > > + return 0;
> > > +
> > > +#ifdef CONFIG_TCG_TPM2_HMAC
> > > +err:
> > > + tpm2_end_auth_session(chip);
> > > + return tpm_ret_to_err(ret);
> > > #endif
> > > }
> > > EXPORT_SYMBOL_GPL(tpm_buf_append_name);
> > > @@ -533,11 +572,9 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
> > > * encryption key and encrypts the first parameter of the command
> > > * buffer with it.
> > > *
> > > - * As with most tpm_buf operations, success is assumed because failure
> > > - * will be caused by an incorrect programming model and indicated by a
> > > - * kernel message.
> > > + * Ends the authorization session on failure.
> > > */
> > > -void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > +int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > {
> > > u32 cc, handles, val;
> > > struct tpm2_auth *auth = chip->auth;
> > > @@ -549,9 +586,12 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > u8 cphash[SHA256_DIGEST_SIZE];
> > > struct sha256_ctx sctx;
> > > struct hmac_sha256_ctx hctx;
> > > + int ret;
> > >
> > > - if (!auth)
> > > - return;
> > > + if (!auth) {
> > > + ret = -EINVAL;
> > > + goto err;
> > > + }
> > >
> > > /* save the command code in BE format */
> > > auth->ordinal = head->ordinal;
> > > @@ -560,9 +600,10 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > >
> > > i = tpm2_find_cc(chip, cc);
> > > if (i < 0) {
> > > - dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
> >
> > Again, I think it's generally helpful to have the error message given that
> > the return (EINVAL) does not help narrow down which value is bad.
>
> Agreed.
>
> >
> > > - return;
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > > +
> > > attrs = chip->cc_attrs_tbl[i];
> > >
> > > handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
> > > @@ -576,9 +617,9 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > u32 handle = tpm_buf_read_u32(buf, &offset_s);
> > >
> > > if (auth->name_h[i] != handle) {
> > > - dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
> > > - i);
> > > - return;
> > > + dev_err(&chip->dev, "invalid handle 0x%08x\n", handle);
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > > }
> > > /* point offset_s to the start of the sessions */
> > > @@ -609,12 +650,14 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > offset_s += len;
> > > }
> > > if (offset_s != offset_p) {
> > > - dev_err(&chip->dev, "TPM session length is incorrect\n");
> > > - return;
> > > + dev_err(&chip->dev, "session length is incorrect\n");
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > > if (!hmac) {
> > > - dev_err(&chip->dev, "TPM could not find HMAC session\n");
> > > - return;
> > > + dev_err(&chip->dev, "could not find HMAC session\n");
> > > + ret = -EINVAL;
> > > + goto err;
> > > }
> > >
> > > /* encrypt before HMAC */
> > > @@ -646,8 +689,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > if (mso == TPM2_MSO_PERSISTENT ||
> > > mso == TPM2_MSO_VOLATILE ||
> > > mso == TPM2_MSO_NVRAM) {
> > > - sha256_update(&sctx, auth->name[i],
> > > - name_size(auth->name[i]));
> > > + ret = name_size(auth->name[i]);
> > > + if (ret < 0)
> > > + goto err;
> > > +
> > > + sha256_update(&sctx, auth->name[i], ret);
> > > } else {
> > > __be32 h = cpu_to_be32(auth->name_h[i]);
> > >
> > > @@ -668,6 +714,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> > > hmac_sha256_update(&hctx, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> > > hmac_sha256_update(&hctx, &auth->attrs, 1);
> > > hmac_sha256_final(&hctx, hmac);
> > > + return 0;
> > > +
> > > +err:
> > > + tpm2_end_auth_session(chip);
> > > + return ret;
> > > }
> > > EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
> > >
> > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > index 0e9e043f728c..1a59f0190eb3 100644
> > > --- a/include/linux/tpm.h
> > > +++ b/include/linux/tpm.h
> > > @@ -528,8 +528,8 @@ static inline struct tpm2_auth *tpm2_chip_auth(struct tpm_chip *chip)
> > > #endif
> > > }
> > >
> > > -void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > - u32 handle, u8 *name);
> > > +int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> > > + u32 handle, u8 *name);
> > > void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
> > > u8 attributes, u8 *passphrase,
> > > int passphraselen);
> > > @@ -562,7 +562,7 @@ static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
> > > #ifdef CONFIG_TCG_TPM2_HMAC
> > >
> > > int tpm2_start_auth_session(struct tpm_chip *chip);
> > > -void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
> > > +int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf);
> > > int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
> > > int rc);
> > > void tpm2_end_auth_session(struct tpm_chip *chip);
> > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> > > index e165b117bbca..7672a4376dad 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > > @@ -283,7 +283,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> > > goto out_put;
> > > }
> > >
> > > - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + if (rc)
> > > + goto out;
> > > +
> > > tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> > > options->keyauth, TPM_DIGEST_SIZE);
> > >
> > > @@ -331,7 +334,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> > > goto out;
> > > }
> > >
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc)
> > > + goto out;
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > > if (rc)
> > > @@ -438,7 +444,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > > return rc;
> > > }
> > >
> > > - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + if (rc)
> > > + goto out;
> > > +
> > > tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
> > > TPM_DIGEST_SIZE);
> > >
> > > @@ -450,7 +459,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > > goto out;
> > > }
> > >
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc)
> > > + goto out;
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > > if (!rc)
> > > @@ -497,7 +509,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> > > return rc;
> > > }
> > >
> > > - tpm_buf_append_name(chip, &buf, blob_handle, NULL);
> > > + rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> > > + if (rc)
> > > + goto out;
> > >
> > > if (!options->policyhandle) {
> > > tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
> > > @@ -522,7 +536,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> > > NULL, 0);
> > > }
> > >
> > > - tpm_buf_fill_hmac_session(chip, &buf);
> > > + rc = tpm_buf_fill_hmac_session(chip, &buf);
> > > + if (rc)
> > > + goto out;
> > > +
> > > rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
> > > rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> > >
> > > --
> > > 2.52.0
> > >
> >
> > J.
> >
> > --
> > Be Ye Not Lost Among Precepts of Order
>
> BR, Jarkko
Addressed in: https://lore.kernel.org/linux-integrity/20251202202643.107108-2-jarkko@kernel.org/
BR, Jarkko
^ permalink raw reply
* [PATCH 0/1] Trim N entries of IMA event logs
From: steven chen @ 2025-12-02 23:28 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring
The Integrity Measurement Architecture (IMA) maintains a measurement list
—a record of system events used for integrity verification. The IMA event
logs are the entries within this measurement list, each representing a
specific event or measurement that contributes to the system's integrity
assessment.
This update introduces the ability to trim, or remove, N entries from the
current measurement list. Trimming involves deleting N entries from the
list and clearing the corresponding entries from the hash table. This
action atomically truncates the measurement list, ensuring that no new
measurements can be added until the operation is complete. Importantly,
only one writer can initiate this trimming process at a time, maintaining
consistency and preventing race conditions.
A userspace interface, ima_trim_log, has been provided for this purpose.
By writing a number N to this interface, userspace can request the kernel
to trim N entries from the IMA event logs. When this interface is read,
it returns the number of entries trimmed during the last operation. This
value is not preserved across kexec soft reboots, as it is not considered
important information.
To maintain a complete record, userspace is responsible for concatenating
and storing the logs before initiating trimming. Userspace can then send
the collected data to remote verifiers for validation. After receiving
confirmation from the remote verifiers, userspace may instruct the kernel
to proceed with trimming the IMA event logs accordingly.
The primary benefit of this solution is the ability to free valuable
kernel memory by delegating the task of reconstructing the full
measurement list from log chunks to userspace. Trust is not required in
userspace for the integrity of the measurement list, as its integrity is
cryptographically protected by the Trusted Platform Module (TPM).
Multiple readers are allowed to access the ima_trim_log interface
concurrently, while only one writer can trigger log trimming at any time.
During trimming, readers do not see the list and cannot access it while
deletion is in progress, ensuring atomicity.
The time required for trimming is minimal, and IMA event logs are briefly
on hold during this process, preventing read or add operations. This short
interruption has no impact on the overall functionality of IMA.
steven chen (1):
IMA event log trimming
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_fs.c | 78 ++++++++++++++++++++++++++++++
security/integrity/ima/ima_queue.c | 42 ++++++++++++++++
3 files changed, 122 insertions(+)
--
2.43.0
^ permalink raw reply
* [PATCH 1/1] IMA event log trimming
From: steven chen @ 2025-12-02 23:28 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring
In-Reply-To: <20251202232857.8211-1-chenste@linux.microsoft.com>
This patch is for trimming N entries of the IMA event logs as well as
cleaning the hash table.
It provides a userspace interface ima_trim_log that can be used to input
number N to let kernel to trim N entries of IMA event logs. When read
this interface, it returns number of entries trimmed last tim.
A mutex ima_trim_list_mutex is provided to allow one trimming request
at a time.
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_fs.c | 78 ++++++++++++++++++++++++++++++
security/integrity/ima/ima_queue.c | 42 ++++++++++++++++
3 files changed, 122 insertions(+)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e3d71d8d56e3..ab0e30ee25ea 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
#ifdef CONFIG_IMA_KEXEC
void ima_measure_kexec_event(const char *event_name);
+long ima_purge_event_log(long number_logs);
#else
static inline void ima_measure_kexec_event(const char *event_name) {}
+static inline long ima_purge_event_log(long number_logs) { return 0; }
#endif
/*
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 87045b09f120..ea93448feedd 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -38,6 +38,11 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
static int valid_policy = 1;
+#define IMA_LOG_TRIM_REQ_LENGTH 11
+static long trimcount;
+/* mutex protects atomicity of trimming measurement list requests */
+static DEFINE_MUTEX(ima_trim_list_mutex);
+
static ssize_t ima_show_htable_value(char __user *buf, size_t count,
loff_t *ppos, atomic_long_t *val)
{
@@ -289,6 +294,69 @@ static const struct file_operations ima_ascii_measurements_ops = {
.release = seq_release,
};
+static int ima_log_trim_open(struct inode *inode, struct file *filp)
+{
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ return 0;
+}
+
+static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
+{
+ char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
+ ssize_t len;
+
+ len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
+ return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
+}
+
+static ssize_t ima_log_trim_write(struct file *file,
+ const char __user *buf, size_t datalen, loff_t *ppos)
+{
+ unsigned char req[IMA_LOG_TRIM_REQ_LENGTH];
+ long count, n;
+ int ret;
+
+ mutex_lock(&ima_trim_list_mutex);
+
+ if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ n = (int)datalen;
+
+ ret = copy_from_user(req, buf, datalen);
+ if (ret < 0)
+ goto out;
+
+ count = 0;
+ for (int i = 0; i < n; ++i) {
+ if (req[i] < '0' || req[i] > '9') {
+ ret = -EINVAL;
+ goto out;
+ }
+ count = count * 10 + req[i] - '0';
+ }
+ ret = ima_purge_event_log(count);
+
+ if (ret < 0)
+ goto out;
+
+ trimcount = ret;
+ ret = datalen;
+out:
+ mutex_unlock(&ima_trim_list_mutex);
+ return ret;
+}
+
+static const struct file_operations ima_log_trim_ops = {
+ .open = ima_log_trim_open,
+ .read = ima_log_trim_read,
+ .write = ima_log_trim_write,
+ .llseek = generic_file_llseek,
+};
+
static ssize_t ima_read_policy(char *path)
{
void *data = NULL;
@@ -528,6 +596,16 @@ int __init ima_fs_init(void)
goto out;
}
+ dentry = securityfs_create_file("ima_trim_log",
+ S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
+ ima_dir, NULL, &ima_log_trim_ops);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
+ trimcount = 0;
+
dentry = securityfs_create_file("runtime_measurements_count",
S_IRUSR | S_IRGRP, ima_dir, NULL,
&ima_measurements_count_ops);
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 590637e81ad1..999cd42c517c 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -220,6 +220,48 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
return result;
}
+/* Delete the IMA event logs */
+long ima_purge_event_log(long number_logs)
+{
+ struct ima_queue_entry *qe;
+ long cur = 0;
+
+ if (number_logs <= 0)
+ return number_logs;
+
+ mutex_lock(&ima_extend_list_mutex);
+ rcu_read_lock();
+
+ /*
+ * Remove this entry from both hash table and the measurement list
+ * When removing from hash table, decrease the length counter
+ * so that the hash table re-sizing logic works correctly
+ */
+ list_for_each_entry_rcu(qe, &ima_measurements, later) {
+ int i;
+
+ /* if CONFIG_IMA_DISABLE_HTABLE is set, the hash table is not used */
+ if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE))
+ hlist_del_rcu(&qe->hnext);
+
+ for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
+ kfree(qe->entry->template_data[i].data);
+ qe->entry->template_data[i].data = NULL;
+ qe->entry->template_data[i].len = 0;
+ }
+
+ atomic_long_dec(&ima_htable.len);
+ list_del_rcu(&qe->later);
+ ++cur;
+ if (cur >= number_logs)
+ break;
+ }
+
+ rcu_read_unlock();
+ mutex_unlock(&ima_extend_list_mutex);
+ return cur;
+}
+
int ima_restore_measurement_entry(struct ima_template_entry *entry)
{
int result = 0;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 00/46] Allow inlining C helpers into Rust when using LTO
From: Boqun Feng @ 2025-12-03 1:47 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, linux-kernel, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Peter Zijlstra, Elle Rhumsaa,
Carlos Llamas, Yury Norov, Andreas Hindborg, linux-block,
FUJITA Tomonori, Miguel Ojeda, Michael Turquette, Stephen Boyd,
linux-clk, Benno Lossin, Danilo Krummrich, Thomas Gleixner,
Rafael J. Wysocki, Viresh Kumar, linux-pm, Paul Moore,
Serge Hallyn, linux-security-module, Daniel Almeida,
Abdiel Janulgue, Robin Murphy, Lyude Paul, Alexander Viro,
Christian Brauner, Jan Kara, linux-fsdevel, Josh Poimboeuf,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Brendan Higgins,
David Gow, Rae Moar, linux-kselftest, Andrew Morton,
Liam R. Howlett, Andrew Ballance, maple-tree, linux-mm,
Lorenzo Stoakes, Uladzislau Rezki, Vitaly Wool, Rob Herring,
Saravana Kannan, devicetree, Bjorn Helgaas,
Krzysztof Wilczy´nski, linux-pci, Remo Senekowitsch,
Paul E. McKenney, rcu, Will Deacon, Fiona Behrens, Gary Guo,
Liam Girdwood, Mark Brown, Alexandre Courbot, Vlastimil Babka,
Christoph Lameter, David Rientjes, Ingo Molnar, Waiman Long,
Mitchell Levy, Frederic Weisbecker, Anna-Maria Behnsen,
John Stultz, linux-usb, Tejun Heo, Lai Jiangshan, Matthew Wilcox,
Tamir Duberstein
In-Reply-To: <20251202-define-rust-helper-v1-0-a2e13cbc17a6@google.com>
On Tue, Dec 02, 2025 at 07:37:24PM +0000, Alice Ryhl wrote:
> This patch series adds __rust_helper to every single rust helper. The
> patches do not depend on each other, so maintainers please go ahead and
> pick up any patches relevant to your subsystem! Or provide your Acked-by
> so that Miguel can pick them up.
>
> These changes were generated by adding __rust_helper and running
> ClangFormat. Unrelated formatting changes were removed manually.
>
> Why is __rust_helper needed?
> ============================
>
> Currently, C helpers cannot be inlined into Rust even when using LTO
> because LLVM detects slightly different options on the codegen units.
>
> * LLVM doesn't want to inline functions compiled with
> `-fno-delete-null-pointer-checks` with code compiled without. The C
> CGUs all have this enabled and Rust CGUs don't. Inlining is okay since
> this is one of the hardening features that does not change the ABI,
> and we shouldn't have null pointer dereferences in these helpers.
>
> * LLVM doesn't want to inline functions with different list of builtins. C
> side has `-fno-builtin-wcslen`; `wcslen` is not a Rust builtin, so
> they should be compatible, but LLVM does not perform inlining due to
> attributes mismatch.
>
> * clang and Rust doesn't have the exact target string. Clang generates
> `+cmov,+cx8,+fxsr` but Rust doesn't enable them (in fact, Rust will
> complain if `-Ctarget-feature=+cmov,+cx8,+fxsr` is used). x86-64
> always enable these features, so they are in fact the same target
> string, but LLVM doesn't understand this and so inlining is inhibited.
> This can be bypassed with `--ignore-tti-inline-compatible`, but this
> is a hidden option.
>
> (This analysis was written by Gary Guo.)
>
> How is this fixed?
> ==================
>
> To fix this we need to add __always_inline to all helpers when compiling
> with LTO. However, it should not be added when running bindgen as
> bindgen will ignore functions marked inline. To achieve this, we are
> using a #define called __rust_helper that is defined differently
> depending on whether bindgen is running or not.
>
> Note that __rust_helper is currently always #defined to nothing.
> Changing it to __always_inline will happen separately in another patch
> series.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
For the whole series:
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Regards,
Boqun
> ---
[...]
^ permalink raw reply
* [GIT PULL] IPE update for 6.19
From: Fan Wu @ 2025-12-03 4:01 UTC (permalink / raw)
To: torvalds
Cc: linux-security-module, Linux Kernel Mailing List,
Borislav Petkov (AMD), Yanzhu Huang
Hi Linus,
Please merge this PR for the IPE (Integrity Policy Enforcement) update for 6.19.
This PR contains three commits. The primary change is the addition of
support for the AT_EXECVE_CHECK flag. This allows interpreters to
signal the kernel to perform IPE security checks on script files
before execution, extending IPE enforcement to indirectly executed
scripts.
These commits have been tested for several weeks in linux-next without
any issues.
Thanks,
Fan
--
The following changes since commit 7d0a66e4bb9081d75c82ec4957c50034cb0ea449:
Linux 6.18 (2025-11-30 14:42:10 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/wufan/ipe.git
tags/ipe-pr-20251202
for you to fetch changes up to d7ba853c0e47d57805181f5269ba250270d2adde:
ipe: Update documentation for script enforcement (2025-12-02 19:37:10 -0800)
----------------------------------------------------------------
ipe/stable-6.19 PR 20251202
----------------------------------------------------------------
Borislav Petkov (AMD) (1):
ipe: Drop a duplicated CONFIG_ prefix in the ifdeffery
Yanzhu Huang (2):
ipe: Add AT_EXECVE_CHECK support for script enforcement
ipe: Update documentation for script enforcement
Documentation/admin-guide/LSM/ipe.rst | 17 ++++++++++++++---
security/ipe/audit.c | 1 +
security/ipe/hooks.c | 29 ++++++++++++++++++++++++++++-
security/ipe/hooks.h | 3 +++
security/ipe/ipe.c | 1 +
5 files changed, 47 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH] selftests/landlock: NULL-terminate unix pathname addresses
From: Günther Noack @ 2025-12-03 9:48 UTC (permalink / raw)
To: Matthieu Buffet; +Cc: Mickaël Salaün, linux-security-module
In-Reply-To: <20251202215141.689986-1-matthieu@buffet.re>
On Tue, Dec 02, 2025 at 10:51:41PM +0100, Matthieu Buffet wrote:
> The size of Unix pathname addresses is computed in selftests using
> offsetof(struct sockaddr_un, sun_path) + strlen(xxx). It should have
> been that +1, which makes addresses passed to the libc and kernel
> non-NULL-terminated. unix_mkname_bsd() fixes that in Linux so there is
> no harm, but just using sizeof(the address struct) should improve
> readability.
>
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
> [...]
Thank you very much, this looks good!
Reviewed-by: Günther Noack <gnoack@google.com>
—Günther
^ permalink raw reply
* Re: Are setuid shell scripts safe? (Implied by security_bprm_creds_for_exec)
From: Bernd Edlinger @ 2025-12-03 13:16 UTC (permalink / raw)
To: Eric W. Biederman, Roberto Sassu
Cc: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet, zohar, linux-integrity, Ryan Lee, apparmor
In-Reply-To: <87ms42rq3t.fsf@email.froward.int.ebiederm.org>
On 12/1/25 19:53, Eric W. Biederman wrote:
> Roberto Sassu <roberto.sassu@huaweicloud.com> writes:
>
>> On Mon, 2025-12-01 at 10:06 -0600, Eric W. Biederman wrote:
>>> Roberto Sassu <roberto.sassu@huaweicloud.com> writes:
>>>
>>>> + Mimi, linux-integrity (would be nice if we are in CC when linux-
>>>> security-module is in CC).
>>>>
>>>> Apologies for not answering earlier, it seems I don't receive the
>>>> emails from the linux-security-module mailing list (thanks Serge for
>>>> letting me know!).
>>>>
>>>> I see two main effects of this patch. First, the bprm_check_security
>>>> hook implementations will not see bprm->cred populated. That was a
>>>> problem before we made this patch:
>>>>
>>>> https://patchew.org/linux/20251008113503.2433343-1-roberto.sassu@huaweicloud.com/
>>>
>>> Thanks, that is definitely needed.
>>>
>>> Does calling process_measurement(CREDS_CHECK) on only the final file
>>> pass review? Do you know of any cases where that will break things?
>>
>> We intentionally changed the behavior of CREDS_CHECK to be invoked only
>> for the final file. We are monitoring for bug reports, if we receive
>> complains from people that the patch breaks their expectation we will
>> revisit the issue.
>>
>> Any LSM implementing bprm_check_security looking for brpm->cred would
>> be affected by recalculating the DAC credentials for the final binary.
>>
>>> As it stands I don't think it should be assumed that any LSM has
>>> computed it's final creds until bprm_creds_from_file. Not just the
>>> uid and gid.
>>
>> Uhm, I can be wrong, but most LSMs calculate their state change in
>> bprm_creds_for_exec (git grep bprm_creds_for_exec|grep LSM_HOOK_INIT).
>>
>>> If the patch you posted for review works that helps sort that mess out.
>>
>> Well, it works because we changed the expectation :)
>
> I just haven't seen that code land in Linus's tree yet so I am a bit
> cautious in adopting that. It is definitely needed as the behavior
> of IMA as v6.18 simply does not work in general.
>
>>>> to work around the problem of not calculating the final DAC credentials
>>>> early enough (well, we actually had to change our CREDS_CHECK hook
>>>> behavior).
>>>>
>>>> The second, I could not check. If I remember well, unlike the
>>>> capability LSM, SELinux/Apparmor/SMACK calculate the final credentials
>>>> based on the first file being executed (thus the script, not the
>>>> interpreter). Is this patch keeping the same behavior despite preparing
>>>> the credentials when the final binary is found?
>>>
>>> The patch I posted was.
>>>
>>> My brain is still reeling from the realization that our security modules
>>> have the implicit assumption that it is safe to calculate their security
>>> information from shell scripts.
>>
>> If I'm interpreting this behavior correctly (please any LSM maintainer
>> could comment on it), the intent is just to transition to a different
>> security context where a different set of rules could apply (since we
>> are executing a script).
>>
>> Imagine if for every script, the security transition is based on the
>> interpreter, it would be hard to differentiate between scripts and
>> associate to the respective processes different security labels.
>>
>>> In the first half of the 90's I remember there was lots of effort to try
>>> and make setuid shell scripts and setuid perl scripts work, and the
>>> final conclusion was it was a lost cause.
>>
>> Definitely I lack a lot of context...
>
> From the usenet comp.unix.faq that was probably updated in 1994:
> http://www.faqs.org/faqs/unix-faq/faq/part4/section-7.html
>
> I have been trying to remember enough details by looking it up, but the
> short version is that one of the big problems is there is a race between
> the kernel doing it's thing and the shell opening the shell script.
>
> Clever people have been able to take advantage of that race and insert
> arbitrary code in that window for the shell to execute. All you have to
> do is google for how to find a reproducer if the one in the link above
> is not enough.
>
>>> Now I look at security_bprm_creds_for_exec and security_bprm_check which
>>> both have the implicit assumption that it is indeed safe to compute the
>>> credentials from a shell script.
>>>
>>> When passing a file descriptor to execat we have
>>> BINPRM_FLAGS_PATH_INACCESSIBLE and use /dev/fd/NNN as the filename
>>> which reduces some of the races.
>>>
>>> However when just plain executing a shell script we pass the filename of
>>> the shell script as a command line argument, and expect the shell to
>>> open the filename again. This has been a time of check to time of use
>>> race for decades, and one of the reasons we don't have setuid shell
>>> scripts.
>>
>> Yes, it would be really nice to fix it!
>
> After 30 years I really don't expect that is even a reasonable request.
>
> I think we are solidly into "Don't do that then", and the LSM security
> hooks are definitely doing that.
>
> There is the partial solution of passing /dev/fd instead of passing the
> name of the script. I suspect that would break things. I don't
> remember why that was never adopted.
>
> I think even with the TOCTOU race fixed there were other serious issues.
>
> I really think it behooves any security module people who want to use
> the shell script as the basis of their security decisions to research
> all of the old well known issues and describe how they don't apply.
>
> All I have energy for is to point out it is broken as is and to start
> moving code down into bprm_creds_from_file to avoid the race.
>
> Right now as far as I can tell anything based upon the script itself
> is worthless junk so changing that would not be breaking anything that
> wasn't already broken.
>
>>> Yet the IMA implementation (without the above mentioned patch) assumes
>>> the final creds will be calculated before security_bprm_check is called,
>>> and security_bprm_creds_for_exec busily calculate the final creds.
>>>
>>> For some of the security modules I believe anyone can set any label they
>>> want on a file and they remain secure (At which point I don't understand
>>> the point of having labels on files). I don't believe that is the case
>>> for selinux, or in general.
>>
>> A simple example for SELinux. Suppose that the parent process has type
>> initrc_t, then the SELinux policy configures the following transitions
>> based on the label of the first file executed (sesearch -T -s initrc_t
>> -c process):
>>
>> type_transition initrc_t NetworkManager_dispatcher_exec_t:process NetworkManager_dispatcher_t;
>> type_transition initrc_t NetworkManager_exec_t:process NetworkManager_t;
>> type_transition initrc_t NetworkManager_initrc_exec_t:process initrc_t;
>> type_transition initrc_t NetworkManager_priv_helper_exec_t:process NetworkManager_priv_helper_t;
>> type_transition initrc_t abrt_dump_oops_exec_t:process abrt_dump_oops_t;
>> type_transition initrc_t abrt_exec_t:process abrt_t;
>> [...]
>>
>> (there are 747 rules in my system).
>>
>> If the transition would be based on the interpreter label, it would be
>> hard to express with rules.
>
> Which is a problem for the people making the rules engine. Because
> 30 years of experience with this problem says basing anything on the
> script is already broken.
>
> I understand the frustration, but it requires a new way of launching
> shell scripts to even begin to make it secure.
>
>> If the transition does not occur for any reason the parent process
>> policy would still apply, but maybe it would not have the necessary
>> permissions for the execution of the script.
>
> Yep.
>
>>> So just to remove the TOCTOU race the security_bprm_creds_for_exec
>>> and security_bprm_check hooks need to be removed, after moving their
>>> code into something like security_bprm_creds_from_file.
>>>
>>> Or am I missing something and even with the TOCTOU race are setuid shell
>>> scripts somehow safe now?
>>
>> Take this with a looot of salt, if there is a TOCTOU race, the script
>> will be executed with a security context that does not belong to it.
>> But the transition already happened. Not sure if it is safe.
>
> Historically it hasn't been safe.
>
>> I also don't know how the TOCTOU race could be solved, but I also would
>> like it to be fixed. I'm available to comment on any proposal!
>
> I am hoping someone who helped put these security hooks where they are
> will speak up, and tell me what I am missing.
>
> All I have the energy for right now is to point out security policies
> based upon shell scripts appear to be security policies that only
> protect you from well behaved programs.
>
Hmm, yes, that looks like an issue.
I would have expected the security engine to look at bprm->filenanme
especially in the case, when bprm->interp != bprm->filename,
and check that it is not a sym-link with write-access for the
current user and of course also that the bprm->file is not a regular file
which is writable by the current user, if that is the case I would have expected
the secuity engine to enforce non-new-privs on a SUID executable somehow.
Bernd.
^ permalink raw reply
* Re: [PATCH 00/46] Allow inlining C helpers into Rust when using LTO
From: Gary Guo @ 2025-12-03 14:33 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, linux-kernel, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Peter Zijlstra, Boqun Feng,
Elle Rhumsaa, Carlos Llamas, Yury Norov, Andreas Hindborg,
linux-block, FUJITA Tomonori, Miguel Ojeda, Michael Turquette,
Stephen Boyd, linux-clk, Benno Lossin, Danilo Krummrich,
Thomas Gleixner, Rafael J. Wysocki, Viresh Kumar, linux-pm,
Paul Moore, Serge Hallyn, linux-security-module, Daniel Almeida,
Abdiel Janulgue, Robin Murphy, Lyude Paul, Alexander Viro,
Christian Brauner, Jan Kara, linux-fsdevel, Josh Poimboeuf,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Brendan Higgins,
David Gow, Rae Moar, linux-kselftest, Andrew Morton,
Liam R. Howlett, Andrew Ballance, maple-tree, linux-mm,
Lorenzo Stoakes, Uladzislau Rezki, Vitaly Wool, Rob Herring,
Saravana Kannan, devicetree, Bjorn Helgaas,
Krzysztof Wilczyński, linux-pci, Remo Senekowitsch,
Paul E. McKenney, rcu, Will Deacon, Fiona Behrens, Liam Girdwood,
Mark Brown, Alexandre Courbot, Vlastimil Babka, Christoph Lameter,
David Rientjes, Ingo Molnar, Waiman Long, Mitchell Levy,
Frederic Weisbecker, Anna-Maria Behnsen, John Stultz, linux-usb,
Tejun Heo, Lai Jiangshan, Matthew Wilcox, Tamir Duberstein
In-Reply-To: <20251202-define-rust-helper-v1-0-a2e13cbc17a6@google.com>
On Tue, 02 Dec 2025 19:37:24 +0000
Alice Ryhl <aliceryhl@google.com> wrote:
> This patch series adds __rust_helper to every single rust helper. The
> patches do not depend on each other, so maintainers please go ahead and
> pick up any patches relevant to your subsystem! Or provide your Acked-by
> so that Miguel can pick them up.
>
> These changes were generated by adding __rust_helper and running
> ClangFormat. Unrelated formatting changes were removed manually.
>
> Why is __rust_helper needed?
> ============================
>
> Currently, C helpers cannot be inlined into Rust even when using LTO
> because LLVM detects slightly different options on the codegen units.
>
> * LLVM doesn't want to inline functions compiled with
> `-fno-delete-null-pointer-checks` with code compiled without. The C
> CGUs all have this enabled and Rust CGUs don't. Inlining is okay since
> this is one of the hardening features that does not change the ABI,
> and we shouldn't have null pointer dereferences in these helpers.
>
> * LLVM doesn't want to inline functions with different list of builtins. C
> side has `-fno-builtin-wcslen`; `wcslen` is not a Rust builtin, so
> they should be compatible, but LLVM does not perform inlining due to
> attributes mismatch.
>
> * clang and Rust doesn't have the exact target string. Clang generates
> `+cmov,+cx8,+fxsr` but Rust doesn't enable them (in fact, Rust will
> complain if `-Ctarget-feature=+cmov,+cx8,+fxsr` is used). x86-64
> always enable these features, so they are in fact the same target
> string, but LLVM doesn't understand this and so inlining is inhibited.
> This can be bypassed with `--ignore-tti-inline-compatible`, but this
> is a hidden option.
>
> (This analysis was written by Gary Guo.)
>
> How is this fixed?
> ==================
>
> To fix this we need to add __always_inline to all helpers when compiling
> with LTO. However, it should not be added when running bindgen as
> bindgen will ignore functions marked inline. To achieve this, we are
> using a #define called __rust_helper that is defined differently
> depending on whether bindgen is running or not.
>
> Note that __rust_helper is currently always #defined to nothing.
> Changing it to __always_inline will happen separately in another patch
> series.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Alice Ryhl (46):
> rust: auxiliary: add __rust_helper to helpers
> rust: barrier: add __rust_helper to helpers
> rust: binder: add __rust_helper to helpers
> rust: bitmap: add __rust_helper to helpers
> rust: bitops: add __rust_helper to helpers
> rust: blk: add __rust_helper to helpers
> rust: bug: add __rust_helper to helpers
> rust: clk: add __rust_helper to helpers
> rust: completion: add __rust_helper to helpers
> rust: cpu: add __rust_helper to helpers
> rust: cpufreq: add __rust_helper to helpers
> rust: cpumask: add __rust_helper to helpers
> rust: cred: add __rust_helper to helpers
> rust: device: add __rust_helper to helpers
> rust: dma: add __rust_helper to helpers
> rust: drm: add __rust_helper to helpers
> rust: err: add __rust_helper to helpers
> rust: fs: add __rust_helper to helpers
> rust: io: add __rust_helper to helpers
> rust: irq: add __rust_helper to helpers
> rust: jump_label: add __rust_helper to helpers
> rust: kunit: add __rust_helper to helpers
> rust: maple_tree: add __rust_helper to helpers
> rust: mm: add __rust_helper to helpers
> rust: of: add __rust_helper to helpers
> rust: pci: add __rust_helper to helpers
> rust: pid_namespace: add __rust_helper to helpers
> rust: platform: add __rust_helper to helpers
> rust: poll: add __rust_helper to helpers
> rust: processor: add __rust_helper to helpers
> rust: property: add __rust_helper to helpers
> rust: rbtree: add __rust_helper to helpers
> rust: rcu: add __rust_helper to helpers
> rust: refcount: add __rust_helper to helpers
> rust: regulator: add __rust_helper to helpers
> rust: scatterlist: add __rust_helper to helpers
> rust: security: add __rust_helper to helpers
> rust: slab: add __rust_helper to helpers
> rust: sync: add __rust_helper to helpers
> rust: task: add __rust_helper to helpers
> rust: time: add __rust_helper to helpers
> rust: uaccess: add __rust_helper to helpers
> rust: usb: add __rust_helper to helpers
> rust: wait: add __rust_helper to helpers
> rust: workqueue: add __rust_helper to helpers
> rust: xarray: add __rust_helper to helpers
Thansk for sending this Alice! With this series in first, my series for
inlining helpers should be much easier to apply.
For the whole series:
Reviewed-by: Gary Guo <gary@garyguo.net>
Best,
Gary
>
> rust/helpers/auxiliary.c | 6 +++--
> rust/helpers/barrier.c | 6 ++---
> rust/helpers/binder.c | 13 ++++-----
> rust/helpers/bitmap.c | 6 +++--
> rust/helpers/bitops.c | 11 +++++---
> rust/helpers/blk.c | 4 +--
> rust/helpers/bug.c | 4 +--
> rust/helpers/build_bug.c | 2 +-
> rust/helpers/clk.c | 24 +++++++++--------
> rust/helpers/completion.c | 2 +-
> rust/helpers/cpu.c | 2 +-
> rust/helpers/cpufreq.c | 3 ++-
> rust/helpers/cpumask.c | 32 +++++++++++++---------
> rust/helpers/cred.c | 4 +--
> rust/helpers/device.c | 16 +++++------
> rust/helpers/dma.c | 15 ++++++-----
> rust/helpers/drm.c | 7 ++---
> rust/helpers/err.c | 6 ++---
> rust/helpers/fs.c | 2 +-
> rust/helpers/io.c | 64 +++++++++++++++++++++++---------------------
> rust/helpers/irq.c | 6 +++--
> rust/helpers/jump_label.c | 2 +-
> rust/helpers/kunit.c | 2 +-
> rust/helpers/maple_tree.c | 3 ++-
> rust/helpers/mm.c | 20 +++++++-------
> rust/helpers/mutex.c | 13 ++++-----
> rust/helpers/of.c | 2 +-
> rust/helpers/page.c | 9 ++++---
> rust/helpers/pci.c | 13 +++++----
> rust/helpers/pid_namespace.c | 8 +++---
> rust/helpers/platform.c | 2 +-
> rust/helpers/poll.c | 5 ++--
> rust/helpers/processor.c | 2 +-
> rust/helpers/property.c | 2 +-
> rust/helpers/rbtree.c | 5 ++--
> rust/helpers/rcu.c | 4 +--
> rust/helpers/refcount.c | 10 +++----
> rust/helpers/regulator.c | 24 ++++++++++-------
> rust/helpers/scatterlist.c | 12 +++++----
> rust/helpers/security.c | 26 ++++++++++--------
> rust/helpers/signal.c | 2 +-
> rust/helpers/slab.c | 14 +++++-----
> rust/helpers/spinlock.c | 13 ++++-----
> rust/helpers/sync.c | 4 +--
> rust/helpers/task.c | 24 ++++++++---------
> rust/helpers/time.c | 12 ++++-----
> rust/helpers/uaccess.c | 8 +++---
> rust/helpers/usb.c | 3 ++-
> rust/helpers/vmalloc.c | 7 ++---
> rust/helpers/wait.c | 2 +-
> rust/helpers/workqueue.c | 8 +++---
> rust/helpers/xarray.c | 10 +++----
> 52 files changed, 280 insertions(+), 226 deletions(-)
> ---
> base-commit: 54e3eae855629702c566bd2e130d9f40e7f35bde
> change-id: 20251202-define-rust-helper-f7b531813007
>
> Best regards,
^ permalink raw reply
* Re: [PATCH v2] security,fs,nfs,net: update security_inode_listsecurity() interface
From: Stephen Smalley @ 2025-12-03 15:35 UTC (permalink / raw)
To: Paul Moore
Cc: Anna Schumaker, Trond Myklebust, Anna Schumaker, Jakub Kicinski,
Casey Schaufler, Alexander Viro, Christian Brauner, Jan Kara,
James Morris, Serge E. Hallyn, Eric Dumazet, Paolo Abeni,
Willem de Bruijn, David S. Miller, Simon Horman, Ondrej Mosnacek,
linux-nfs, linux-kernel, linux-fsdevel, linux-security-module,
netdev, selinux
In-Reply-To: <CAHC9VhQnR6TKzzzpE9XQqiFivV0ECbVx7GH+1fQmz917-MAhsw@mail.gmail.com>
On Wed, Jul 23, 2025 at 10:10 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Jun 19, 2025 at 5:18 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
> > <anna.schumaker@oracle.com> wrote:
> > > On 5/20/25 5:31 PM, Paul Moore wrote:
> > > > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> > > >> <stephen.smalley.work@gmail.com> wrote:
> > > >>>
> > > >>> Update the security_inode_listsecurity() interface to allow
> > > >>> use of the xattr_list_one() helper and update the hook
> > > >>> implementations.
> > > >>>
> > > >>> Link: https://lore.kernel.org/selinux/20250424152822.2719-1-stephen.smalley.work@gmail.com/
> > > >>>
> > > >>> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> > > >>> ---
> > > >>> This patch is relative to the one linked above, which in theory is on
> > > >>> vfs.fixes but doesn't appear to have been pushed when I looked.
> > > >>>
> > > >>> fs/nfs/nfs4proc.c | 10 ++++++----
> > > >>> fs/xattr.c | 19 +++++++------------
> > > >>> include/linux/lsm_hook_defs.h | 4 ++--
> > > >>> include/linux/security.h | 5 +++--
> > > >>> net/socket.c | 17 +++++++----------
> > > >>> security/security.c | 16 ++++++++--------
> > > >>> security/selinux/hooks.c | 10 +++-------
> > > >>> security/smack/smack_lsm.c | 13 ++++---------
> > > >>> 8 files changed, 40 insertions(+), 54 deletions(-)
> > > >>
> > > >> Thanks Stephen. Once we get ACKs from the NFS, netdev, and Smack
> > > >> folks I can pull this into the LSM tree.
> > > >
> > > > Gentle ping for Trond, Anna, Jakub, and Casey ... can I get some ACKs
> > > > on this patch? It's a little late for the upcoming merge window, but
> > > > I'd like to merge this via the LSM tree after the merge window closes.
> > >
> > > For the NFS change:
> > > Acked-by: Anna Schumaker <anna.schumaker@oracle.com>
> >
> > Hi Anna,
> >
> > Thanks for reviewing the patch. Unfortunately when merging the patch
> > today and fixing up some merge conflicts I bumped into an odd case in
> > the NFS space and I wanted to check with you on how you would like to
> > resolve it.
> >
> > Commit 243fea134633 ("NFSv4.2: fix listxattr to return selinux
> > security label")[1] adds a direct call to
> > security_inode_listsecurity() in nfs4_listxattr(), despite the
> > existing nfs4_listxattr_nfs4_label() call which calls into the same
> > LSM hook, although that call is conditional on the server supporting
> > NFS_CAP_SECURITY_LABEL. Based on a quick search, it appears the only
> > caller for nfs4_listxattr_nfs4_label() is nfs4_listxattr() so I'm
> > wondering if there isn't some room for improvement here.
> >
> > I think there are two obvious options, and I'm curious about your
> > thoughts on which of these you would prefer, or if there is another
> > third option that you would like to see merged.
> >
> > Option #1:
> > Essentially back out commit 243fea134633, removing the direct LSM call
> > in nfs4_listxattr() and relying on the nfs4_listxattr_nfs4_label() for
> > the LSM/SELinux xattrs. I think we would want to remove the
> > NFS_CAP_SECURITY_LABEL check and build nfs4_listxattr_nfs4_label()
> > regardless of CONFIG_NFS_V4_SECURITY_LABEL.
> >
> > Option #2:
> > Remove nfs4_listxattr_nfs4_label() entirely and keep the direct LSM
> > call in nfs4_listxattr(), with the required changes for this patch.
> >
> > Thoughts?
> >
> > [1] https://lore.kernel.org/all/20250425180921.86702-1-okorniev@redhat.com/
>
> A gentle ping on the question above for the NFS folks. If I don't
> hear anything I'll hack up something and send it out for review, but I
> thought it would nice if we could sort out the proper fix first.
Raising this thread back up again to see if the NFS folks have a
preference on option #1 or #2 above, or
something else altogether. Should returning of the security.selinux
xattr name from listxattr() be dependent on
NFS_CAP_SECURITY_LABEL being set by the server and should it be
dependent on CONFIG_NFS_V4_SECURITY_LABEL?
^ 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