* Re: [PATCH] tpm2-sessions: Fix out of range indexing in name_size
From: Jarkko Sakkinen @ 2026-01-08 12:33 UTC (permalink / raw)
To: stable; +Cc: linux-integrity, Jonathan McDowell
In-Reply-To: <20260108123159.1008858-1-jarkko@kernel.org>
On Thu, Jan 08, 2026 at 02:31:59PM +0200, Jarkko Sakkinen wrote:
> [ Upstream commit 6e9722e9a7bfe1bbad649937c811076acf86e1fd ]
>
> '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")
> Reviewed-by: Jonathan McDowell <noodles@meta.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
This is for v6.12.
BR, Jarkko
^ permalink raw reply
* [PATCH] tpm2-sessions: Fix out of range indexing in name_size
From: Jarkko Sakkinen @ 2026-01-08 12:31 UTC (permalink / raw)
To: stable; +Cc: linux-integrity, Jarkko Sakkinen, Jonathan McDowell
[ Upstream commit 6e9722e9a7bfe1bbad649937c811076acf86e1fd ]
'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")
Reviewed-by: Jonathan McDowell <noodles@meta.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm2-cmd.c | 23 +++-
drivers/char/tpm/tpm2-sessions.c | 134 +++++++++++++++-------
include/linux/tpm.h | 13 ++-
security/keys/trusted-keys/trusted_tpm2.c | 29 ++++-
4 files changed, 143 insertions(+), 56 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index dfdcbd009720..74764eae9f10 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -250,7 +250,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);
@@ -265,8 +269,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);
@@ -324,7 +334,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 cf0b83154044..d020b74ed23c 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -144,16 +144,23 @@ struct tpm2_auth {
/*
* Name Size based on TPM algorithm (assumes no hash bigger than 255)
*/
-static u8 name_size(const u8 *name)
+static int name_size(const u8 *name)
{
- static u8 size_map[] = {
- [TPM_ALG_SHA1] = SHA1_DIGEST_SIZE,
- [TPM_ALG_SHA256] = SHA256_DIGEST_SIZE,
- [TPM_ALG_SHA384] = SHA384_DIGEST_SIZE,
- [TPM_ALG_SHA512] = SHA512_DIGEST_SIZE,
- };
- u16 alg = get_unaligned_be16(name);
- return size_map[alg] + 2;
+ u16 hash_alg = get_unaligned_be16(name);
+
+ switch (hash_alg) {
+ case TPM_ALG_SHA1:
+ return SHA1_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA256:
+ return SHA256_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA384:
+ return SHA384_DIGEST_SIZE + 2;
+ case TPM_ALG_SHA512:
+ return SHA512_DIGEST_SIZE + 2;
+ default:
+ pr_warn("tpm: unsupported name algorithm: 0x%04x\n", hash_alg);
+ return -EINVAL;
+ }
}
static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
@@ -161,6 +168,7 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
struct tpm_header *head = (struct tpm_header *)buf->data;
off_t offset = TPM_HEADER_SIZE;
u32 tot_len = be32_to_cpu(head->length);
+ int ret;
u32 val;
/* we're starting after the header so adjust the length */
@@ -173,8 +181,13 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
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 +234,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);
@@ -578,11 +617,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;
@@ -592,10 +629,13 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
u8 *hmac = NULL;
u32 attrs;
u8 cphash[SHA256_DIGEST_SIZE];
- struct sha256_state sctx;
+ struct sha256_ctx sctx;
+ int ret;
- if (!auth)
- return;
+ if (!auth) {
+ ret = -EIO;
+ goto err;
+ }
/* save the command code in BE format */
auth->ordinal = head->ordinal;
@@ -604,9 +644,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);
@@ -620,9 +662,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 */
@@ -653,12 +695,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 */
@@ -690,8 +734,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]);
@@ -712,6 +759,11 @@ void tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
sha256_update(&sctx, &auth->attrs, 1);
tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key)
+ auth->passphrase_len, 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 a3d8305e88a5..9472e13f33c2 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -521,8 +521,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);
@@ -555,7 +555,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);
@@ -569,10 +569,13 @@ static inline int tpm2_start_auth_session(struct tpm_chip *chip)
static inline void tpm2_end_auth_session(struct tpm_chip *chip)
{
}
-static inline void tpm_buf_fill_hmac_session(struct tpm_chip *chip,
- struct tpm_buf *buf)
+
+static inline int tpm_buf_fill_hmac_session(struct tpm_chip *chip,
+ struct tpm_buf *buf)
{
+ return 0;
}
+
static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
struct tpm_buf *buf,
int rc)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 024be262702f..10d1201a33e4 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)
@@ -444,7 +450,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);
@@ -456,7 +465,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)
@@ -506,7 +518,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,
@@ -531,7 +545,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
NULL, 0);
}
- tpm_buf_fill_hmac_session(chip, &buf);
+ rc = tpm_buf_fill_hmac_session(chip, &buf);
+ if (rc)
+ goto out;
+
rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
if (rc > 0)
--
2.52.0
^ permalink raw reply related
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Jeff Layton @ 2026-01-07 21:10 UTC (permalink / raw)
To: Frederick Lawler
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <aV66am9A5MmdNPbY@CMGLRV3>
On Wed, 2026-01-07 at 13:56 -0600, Frederick Lawler wrote:
> On Tue, Jan 06, 2026 at 02:50:31PM -0500, Jeff Layton wrote:
> > On Tue, 2026-01-06 at 13:33 -0600, Frederick Lawler wrote:
> > > On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> > > > Hi Jeff,
> > > >
> > > > On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > > > > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > > > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > > > > is no longer able to correctly track inode.i_version due to the struct
> > > > > > kstat.change_cookie no longer containing an updated i_version.
> > > > > >
> > > > > > Introduce a fallback mechanism for IMA that instead tracks a
> > > > > > integrity_ctime_guard() in absence of or outdated i_version
> > > > > > for stacked file systems.
> > > > > >
> > > > > > EVM is left alone since it mostly cares about the backing inode.
> > > > > >
> > > > > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > > > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > > > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > > > ---
> > > > > > The motivation behind this was that file systems that use the
> > > > > > cookie to set the i_version for stacked file systems may still do so.
> > > > > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > > > > The assumption is that the ctime will be different if the i_version is
> > > > > > different anyway for non-stacked file systems.
> > > > > >
> > > > > > I'm not too pleased with passing in struct file* to
> > > > > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > > > > that for now, but I couldn't come up with another idea to get the
> > > > > > stat without coming up with a new stat function to accommodate just
> > > > > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > > > > file system support to EVM (which doesn't make much sense to me
> > > > > > at the moment).
> > > > > >
> > > > > > I plan on adding in self test infrastructure for the v1, but I would
> > > > > > like to get some early feedback on the approach first.
> > > > > > ---
> > > > > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > > > > security/integrity/evm/evm_crypto.c | 2 +-
> > > > > > security/integrity/evm/evm_main.c | 2 +-
> > > > > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > > > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > > > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > > > > >
> > > > > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > > > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > > > > --- a/include/linux/integrity.h
> > > > > > +++ b/include/linux/integrity.h
> > > > > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > > > > >
> > > > > > /* An inode's attributes for detection of changes */
> > > > > > struct integrity_inode_attributes {
> > > > > > + u64 ctime_guard;
> > > > > > u64 version; /* track inode changes */
> > > > > > unsigned long ino;
> > > > > > dev_t dev;
> > > > > > };
> > > > > >
> > > > > > +static inline u64 integrity_ctime_guard(struct kstat stat)
> > > > > > +{
> > > > > > + return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > > > > > +}
> > > > > > +
> > > > > > /*
> > > > > > * On stacked filesystems the i_version alone is not enough to detect file data
> > > > > > * or metadata change. Additional metadata is required.
> > > > > > */
> > > > > > static inline void
> > > > > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > > - u64 i_version, const struct inode *inode)
> > > > > > + u64 i_version, u64 ctime_guard,
> > > > > > + const struct inode *inode)
> > > > > > {
> > > > > > + attrs->ctime_guard = ctime_guard;
> > > > > > attrs->version = i_version;
> > > > > > attrs->dev = inode->i_sb->s_dev;
> > > > > > attrs->ino = inode->i_ino;
> > > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > > */
> > > > > > static inline bool
> > > > > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > > > > - const struct inode *inode)
> > > > > > + struct file *file, struct inode *inode)
> > > > > > {
> > > > > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > > > > - inode->i_ino != attrs->ino ||
> > > > > > - !inode_eq_iversion(inode, attrs->version));
> > > > > > + struct kstat stat;
> > > > > > +
> > > > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > > > + inode->i_ino != attrs->ino)
> > > > > > + return true;
> > > > > > +
> > > > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > > > + return false;
> > > > > > +
> > > > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > > > + AT_STATX_SYNC_AS_STAT))
> > > > > > + return true;
> > > > > > +
> > > > >
> > > > > This is rather odd. You're sampling the i_version field directly, but
> > > > > if it's not equal then you go through ->getattr() to get the ctime.
> > > > >
> > > > > It's particularly odd since you don't know whether the i_version field
> > > > > is even implemented on the fs. On filesystems where it isn't, the
> > > > > i_version field generally stays at 0, so won't this never fall through
> > > > > to do the vfs_getattr_nosec() call on those filesystems?
> > > > >
> > > >
> > > > You're totally right. I didn't consider FS's caching the value at zero.
> > >
> > > Actually, I'm going to amend this. I think I did consider FSs without an
> > > implementation. Where this is called at, it is often guarded by a
> > > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> > > understanding this correctly, the check call doesn't occur unless the inode
> > > has i_version support.
> > >
> >
> >
> > It depends on what you mean by i_version support:
> >
> > That flag just tells the VFS that it needs to bump the i_version field
> > when updating timestamps. It's not a reliable indicator of whether the
> > i_version field is suitable for the purpose you want here.
> >
>
> So, it would make sense then to also remove those guards at the callsite
> then if the intention is to compare against the cookie & ctime regardless?
>
Yes, I'd say so. IS_I_VERSION() is not a reliable indicator of whether
the filesystem can provide a proper change attribute. For instance,
cephfs and NFS don't set that flag, but they can (sometimes) provide
STATX_CHANGE_COOKIE. Going through getattr() is a much more reliable
method.
> > The problem here and the one that we ultimately fixed with multigrain
> > timestamps is that XFS in particular will bump i_version on any change
> > to the log. That includes atime updates due to reads.
> >
> > XFS still tracks the i_version the way it always has, but we've stopped
> > getattr() from reporting it because it's not suitable for the purpose
> > that nfsd (and IMA) need it for.
> >
> > > It seems to me the suggestion then is to remove the IS_I_VERSION()
> > > checks guarding the call sites, grab both ctime and cookie from stat,
> > > and if IS_I_VERSION() use that, otherwise cookie, and compare
> > > against the cached i_version with one of those values, and then fall
> > > back to ctime?
> > >
> >
> > Not exactly.
> >
> > You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
> > then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
> > use that. If it's not then use the ctime.
>
> Ok, I think I understand. To reiterate my understanding, ignore calling
> into inode_eq_iversion() all together.
>
> return other_checks || ((mask & cookie) ? cache->i_version == cookie_val :
> compare_ctime())
>
Yes, that psuedocode looks about right. IS_I_VERSION(),
inode_eq_iversion(), etc. are really filesystem-internal APIs and not
something IMA should be relying on as a fstype-agnostic LSM.
> >
> > The part I'm not sure about is whether it's actually safe to do this.
> > vfs_getattr_nosec() can block in some situations. Is it ok to do this
> > in any context where integrity_inode_attrs_changed() may be called?
> >
> > ISTR that this was an issue at one point, but maybe isn't now that IMA
> > is an LSM?
>
> Poking around, callers to integrity_inode_attrs_changed() are currently behind
> mutex_lock(&iinit->mutex) (similar to vfs_getattr_nosec() calls), and currently
> only called from process_measurement().
>
> While I can't say for certain this will always be the case for future use
> use cases, would it be helpful to include a may_sleep() in
> integrity_inode_attrs_changed() to drive this point and make a comment?
>
Those kind of annotations do tend to be useful.
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Frederick Lawler @ 2026-01-07 19:56 UTC (permalink / raw)
To: Jeff Layton
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <25b6d1b42ea07b058be4e4f48bb5a7c6b879b3ed.camel@kernel.org>
On Tue, Jan 06, 2026 at 02:50:31PM -0500, Jeff Layton wrote:
> On Tue, 2026-01-06 at 13:33 -0600, Frederick Lawler wrote:
> > On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> > > Hi Jeff,
> > >
> > > On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > > > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > > > is no longer able to correctly track inode.i_version due to the struct
> > > > > kstat.change_cookie no longer containing an updated i_version.
> > > > >
> > > > > Introduce a fallback mechanism for IMA that instead tracks a
> > > > > integrity_ctime_guard() in absence of or outdated i_version
> > > > > for stacked file systems.
> > > > >
> > > > > EVM is left alone since it mostly cares about the backing inode.
> > > > >
> > > > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > > ---
> > > > > The motivation behind this was that file systems that use the
> > > > > cookie to set the i_version for stacked file systems may still do so.
> > > > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > > > The assumption is that the ctime will be different if the i_version is
> > > > > different anyway for non-stacked file systems.
> > > > >
> > > > > I'm not too pleased with passing in struct file* to
> > > > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > > > that for now, but I couldn't come up with another idea to get the
> > > > > stat without coming up with a new stat function to accommodate just
> > > > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > > > file system support to EVM (which doesn't make much sense to me
> > > > > at the moment).
> > > > >
> > > > > I plan on adding in self test infrastructure for the v1, but I would
> > > > > like to get some early feedback on the approach first.
> > > > > ---
> > > > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > > > security/integrity/evm/evm_crypto.c | 2 +-
> > > > > security/integrity/evm/evm_main.c | 2 +-
> > > > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > > > >
> > > > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > > > --- a/include/linux/integrity.h
> > > > > +++ b/include/linux/integrity.h
> > > > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > > > >
> > > > > /* An inode's attributes for detection of changes */
> > > > > struct integrity_inode_attributes {
> > > > > + u64 ctime_guard;
> > > > > u64 version; /* track inode changes */
> > > > > unsigned long ino;
> > > > > dev_t dev;
> > > > > };
> > > > >
> > > > > +static inline u64 integrity_ctime_guard(struct kstat stat)
> > > > > +{
> > > > > + return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > > > > +}
> > > > > +
> > > > > /*
> > > > > * On stacked filesystems the i_version alone is not enough to detect file data
> > > > > * or metadata change. Additional metadata is required.
> > > > > */
> > > > > static inline void
> > > > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > - u64 i_version, const struct inode *inode)
> > > > > + u64 i_version, u64 ctime_guard,
> > > > > + const struct inode *inode)
> > > > > {
> > > > > + attrs->ctime_guard = ctime_guard;
> > > > > attrs->version = i_version;
> > > > > attrs->dev = inode->i_sb->s_dev;
> > > > > attrs->ino = inode->i_ino;
> > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > */
> > > > > static inline bool
> > > > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > > > - const struct inode *inode)
> > > > > + struct file *file, struct inode *inode)
> > > > > {
> > > > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > > > - inode->i_ino != attrs->ino ||
> > > > > - !inode_eq_iversion(inode, attrs->version));
> > > > > + struct kstat stat;
> > > > > +
> > > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > > + inode->i_ino != attrs->ino)
> > > > > + return true;
> > > > > +
> > > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > > + return false;
> > > > > +
> > > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > > + AT_STATX_SYNC_AS_STAT))
> > > > > + return true;
> > > > > +
> > > >
> > > > This is rather odd. You're sampling the i_version field directly, but
> > > > if it's not equal then you go through ->getattr() to get the ctime.
> > > >
> > > > It's particularly odd since you don't know whether the i_version field
> > > > is even implemented on the fs. On filesystems where it isn't, the
> > > > i_version field generally stays at 0, so won't this never fall through
> > > > to do the vfs_getattr_nosec() call on those filesystems?
> > > >
> > >
> > > You're totally right. I didn't consider FS's caching the value at zero.
> >
> > Actually, I'm going to amend this. I think I did consider FSs without an
> > implementation. Where this is called at, it is often guarded by a
> > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> > understanding this correctly, the check call doesn't occur unless the inode
> > has i_version support.
> >
>
>
> It depends on what you mean by i_version support:
>
> That flag just tells the VFS that it needs to bump the i_version field
> when updating timestamps. It's not a reliable indicator of whether the
> i_version field is suitable for the purpose you want here.
>
So, it would make sense then to also remove those guards at the callsite
then if the intention is to compare against the cookie & ctime regardless?
> The problem here and the one that we ultimately fixed with multigrain
> timestamps is that XFS in particular will bump i_version on any change
> to the log. That includes atime updates due to reads.
>
> XFS still tracks the i_version the way it always has, but we've stopped
> getattr() from reporting it because it's not suitable for the purpose
> that nfsd (and IMA) need it for.
>
> > It seems to me the suggestion then is to remove the IS_I_VERSION()
> > checks guarding the call sites, grab both ctime and cookie from stat,
> > and if IS_I_VERSION() use that, otherwise cookie, and compare
> > against the cached i_version with one of those values, and then fall
> > back to ctime?
> >
>
> Not exactly.
>
> You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
> then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
> use that. If it's not then use the ctime.
Ok, I think I understand. To reiterate my understanding, ignore calling
into inode_eq_iversion() all together.
return other_checks || ((mask & cookie) ? cache->i_version == cookie_val :
compare_ctime())
>
> The part I'm not sure about is whether it's actually safe to do this.
> vfs_getattr_nosec() can block in some situations. Is it ok to do this
> in any context where integrity_inode_attrs_changed() may be called?
>
> ISTR that this was an issue at one point, but maybe isn't now that IMA
> is an LSM?
Poking around, callers to integrity_inode_attrs_changed() are currently behind
mutex_lock(&iinit->mutex) (similar to vfs_getattr_nosec() calls), and currently
only called from process_measurement().
While I can't say for certain this will always be the case for future use
use cases, would it be helpful to include a may_sleep() in
integrity_inode_attrs_changed() to drive this point and make a comment?
>
> > >
> > > > Ideally, you should just call vfs_getattr_nosec() early on with
> > > > STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
> > > > STATX_CHANGE_COOKIE if it's set in the returned mask.
> > > >
> > >
> > > Yes, that makes sense.
> > >
> > > I'll spin that in v1, thanks!
> > >
> > > > > + return attrs->ctime_guard != integrity_ctime_guard(stat);
> > > > > }
> > > > >
> > > > >
> > > > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> > > > > --- a/security/integrity/evm/evm_crypto.c
> > > > > +++ b/security/integrity/evm/evm_crypto.c
> > > > > @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > > > if (IS_I_VERSION(inode))
> > > > > i_version = inode_query_iversion(inode);
> > > > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > > > - inode);
> > > > > + 0, inode);
> > > > > }
> > > > >
> > > > > /* Portable EVM signatures must include an IMA hash */
> > > > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > > > > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..0712802628fd6533383f9855687e19bef7b771c7 100644
> > > > > --- a/security/integrity/evm/evm_main.c
> > > > > +++ b/security/integrity/evm/evm_main.c
> > > > > @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > > > if (iint) {
> > > > > ret = (!IS_I_VERSION(metadata_inode) ||
> > > > > integrity_inode_attrs_changed(&iint->metadata_inode,
> > > > > - metadata_inode));
> > > > > + NULL, metadata_inode));
> > > > > if (ret)
> > > > > iint->evm_status = INTEGRITY_UNKNOWN;
> > > > > }
> > > > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > > > > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..72bba8daa588a0f4e45e4249276edb54ca3d77ef 100644
> > > > > --- a/security/integrity/ima/ima_api.c
> > > > > +++ b/security/integrity/ima/ima_api.c
> > > > > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > > > int length;
> > > > > void *tmpbuf;
> > > > > u64 i_version = 0;
> > > > > + u64 ctime_guard = 0;
> > > > >
> > > > > /*
> > > > > * Always collect the modsig, because IMA might have already collected
> > > > > @@ -272,10 +273,16 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > > > * to an initial measurement/appraisal/audit, but was modified to
> > > > > * assume the file changed.
> > > > > */
> > > > > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > > > > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > > > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > > > AT_STATX_SYNC_AS_STAT);
> > > > > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > > > > - i_version = stat.change_cookie;
> > > > > + if (!result) {
> > > > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > > > + i_version = stat.change_cookie;
> > > > > +
> > > > > + if (stat.result_mask & STATX_CTIME)
> > > > > + ctime_guard = integrity_ctime_guard(stat);
> > > > > + }
> > > > > hash.hdr.algo = algo;
> > > > > hash.hdr.length = hash_digest_size[algo];
> > > > >
> > > > > @@ -305,11 +312,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > > >
> > > > > iint->ima_hash = tmpbuf;
> > > > > memcpy(iint->ima_hash, &hash, length);
> > > > > - if (real_inode == inode)
> > > > > + if (real_inode == inode) {
> > > > > iint->real_inode.version = i_version;
> > > > > - else
> > > > > + iint->real_inode.ctime_guard = ctime_guard;
> > > > > + } else {
> > > > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > > > - real_inode);
> > > > > + ctime_guard, real_inode);
> > > > > + }
> > > > >
> > > > > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > > > > if (!result)
> > > > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > > > index 5770cf691912aa912fc65280c59f5baac35dd725..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 100644
> > > > > --- a/security/integrity/ima/ima_main.c
> > > > > +++ b/security/integrity/ima/ima_main.c
> > > > > @@ -22,6 +22,7 @@
> > > > > #include <linux/mount.h>
> > > > > #include <linux/mman.h>
> > > > > #include <linux/slab.h>
> > > > > +#include <linux/stat.h>
> > > > > #include <linux/xattr.h>
> > > > > #include <linux/ima.h>
> > > > > #include <linux/fs.h>
> > > > > @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > > > {
> > > > > fmode_t mode = file->f_mode;
> > > > > bool update;
> > > > > + int ret;
> > > > >
> > > > > if (!(mode & FMODE_WRITE))
> > > > > return;
> > > > > @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > > >
> > > > > update = test_and_clear_bit(IMA_UPDATE_XATTR,
> > > > > &iint->atomic_flags);
> > > > > - if ((iint->flags & IMA_NEW_FILE) ||
> > > > > - vfs_getattr_nosec(&file->f_path, &stat,
> > > > > - STATX_CHANGE_COOKIE,
> > > > > - AT_STATX_SYNC_AS_STAT) ||
> > > > > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > > > > - stat.change_cookie != iint->real_inode.version) {
> > > > > + ret = vfs_getattr_nosec(&file->f_path, &stat,
> > > > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > > > + AT_STATX_SYNC_AS_STAT);
> > > > > + if ((iint->flags & IMA_NEW_FILE) || ret ||
> > > > > + (!ret && stat.change_cookie != iint->real_inode.version) ||
> > > > > + (!ret && integrity_ctime_guard(stat) !=
> > > > > + iint->real_inode.ctime_guard)) {
> > > > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > > > iint->measured_pcrs = 0;
> > > > > if (update)
> > > > > @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > > > if (!IS_I_VERSION(real_inode) ||
> > > > > integrity_inode_attrs_changed(&iint->real_inode,
> > > > > - real_inode)) {
> > > > > + file, real_inode)) {
> > > > > iint->flags &= ~IMA_DONE_MASK;
> > > > > iint->measured_pcrs = 0;
> > > > > }
> > > > >
> > > > > ---
> > > > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > > > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> > > > >
> > > > > Best regards,
> > > >
> > > > --
> > > > Jeff Layton <jlayton@kernel.org>
>
> --
> Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/2] ima_kexec.sh: Detect kernel image
From: Petr Vorel @ 2026-01-07 16:20 UTC (permalink / raw)
To: ltp; +Cc: Mimi Zohar, linux-integrity
In-Reply-To: <20260107155737.791588-1-pvorel@suse.cz>
Hi all,
...
> + if [ ! -f "$IMA_KEXEC_IMAGE" ]; then
> + uname="$(uname -r)"
> +
> + # x86_64
> + f="/boot/vmlinuz-$uname"
> +
> + # ppc64le, s390x
> + if [ ! -f "$f" ]; then
> + f="/boot/vmlinux-$uname"
> + fi
> +
> + # aarch64
> + if [ ! -f "$f" ]; then
> + f="/boot/Image-$uname"
> + fi
> +
> + # aarch64 often uses compression
> + if [ ! -f "$f" ]; then
> + f="$(ls /boot/Image-$uname.* || true)"
> + fi
> +
> + if [ -f "$f" ]; then
> + IMA_KEXEC_IMAGE="$f"
> + fi
> + fi
> +
> if [ ! -f "$IMA_KEXEC_IMAGE" ]; then
> tst_brk TCONF "kernel image not found, specify path in \$IMA_KEXEC_IMAGE"
> fi
I'm sorry for the noise, I found our s390x emulation actually uses
/boot/image-$uname. I suggest in the end to merge with following diff.
Kind regards,
Petr
+++ testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
@@ -69,18 +69,16 @@ setup()
if [ ! -f "$IMA_KEXEC_IMAGE" ]; then
uname="$(uname -r)"
- # x86_64
- f="/boot/vmlinuz-$uname"
-
- # ppc64le, s390x
- if [ ! -f "$f" ]; then
- f="/boot/vmlinux-$uname"
- fi
-
- # aarch64
- if [ ! -f "$f" ]; then
- f="/boot/Image-$uname"
- fi
+ for f in \
+ /boot/vmlinuz-$uname \
+ /boot/vmlinux-$uname \
+ /boot/Image-$uname \
+ /boot/image-$uname \
+ ; do
+ if [ -f "$f" ]; then
+ break
+ fi
+ done
# aarch64 often uses compression
if [ ! -f "$f" ]; then
^ permalink raw reply
* [PATCH] tpm: Cap the number of PCR banks
From: Jarkko Sakkinen @ 2026-01-07 16:15 UTC (permalink / raw)
To: stable
Cc: linux-integrity, Jarkko Sakkinen, Lai Yi, Jonathan McDowell,
Roberto Sassu
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
[ Upstream commit faf07e611dfa464b201223a7253e9dc5ee0f3c9e ]
tpm2_get_pcr_allocation() does not cap any upper limit for the number of
banks. Cap the limit to eight banks so that out of bounds values coming
from external I/O cause on only limited harm.
Cc: stable@vger.kernel.org # v5.10+
Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array")
Tested-by: Lai Yi <yi1.lai@linux.intel.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
Backport for v5.10, v5.15, v6.1 and v6.6
drivers/char/tpm/tpm-chip.c | 1 -
drivers/char/tpm/tpm1-cmd.c | 5 -----
drivers/char/tpm/tpm2-cmd.c | 8 +++-----
include/linux/tpm.h | 8 +++++---
4 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 70e3fe20fdcf..458a3e9ea73a 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -279,7 +279,6 @@ static void tpm_dev_release(struct device *dev)
kfree(chip->work_space.context_buf);
kfree(chip->work_space.session_buf);
- kfree(chip->allocated_banks);
kfree(chip);
}
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index cf64c7385105..b49a790f1bd5 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -799,11 +799,6 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
*/
int tpm1_get_pcr_allocation(struct tpm_chip *chip)
{
- chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
- GFP_KERNEL);
- if (!chip->allocated_banks)
- return -ENOMEM;
-
chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 93545be190a5..57bb3e34b770 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -574,11 +574,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
nr_possible_banks = be32_to_cpup(
(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
-
- chip->allocated_banks = kcalloc(nr_possible_banks,
- sizeof(*chip->allocated_banks),
- GFP_KERNEL);
- if (!chip->allocated_banks) {
+ if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
+ pr_err("tpm: out of bank capacity: %u > %u\n",
+ nr_possible_banks, TPM2_MAX_PCR_BANKS);
rc = -ENOMEM;
goto out;
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index bf8a4ec8a01c..f5e7cca2f257 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -25,7 +25,9 @@
#include <crypto/hash_info.h>
#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
-#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
+
+#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
+#define TPM2_MAX_PCR_BANKS 8
struct tpm_chip;
struct trusted_key_payload;
@@ -51,7 +53,7 @@ enum tpm_algorithms {
struct tpm_digest {
u16 alg_id;
- u8 digest[TPM_MAX_DIGEST_SIZE];
+ u8 digest[TPM2_MAX_DIGEST_SIZE];
} __packed;
struct tpm_bank_info {
@@ -157,7 +159,7 @@ struct tpm_chip {
unsigned int groups_cnt;
u32 nr_allocated_banks;
- struct tpm_bank_info *allocated_banks;
+ struct tpm_bank_info allocated_banks[TPM2_MAX_PCR_BANKS];
#ifdef CONFIG_ACPI
acpi_handle acpi_dev_handle;
char ppi_version[TPM_PPI_VERSION_LEN + 1];
--
2.52.0
^ permalink raw reply related
* [PATCH 1/2] ima_kexec.sh: Detect kernel image
From: Petr Vorel @ 2026-01-07 15:57 UTC (permalink / raw)
To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity
Sometimes BOOT_IMAGE contains partition which does not point to /boot
e.g. BOOT_IMAGE=(hd0,gpt1)/opensuse-tumbleweed/6.18.3-1-default/linux-30afdbce3ab6d0eff8f42b71df1a66f4baf2daf8
on Tumbleweed aarch64. Therefore detect common kernel image paths.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
.../security/integrity/ima/tests/ima_kexec.sh | 28 ++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh b/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
index d6eb0829d8..7688690af2 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
@@ -42,7 +42,7 @@ measure()
setup()
{
- local arch
+ local arch f uname
if [ ! -f "$IMA_KEXEC_IMAGE" ]; then
for arg in $(cat /proc/cmdline); do
@@ -63,6 +63,32 @@ setup()
fi
fi
+ if [ ! -f "$IMA_KEXEC_IMAGE" ]; then
+ uname="$(uname -r)"
+
+ # x86_64
+ f="/boot/vmlinuz-$uname"
+
+ # ppc64le, s390x
+ if [ ! -f "$f" ]; then
+ f="/boot/vmlinux-$uname"
+ fi
+
+ # aarch64
+ if [ ! -f "$f" ]; then
+ f="/boot/Image-$uname"
+ fi
+
+ # aarch64 often uses compression
+ if [ ! -f "$f" ]; then
+ f="$(ls /boot/Image-$uname.* || true)"
+ fi
+
+ if [ -f "$f" ]; then
+ IMA_KEXEC_IMAGE="$f"
+ fi
+ fi
+
if [ ! -f "$IMA_KEXEC_IMAGE" ]; then
tst_brk TCONF "kernel image not found, specify path in \$IMA_KEXEC_IMAGE"
fi
--
2.51.0
^ permalink raw reply related
* [PATCH 2/2] ima_kexec.sh: Document kernel config dependencies
From: Petr Vorel @ 2026-01-07 15:57 UTC (permalink / raw)
To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity
In-Reply-To: <20260107155737.791588-1-pvorel@suse.cz>
CONFIG_HAVE_IMA_KEXEC=y is enough for test, ie. test is working with:
# CONFIG_IMA_KEXEC is not set
CONFIG_HAVE_IMA_KEXEC=y
Probably obvious as CONFIG_HAVE_IMA_KEXEC is arch specific and
CONFIG_IMA_KEXEC is "TPM PCRs are only reset on a hard reboot."
and ima_kexec.c requires CONFIG_HAVE_IMA_KEXEC (only parts are skipped
when CONFIG_IMA_KEXEC not set) but better to clarify for users.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
testcases/kernel/security/integrity/ima/tests/ima_kexec.sh | 3 +++
1 file changed, 3 insertions(+)
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh b/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
index 7688690af2..de595fcdd7 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_kexec.sh
@@ -6,8 +6,11 @@
#
# Verify that kexec cmdline is measured correctly.
# Test attempts to kexec the existing running kernel image.
+#
# To kexec a different kernel image export IMA_KEXEC_IMAGE=<pathname>.
# Test requires example IMA policy loadable with LTP_IMA_LOAD_POLICY=1.
+#
+# Test requires CONFIG_HAVE_IMA_KEXEC=y (CONFIG_IMA_KEXEC is not mandatory).
TST_NEEDS_CMDS="grep kexec sed"
TST_CNT=3
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v3 2/3] ima: trim N IMA event log records
From: Roberto Sassu @ 2026-01-07 10:06 UTC (permalink / raw)
To: steven chen, linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260106020713.3994-3-chenste@linux.microsoft.com>
On Mon, 2026-01-05 at 18:07 -0800, steven chen wrote:
> Trim N entries of the IMA event logs. Clean the hash table if
> ima_flush_htable is set.
>
> Provide 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 time.
>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
> .../admin-guide/kernel-parameters.txt | 4 +
> security/integrity/ima/ima.h | 2 +
> security/integrity/ima/ima_fs.c | 164 +++++++++++++++++-
> security/integrity/ima/ima_queue.c | 85 +++++++++
> 4 files changed, 251 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e92c0056e4e0..cd1a1d0bf0e2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2197,6 +2197,10 @@
> Use the canonical format for the binary runtime
> measurements, instead of host native format.
>
> + ima_flush_htable [IMA]
> + Flush the measurement list hash table when trim all
> + or a part of it for deletion.
> +
> ima_hash= [IMA]
> Format: { md5 | sha1 | rmd160 | sha256 | sha384
> | sha512 | ... }
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index e3d71d8d56e3..2102c523dca0 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_delete_event_log(long req_val);
> #else
> static inline void ima_measure_kexec_event(const char *event_name) {}
> +static inline long ima_delete_event_log(long req_val) { return 0; }
> #endif
>
> /*
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 87045b09f120..67ff0cfc3d3f 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -21,6 +21,9 @@
> #include <linux/rcupdate.h>
> #include <linux/parser.h>
> #include <linux/vmalloc.h>
> +#include <linux/ktime.h>
> +#include <linux/timekeeping.h>
> +#include <linux/ima.h>
>
> #include "ima.h"
>
> @@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>
> static int valid_policy = 1;
>
> +#define IMA_LOG_TRIM_REQ_LENGTH 11
> +#define IMA_LOG_TRIM_EVENT_LEN 256
Shouldn't this belong to the next patch?
> +
> +static long trimcount;
> +/* mutex protects atomicity of trimming measurement list
> + * and also protects atomicity the measurement list read
> + * write operation.
> + */
> +static DEFINE_MUTEX(ima_measure_lock);
> +static long ima_measure_users;
> +
> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> loff_t *ppos, atomic_long_t *val)
> {
> @@ -202,16 +216,77 @@ static const struct seq_operations ima_measurments_seqops = {
> .show = ima_measurements_show
> };
>
> +/*
> + * _ima_measurements_open - open the IMA measurements file
> + * @inode: inode of the file being opened
> + * @file: file being opened
> + * @seq_ops: sequence operations for the file
> + *
> + * Returns 0 on success, or negative error code.
> + * Implements mutual exclusion between readers and writer
> + * of the measurements file. Multiple readers are allowed,
> + * but writer get exclusive access only no other readers/writers.
> + * Readers is not allowed when there is a writer.
> + */
> +static int _ima_measurements_open(struct inode *inode, struct file *file,
> + const struct seq_operations *seq_ops)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + if (write && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + mutex_lock(&ima_measure_lock);
> + if ((write && ima_measure_users != 0) ||
> + (!write && ima_measure_users < 0)) {
> + mutex_unlock(&ima_measure_lock);
> + return -EBUSY;
> + }
> +
> + ret = seq_open(file, seq_ops);
> + if (ret < 0) {
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> + }
> +
> + if (write)
> + ima_measure_users--;
> + else
> + ima_measure_users++;
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> +}
> +
> static int ima_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_measurments_seqops);
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static int ima_measurements_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + mutex_lock(&ima_measure_lock);
> + ret = seq_release(inode, file);
> + if (!ret) {
> + if (write)
> + ima_measure_users++;
> + else
> + ima_measure_users--;
> + }
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> }
>
> static const struct file_operations ima_measurements_ops = {
> .open = ima_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> };
>
> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
> @@ -279,14 +354,83 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>
> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_ascii_measurements_seqops);
> + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
> }
>
> static const struct file_operations ima_ascii_measurements_ops = {
> .open = ima_ascii_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> +};
> +
> +static int ima_log_trim_open(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> +
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
> +{
> + char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
> + ssize_t len;
> +
> + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
> + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
> +}
> +
> +static ssize_t ima_log_trim_write(struct file *file,
> + const char __user *buf, size_t datalen, loff_t *ppos)
> +{
> + long count, n, ret;
> +
> + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + n = (int)datalen;
> +
> + ret = kstrtol_from_user(buf, n, 10, &count);
> + if (ret < 0)
> + goto out;
> +
> + ret = ima_delete_event_log(count);
> +
> + if (ret < 0)
> + goto out;
> +
> + trimcount = ret;
> +
> + ret = datalen;
> +out:
> + return ret;
> +}
> +
> +static int ima_log_trim_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> +
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return ima_measurements_release(inode, file);
> +}
> +
> +static const struct file_operations ima_log_trim_ops = {
> + .open = ima_log_trim_open,
> + .read = ima_log_trim_read,
> + .write = ima_log_trim_write,
> + .llseek = generic_file_llseek,
> + .release = ima_log_trim_release
> };
>
> static ssize_t ima_read_policy(char *path)
> @@ -528,6 +672,18 @@ int __init ima_fs_init(void)
> goto out;
> }
>
> + if (IS_ENABLED(CONFIG_IMA_LOG_TRIMMING)) {
> + 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..33bb5414b8cc 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -22,6 +22,14 @@
>
> #define AUDIT_CAUSE_LEN_MAX 32
>
> +bool ima_flush_htable;
> +static int __init ima_flush_htable_setup(char *str)
> +{
> + ima_flush_htable = true;
> + return 1;
> +}
> +__setup("ima_flush_htable", ima_flush_htable_setup);
> +
> /* pre-allocated array of tpm_digest structures to extend a PCR */
> static struct tpm_digest *digests;
>
> @@ -220,6 +228,83 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> return result;
> }
>
> +/**
> + * ima_delete_event_log - delete IMA event entry
> + * @num_records: number of records to delete
> + *
> + * delete num_records entries off the measurement list.
> + * Returns the number of entries deleted, or negative error code.
This is not according to the format stated in the documentation.
> + */
> +long ima_delete_event_log(long num_records)
> +{
> + long len, cur = num_records, tmp_len = 0;
> + struct ima_queue_entry *qe, *qe_tmp;
> + LIST_HEAD(ima_measurements_staged);
> + struct list_head *list_ptr;
> +
> + if (num_records <= 0)
> + return num_records;
> +
> + if (!IS_ENABLED(CONFIG_IMA_LOG_TRIMMING))
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&ima_extend_list_mutex);
> + len = atomic_long_read(&ima_htable.len);
> +
> + if (num_records > len) {
> + mutex_unlock(&ima_extend_list_mutex);
> + return -ENOENT;
> + }
> +
> + list_ptr = &ima_measurements;
> +
> + if (cur == len) {
> + list_replace(&ima_measurements, &ima_measurements_staged);
> + INIT_LIST_HEAD(&ima_measurements);
> + atomic_long_set(&ima_htable.len, 0);
> + list_ptr = &ima_measurements_staged;
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size = 0;
Like in my patch, we should have kept the original value of
binary_runtime_size, to avoid breaking the kexec critical data records.
> + }
> +
> + list_for_each_entry(qe, list_ptr, later) {
> + if (num_records > 0) {
> + if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable)
> + hlist_del_rcu(&qe->hnext);
> +
> + --num_records;
> + if (num_records == 0)
> + qe_tmp = qe;
> + continue;
> + }
> + if (len != cur && IS_ENABLED(CONFIG_IMA_KEXEC))
> + tmp_len += get_binary_runtime_size(qe->entry);
> + else
> + break;
> + }
> +
> + if (len != cur) {
> + __list_cut_position(&ima_measurements_staged, &ima_measurements,
> + &qe_tmp->later);
> + atomic_long_sub(cur, &ima_htable.len);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size = tmp_len;
> + }
> +
> + mutex_unlock(&ima_extend_list_mutex);
> +
> + if (ima_flush_htable)
> + synchronize_rcu();
> +
> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
> + ima_free_template_entry(qe->entry);
> + list_del(&qe->later);
> + kfree(qe);
If you don't flush the hash table, you cannot delete the entry.
Roberto
> + }
> +
> + return cur;
> +}
> +
> int ima_restore_measurement_entry(struct ima_template_entry *entry)
> {
> int result = 0;
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jens Wiklander @ 2026-01-07 9:38 UTC (permalink / raw)
To: Sudeep Holla
Cc: Alexandre Belloni, Uwe Kleine-König, Jonathan Corbet,
Sumit Garg, Olivia Mackall, Herbert Xu, Clément Léger,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <aV0Qx5BOso5co3tm@bogus>
On Tue, Jan 6, 2026 at 2:40 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Mon, Jan 05, 2026 at 10:16:09AM +0100, Jens Wiklander wrote:
> > Hi,
> >
> > On Thu, Dec 18, 2025 at 5:29 PM Jens Wiklander
> > <jens.wiklander@linaro.org> wrote:
> > >
> > > On Thu, Dec 18, 2025 at 2:53 PM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > > >
> > > > On 18/12/2025 08:21:27+0100, Jens Wiklander wrote:
> > > > > Hi,
> > > > >
> > > > > On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
> > > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > the objective of this series is to make tee driver stop using callbacks
> > > > > > in struct device_driver. These were superseded by bus methods in 2006
> > > > > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > > > > methods.")) but nobody cared to convert all subsystems accordingly.
> > > > > >
> > > > > > Here the tee drivers are converted. The first commit is somewhat
> > > > > > unrelated, but simplifies the conversion (and the drivers). It
> > > > > > introduces driver registration helpers that care about setting the bus
> > > > > > and owner. (The latter is missing in all drivers, so by using these
> > > > > > helpers the drivers become more correct.)
> > > > > >
> > > > > > v1 of this series is available at
> > > > > > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> > > > > >
> > > > > > Changes since v1:
> > > > > >
> > > > > > - rebase to v6.19-rc1 (no conflicts)
> > > > > > - add tags received so far
> > > > > > - fix whitespace issues pointed out by Sumit Garg
> > > > > > - fix shutdown callback to shutdown and not remove
> > > > > >
> > > > > > As already noted in v1's cover letter, this series should go in during a
> > > > > > single merge window as there are runtime warnings when the series is
> > > > > > only applied partially. Sumit Garg suggested to apply the whole series
> > > > > > via Jens Wiklander's tree.
> > > > > > If this is done the dependencies in this series are honored, in case the
> > > > > > plan changes: Patches #4 - #17 depend on the first two.
> > > > > >
> > > > > > Note this series is only build tested.
> > > > > >
> > > > > > Uwe Kleine-König (17):
> > > > > > tee: Add some helpers to reduce boilerplate for tee client drivers
> > > > > > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > > > > > tee: Adapt documentation to cover recent additions
> > > > > > hwrng: optee - Make use of module_tee_client_driver()
> > > > > > hwrng: optee - Make use of tee bus methods
> > > > > > rtc: optee: Migrate to use tee specific driver registration function
> > > > > > rtc: optee: Make use of tee bus methods
> > > > > > efi: stmm: Make use of module_tee_client_driver()
> > > > > > efi: stmm: Make use of tee bus methods
> > > > > > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > > > > > firmware: arm_scmi: Make use of tee bus methods
> > > > > > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > > > > > firmware: tee_bnxt: Make use of tee bus methods
> > > > > > KEYS: trusted: Migrate to use tee specific driver registration
> > > > > > function
> > > > > > KEYS: trusted: Make use of tee bus methods
> > > > > > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > > > > > tpm/tpm_ftpm_tee: Make use of tee bus methods
> > > > > >
> > > > > > Documentation/driver-api/tee.rst | 18 +----
> > > > > > drivers/char/hw_random/optee-rng.c | 26 ++----
> > > > > > drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> > > > > > drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> > > > > > drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> > > > > > drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> > > > > > drivers/rtc/rtc-optee.c | 27 ++-----
> > > > > > drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> > > > > > include/linux/tee_drv.h | 12 +++
> > > > > > security/keys/trusted-keys/trusted_tee.c | 17 ++--
> > > > > > 10 files changed, 164 insertions(+), 138 deletions(-)
> > > > > >
> > > > > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > > > > --
> > > > > > 2.47.3
> > > > > >
> > > > >
> > > > > Thank you for the nice cleanup, Uwe.
> > > > >
> > > > > I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
> > > > > tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
> > > > >
> > > > > The branch is based on v6.19-rc1, and I'll try to keep it stable for
> > > > > others to depend on, if needed. Let's see if we can agree on taking
> > > > > the remaining patches via that branch.
> > > >
> > > > 6 and 7 can go through your branch.
> > >
> > > Good, I've added them to my branch now.
> >
> > This entire patch set should go in during a single merge window. I
> > will not send any pull request until I'm sure all patches will be
> > merged.
> >
> > So far (if I'm not mistaken), only the patches I've already added to
> > next have appeared next. I can take the rest of the patches, too, but
> > I need OK for the following:
> >
>
> [...]
>
> >
> > Sudeep, you seem happy with the following patches
> > - firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > - firmware: arm_scmi: Make use of tee bus methods
> > OK if I take them via my tree, or would you rather take them yourself?
> >
>
> I am happy if you want to take all of them in one go. I think I have
> already acked it. Please shout if you need anything else from me, happy to
> help in anyway to make it easier to handle this change set.
Thanks, I've applied all the patches in the series now, since it
otherwise causes warnings during boot.
/Jens
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jens Wiklander @ 2026-01-07 9:36 UTC (permalink / raw)
To: Jon Hunter
Cc: Uwe Kleine-König, Jonathan Corbet, Sumit Garg,
Olivia Mackall, Herbert Xu, Clément Léger,
Alexandre Belloni, Ard Biesheuvel, Maxime Coquelin,
Alexandre Torgue, Sumit Garg, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Rafał Miłecki,
Michael Chan, Pavan Chebbi, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-doc,
linux-crypto, linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, linux-mips, netdev, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe,
linux-tegra@vger.kernel.org
In-Reply-To: <d14a9c41-9df7-438f-bb58-097644d5d93f@nvidia.com>
Hi Jon,
On Tue, Jan 6, 2026 at 10:40 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
> Hi Uwe,
>
> On 15/12/2025 14:16, Uwe Kleine-König wrote:
> > Hello,
> >
> > the objective of this series is to make tee driver stop using callbacks
> > in struct device_driver. These were superseded by bus methods in 2006
> > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > methods.")) but nobody cared to convert all subsystems accordingly.
> >
> > Here the tee drivers are converted. The first commit is somewhat
> > unrelated, but simplifies the conversion (and the drivers). It
> > introduces driver registration helpers that care about setting the bus
> > and owner. (The latter is missing in all drivers, so by using these
> > helpers the drivers become more correct.)
> >
> > v1 of this series is available at
> > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> >
> > Changes since v1:
> >
> > - rebase to v6.19-rc1 (no conflicts)
> > - add tags received so far
> > - fix whitespace issues pointed out by Sumit Garg
> > - fix shutdown callback to shutdown and not remove
> >
> > As already noted in v1's cover letter, this series should go in during a
> > single merge window as there are runtime warnings when the series is
> > only applied partially. Sumit Garg suggested to apply the whole series
> > via Jens Wiklander's tree.
> > If this is done the dependencies in this series are honored, in case the
> > plan changes: Patches #4 - #17 depend on the first two.
> >
> > Note this series is only build tested.
> >
> > Uwe Kleine-König (17):
> > tee: Add some helpers to reduce boilerplate for tee client drivers
> > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > tee: Adapt documentation to cover recent additions
> > hwrng: optee - Make use of module_tee_client_driver()
> > hwrng: optee - Make use of tee bus methods
> > rtc: optee: Migrate to use tee specific driver registration function
> > rtc: optee: Make use of tee bus methods
> > efi: stmm: Make use of module_tee_client_driver()
> > efi: stmm: Make use of tee bus methods
> > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > firmware: arm_scmi: Make use of tee bus methods
> > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > firmware: tee_bnxt: Make use of tee bus methods
> > KEYS: trusted: Migrate to use tee specific driver registration
> > function
> > KEYS: trusted: Make use of tee bus methods
> > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > tpm/tpm_ftpm_tee: Make use of tee bus methods
>
>
> On the next-20260105 I am seeing the following warnings ...
>
> WARNING KERN Driver 'optee-rng' needs updating - please use bus_type methods
> WARNING KERN Driver 'scmi-optee' needs updating - please use bus_type methods
> WARNING KERN Driver 'tee_bnxt_fw' needs updating - please use bus_type methods
>
> I bisected the first warning and this point to the following
> commit ...
>
> # first bad commit: [a707eda330b932bcf698be9460e54e2f389e24b7] tee: Add some helpers to reduce boilerplate for tee client drivers
>
> I have not bisected the others, but guess they are related
> to this series. Do you observe the same?
Yes, I see the same.
I'm sorry, I didn't realize that someone might bisect this when I took
only a few of the patches into next. I've applied all the patches in
this series now.
Thanks,
Jens
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Jeff Layton @ 2026-01-06 19:50 UTC (permalink / raw)
To: Frederick Lawler
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <aV1jhIS24tE-dL9A@CMGLRV3>
On Tue, 2026-01-06 at 13:33 -0600, Frederick Lawler wrote:
> On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> > Hi Jeff,
> >
> > On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > > is no longer able to correctly track inode.i_version due to the struct
> > > > kstat.change_cookie no longer containing an updated i_version.
> > > >
> > > > Introduce a fallback mechanism for IMA that instead tracks a
> > > > integrity_ctime_guard() in absence of or outdated i_version
> > > > for stacked file systems.
> > > >
> > > > EVM is left alone since it mostly cares about the backing inode.
> > > >
> > > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > ---
> > > > The motivation behind this was that file systems that use the
> > > > cookie to set the i_version for stacked file systems may still do so.
> > > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > > The assumption is that the ctime will be different if the i_version is
> > > > different anyway for non-stacked file systems.
> > > >
> > > > I'm not too pleased with passing in struct file* to
> > > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > > that for now, but I couldn't come up with another idea to get the
> > > > stat without coming up with a new stat function to accommodate just
> > > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > > file system support to EVM (which doesn't make much sense to me
> > > > at the moment).
> > > >
> > > > I plan on adding in self test infrastructure for the v1, but I would
> > > > like to get some early feedback on the approach first.
> > > > ---
> > > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > > security/integrity/evm/evm_crypto.c | 2 +-
> > > > security/integrity/evm/evm_main.c | 2 +-
> > > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > > --- a/include/linux/integrity.h
> > > > +++ b/include/linux/integrity.h
> > > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > > >
> > > > /* An inode's attributes for detection of changes */
> > > > struct integrity_inode_attributes {
> > > > + u64 ctime_guard;
> > > > u64 version; /* track inode changes */
> > > > unsigned long ino;
> > > > dev_t dev;
> > > > };
> > > >
> > > > +static inline u64 integrity_ctime_guard(struct kstat stat)
> > > > +{
> > > > + return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > > > +}
> > > > +
> > > > /*
> > > > * On stacked filesystems the i_version alone is not enough to detect file data
> > > > * or metadata change. Additional metadata is required.
> > > > */
> > > > static inline void
> > > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > - u64 i_version, const struct inode *inode)
> > > > + u64 i_version, u64 ctime_guard,
> > > > + const struct inode *inode)
> > > > {
> > > > + attrs->ctime_guard = ctime_guard;
> > > > attrs->version = i_version;
> > > > attrs->dev = inode->i_sb->s_dev;
> > > > attrs->ino = inode->i_ino;
> > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > */
> > > > static inline bool
> > > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > > - const struct inode *inode)
> > > > + struct file *file, struct inode *inode)
> > > > {
> > > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > > - inode->i_ino != attrs->ino ||
> > > > - !inode_eq_iversion(inode, attrs->version));
> > > > + struct kstat stat;
> > > > +
> > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > + inode->i_ino != attrs->ino)
> > > > + return true;
> > > > +
> > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > + return false;
> > > > +
> > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > + AT_STATX_SYNC_AS_STAT))
> > > > + return true;
> > > > +
> > >
> > > This is rather odd. You're sampling the i_version field directly, but
> > > if it's not equal then you go through ->getattr() to get the ctime.
> > >
> > > It's particularly odd since you don't know whether the i_version field
> > > is even implemented on the fs. On filesystems where it isn't, the
> > > i_version field generally stays at 0, so won't this never fall through
> > > to do the vfs_getattr_nosec() call on those filesystems?
> > >
> >
> > You're totally right. I didn't consider FS's caching the value at zero.
>
> Actually, I'm going to amend this. I think I did consider FSs without an
> implementation. Where this is called at, it is often guarded by a
> !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> understanding this correctly, the check call doesn't occur unless the inode
> has i_version support.
>
It depends on what you mean by i_version support:
That flag just tells the VFS that it needs to bump the i_version field
when updating timestamps. It's not a reliable indicator of whether the
i_version field is suitable for the purpose you want here.
The problem here and the one that we ultimately fixed with multigrain
timestamps is that XFS in particular will bump i_version on any change
to the log. That includes atime updates due to reads.
XFS still tracks the i_version the way it always has, but we've stopped
getattr() from reporting it because it's not suitable for the purpose
that nfsd (and IMA) need it for.
> It seems to me the suggestion then is to remove the IS_I_VERSION()
> checks guarding the call sites, grab both ctime and cookie from stat,
> and if IS_I_VERSION() use that, otherwise cookie, and compare
> against the cached i_version with one of those values, and then fall
> back to ctime?
>
Not exactly.
You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
use that. If it's not then use the ctime.
The part I'm not sure about is whether it's actually safe to do this.
vfs_getattr_nosec() can block in some situations. Is it ok to do this
in any context where integrity_inode_attrs_changed() may be called?
ISTR that this was an issue at one point, but maybe isn't now that IMA
is an LSM?
> >
> > > Ideally, you should just call vfs_getattr_nosec() early on with
> > > STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
> > > STATX_CHANGE_COOKIE if it's set in the returned mask.
> > >
> >
> > Yes, that makes sense.
> >
> > I'll spin that in v1, thanks!
> >
> > > > + return attrs->ctime_guard != integrity_ctime_guard(stat);
> > > > }
> > > >
> > > >
> > > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> > > > --- a/security/integrity/evm/evm_crypto.c
> > > > +++ b/security/integrity/evm/evm_crypto.c
> > > > @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > > if (IS_I_VERSION(inode))
> > > > i_version = inode_query_iversion(inode);
> > > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > > - inode);
> > > > + 0, inode);
> > > > }
> > > >
> > > > /* Portable EVM signatures must include an IMA hash */
> > > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > > > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..0712802628fd6533383f9855687e19bef7b771c7 100644
> > > > --- a/security/integrity/evm/evm_main.c
> > > > +++ b/security/integrity/evm/evm_main.c
> > > > @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > > if (iint) {
> > > > ret = (!IS_I_VERSION(metadata_inode) ||
> > > > integrity_inode_attrs_changed(&iint->metadata_inode,
> > > > - metadata_inode));
> > > > + NULL, metadata_inode));
> > > > if (ret)
> > > > iint->evm_status = INTEGRITY_UNKNOWN;
> > > > }
> > > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > > > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..72bba8daa588a0f4e45e4249276edb54ca3d77ef 100644
> > > > --- a/security/integrity/ima/ima_api.c
> > > > +++ b/security/integrity/ima/ima_api.c
> > > > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > > int length;
> > > > void *tmpbuf;
> > > > u64 i_version = 0;
> > > > + u64 ctime_guard = 0;
> > > >
> > > > /*
> > > > * Always collect the modsig, because IMA might have already collected
> > > > @@ -272,10 +273,16 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > > * to an initial measurement/appraisal/audit, but was modified to
> > > > * assume the file changed.
> > > > */
> > > > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > > > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > > AT_STATX_SYNC_AS_STAT);
> > > > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > > > - i_version = stat.change_cookie;
> > > > + if (!result) {
> > > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > > + i_version = stat.change_cookie;
> > > > +
> > > > + if (stat.result_mask & STATX_CTIME)
> > > > + ctime_guard = integrity_ctime_guard(stat);
> > > > + }
> > > > hash.hdr.algo = algo;
> > > > hash.hdr.length = hash_digest_size[algo];
> > > >
> > > > @@ -305,11 +312,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > >
> > > > iint->ima_hash = tmpbuf;
> > > > memcpy(iint->ima_hash, &hash, length);
> > > > - if (real_inode == inode)
> > > > + if (real_inode == inode) {
> > > > iint->real_inode.version = i_version;
> > > > - else
> > > > + iint->real_inode.ctime_guard = ctime_guard;
> > > > + } else {
> > > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > > - real_inode);
> > > > + ctime_guard, real_inode);
> > > > + }
> > > >
> > > > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > > > if (!result)
> > > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > > index 5770cf691912aa912fc65280c59f5baac35dd725..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 100644
> > > > --- a/security/integrity/ima/ima_main.c
> > > > +++ b/security/integrity/ima/ima_main.c
> > > > @@ -22,6 +22,7 @@
> > > > #include <linux/mount.h>
> > > > #include <linux/mman.h>
> > > > #include <linux/slab.h>
> > > > +#include <linux/stat.h>
> > > > #include <linux/xattr.h>
> > > > #include <linux/ima.h>
> > > > #include <linux/fs.h>
> > > > @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > > {
> > > > fmode_t mode = file->f_mode;
> > > > bool update;
> > > > + int ret;
> > > >
> > > > if (!(mode & FMODE_WRITE))
> > > > return;
> > > > @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > >
> > > > update = test_and_clear_bit(IMA_UPDATE_XATTR,
> > > > &iint->atomic_flags);
> > > > - if ((iint->flags & IMA_NEW_FILE) ||
> > > > - vfs_getattr_nosec(&file->f_path, &stat,
> > > > - STATX_CHANGE_COOKIE,
> > > > - AT_STATX_SYNC_AS_STAT) ||
> > > > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > > > - stat.change_cookie != iint->real_inode.version) {
> > > > + ret = vfs_getattr_nosec(&file->f_path, &stat,
> > > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > > + AT_STATX_SYNC_AS_STAT);
> > > > + if ((iint->flags & IMA_NEW_FILE) || ret ||
> > > > + (!ret && stat.change_cookie != iint->real_inode.version) ||
> > > > + (!ret && integrity_ctime_guard(stat) !=
> > > > + iint->real_inode.ctime_guard)) {
> > > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > > iint->measured_pcrs = 0;
> > > > if (update)
> > > > @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > > if (!IS_I_VERSION(real_inode) ||
> > > > integrity_inode_attrs_changed(&iint->real_inode,
> > > > - real_inode)) {
> > > > + file, real_inode)) {
> > > > iint->flags &= ~IMA_DONE_MASK;
> > > > iint->measured_pcrs = 0;
> > > > }
> > > >
> > > > ---
> > > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> > > >
> > > > Best regards,
> > >
> > > --
> > > Jeff Layton <jlayton@kernel.org>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Frederick Lawler @ 2026-01-06 19:33 UTC (permalink / raw)
To: Jeff Layton
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <aV07lY6NOkNvUk3Z@CMGLRV3>
On Tue, Jan 06, 2026 at 10:43:01AM -0600, Frederick Lawler wrote:
> Hi Jeff,
>
> On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> > On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > > is no longer able to correctly track inode.i_version due to the struct
> > > kstat.change_cookie no longer containing an updated i_version.
> > >
> > > Introduce a fallback mechanism for IMA that instead tracks a
> > > integrity_ctime_guard() in absence of or outdated i_version
> > > for stacked file systems.
> > >
> > > EVM is left alone since it mostly cares about the backing inode.
> > >
> > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > ---
> > > The motivation behind this was that file systems that use the
> > > cookie to set the i_version for stacked file systems may still do so.
> > > Then add in the ctime_guard as a fallback if there's a detected change.
> > > The assumption is that the ctime will be different if the i_version is
> > > different anyway for non-stacked file systems.
> > >
> > > I'm not too pleased with passing in struct file* to
> > > integrity_inode_attrs_changed() since EVM doesn't currently use
> > > that for now, but I couldn't come up with another idea to get the
> > > stat without coming up with a new stat function to accommodate just
> > > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > > file system support to EVM (which doesn't make much sense to me
> > > at the moment).
> > >
> > > I plan on adding in self test infrastructure for the v1, but I would
> > > like to get some early feedback on the approach first.
> > > ---
> > > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > > security/integrity/evm/evm_crypto.c | 2 +-
> > > security/integrity/evm/evm_main.c | 2 +-
> > > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > > 5 files changed, 51 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > > --- a/include/linux/integrity.h
> > > +++ b/include/linux/integrity.h
> > > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> > >
> > > /* An inode's attributes for detection of changes */
> > > struct integrity_inode_attributes {
> > > + u64 ctime_guard;
> > > u64 version; /* track inode changes */
> > > unsigned long ino;
> > > dev_t dev;
> > > };
> > >
> > > +static inline u64 integrity_ctime_guard(struct kstat stat)
> > > +{
> > > + return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > > +}
> > > +
> > > /*
> > > * On stacked filesystems the i_version alone is not enough to detect file data
> > > * or metadata change. Additional metadata is required.
> > > */
> > > static inline void
> > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > - u64 i_version, const struct inode *inode)
> > > + u64 i_version, u64 ctime_guard,
> > > + const struct inode *inode)
> > > {
> > > + attrs->ctime_guard = ctime_guard;
> > > attrs->version = i_version;
> > > attrs->dev = inode->i_sb->s_dev;
> > > attrs->ino = inode->i_ino;
> > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > */
> > > static inline bool
> > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > - const struct inode *inode)
> > > + struct file *file, struct inode *inode)
> > > {
> > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > - inode->i_ino != attrs->ino ||
> > > - !inode_eq_iversion(inode, attrs->version));
> > > + struct kstat stat;
> > > +
> > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > + inode->i_ino != attrs->ino)
> > > + return true;
> > > +
> > > + if (inode_eq_iversion(inode, attrs->version))
> > > + return false;
> > > +
> > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT))
> > > + return true;
> > > +
> >
> > This is rather odd. You're sampling the i_version field directly, but
> > if it's not equal then you go through ->getattr() to get the ctime.
> >
> > It's particularly odd since you don't know whether the i_version field
> > is even implemented on the fs. On filesystems where it isn't, the
> > i_version field generally stays at 0, so won't this never fall through
> > to do the vfs_getattr_nosec() call on those filesystems?
> >
>
> You're totally right. I didn't consider FS's caching the value at zero.
Actually, I'm going to amend this. I think I did consider FSs without an
implementation. Where this is called at, it is often guarded by a
!IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
understanding this correctly, the check call doesn't occur unless the inode
has i_version support.
It seems to me the suggestion then is to remove the IS_I_VERSION()
checks guarding the call sites, grab both ctime and cookie from stat,
and if IS_I_VERSION() use that, otherwise cookie, and compare
against the cached i_version with one of those values, and then fall
back to ctime?
>
> > Ideally, you should just call vfs_getattr_nosec() early on with
> > STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
> > STATX_CHANGE_COOKIE if it's set in the returned mask.
> >
>
> Yes, that makes sense.
>
> I'll spin that in v1, thanks!
>
> > > + return attrs->ctime_guard != integrity_ctime_guard(stat);
> > > }
> > >
> > >
> > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> > > --- a/security/integrity/evm/evm_crypto.c
> > > +++ b/security/integrity/evm/evm_crypto.c
> > > @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > if (IS_I_VERSION(inode))
> > > i_version = inode_query_iversion(inode);
> > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > - inode);
> > > + 0, inode);
> > > }
> > >
> > > /* Portable EVM signatures must include an IMA hash */
> > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..0712802628fd6533383f9855687e19bef7b771c7 100644
> > > --- a/security/integrity/evm/evm_main.c
> > > +++ b/security/integrity/evm/evm_main.c
> > > @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > if (iint) {
> > > ret = (!IS_I_VERSION(metadata_inode) ||
> > > integrity_inode_attrs_changed(&iint->metadata_inode,
> > > - metadata_inode));
> > > + NULL, metadata_inode));
> > > if (ret)
> > > iint->evm_status = INTEGRITY_UNKNOWN;
> > > }
> > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..72bba8daa588a0f4e45e4249276edb54ca3d77ef 100644
> > > --- a/security/integrity/ima/ima_api.c
> > > +++ b/security/integrity/ima/ima_api.c
> > > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > int length;
> > > void *tmpbuf;
> > > u64 i_version = 0;
> > > + u64 ctime_guard = 0;
> > >
> > > /*
> > > * Always collect the modsig, because IMA might have already collected
> > > @@ -272,10 +273,16 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > * to an initial measurement/appraisal/audit, but was modified to
> > > * assume the file changed.
> > > */
> > > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > AT_STATX_SYNC_AS_STAT);
> > > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > > - i_version = stat.change_cookie;
> > > + if (!result) {
> > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > + i_version = stat.change_cookie;
> > > +
> > > + if (stat.result_mask & STATX_CTIME)
> > > + ctime_guard = integrity_ctime_guard(stat);
> > > + }
> > > hash.hdr.algo = algo;
> > > hash.hdr.length = hash_digest_size[algo];
> > >
> > > @@ -305,11 +312,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > >
> > > iint->ima_hash = tmpbuf;
> > > memcpy(iint->ima_hash, &hash, length);
> > > - if (real_inode == inode)
> > > + if (real_inode == inode) {
> > > iint->real_inode.version = i_version;
> > > - else
> > > + iint->real_inode.ctime_guard = ctime_guard;
> > > + } else {
> > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > - real_inode);
> > > + ctime_guard, real_inode);
> > > + }
> > >
> > > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > > if (!result)
> > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > index 5770cf691912aa912fc65280c59f5baac35dd725..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -22,6 +22,7 @@
> > > #include <linux/mount.h>
> > > #include <linux/mman.h>
> > > #include <linux/slab.h>
> > > +#include <linux/stat.h>
> > > #include <linux/xattr.h>
> > > #include <linux/ima.h>
> > > #include <linux/fs.h>
> > > @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > {
> > > fmode_t mode = file->f_mode;
> > > bool update;
> > > + int ret;
> > >
> > > if (!(mode & FMODE_WRITE))
> > > return;
> > > @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > >
> > > update = test_and_clear_bit(IMA_UPDATE_XATTR,
> > > &iint->atomic_flags);
> > > - if ((iint->flags & IMA_NEW_FILE) ||
> > > - vfs_getattr_nosec(&file->f_path, &stat,
> > > - STATX_CHANGE_COOKIE,
> > > - AT_STATX_SYNC_AS_STAT) ||
> > > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > > - stat.change_cookie != iint->real_inode.version) {
> > > + ret = vfs_getattr_nosec(&file->f_path, &stat,
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT);
> > > + if ((iint->flags & IMA_NEW_FILE) || ret ||
> > > + (!ret && stat.change_cookie != iint->real_inode.version) ||
> > > + (!ret && integrity_ctime_guard(stat) !=
> > > + iint->real_inode.ctime_guard)) {
> > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > iint->measured_pcrs = 0;
> > > if (update)
> > > @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > if (!IS_I_VERSION(real_inode) ||
> > > integrity_inode_attrs_changed(&iint->real_inode,
> > > - real_inode)) {
> > > + file, real_inode)) {
> > > iint->flags &= ~IMA_DONE_MASK;
> > > iint->measured_pcrs = 0;
> > > }
> > >
> > > ---
> > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> > >
> > > Best regards,
> >
> > --
> > Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Frederick Lawler @ 2026-01-06 16:43 UTC (permalink / raw)
To: Jeff Layton
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <3ad9ded9b3a269908eee6c79b70dbf432e60ce8d.camel@kernel.org>
Hi Jeff,
On Tue, Jan 06, 2026 at 07:01:08AM -0500, Jeff Layton wrote:
> On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> > Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> > is no longer able to correctly track inode.i_version due to the struct
> > kstat.change_cookie no longer containing an updated i_version.
> >
> > Introduce a fallback mechanism for IMA that instead tracks a
> > integrity_ctime_guard() in absence of or outdated i_version
> > for stacked file systems.
> >
> > EVM is left alone since it mostly cares about the backing inode.
> >
> > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > The motivation behind this was that file systems that use the
> > cookie to set the i_version for stacked file systems may still do so.
> > Then add in the ctime_guard as a fallback if there's a detected change.
> > The assumption is that the ctime will be different if the i_version is
> > different anyway for non-stacked file systems.
> >
> > I'm not too pleased with passing in struct file* to
> > integrity_inode_attrs_changed() since EVM doesn't currently use
> > that for now, but I couldn't come up with another idea to get the
> > stat without coming up with a new stat function to accommodate just
> > the file path, fully separate out IMA/EVM checks, or lastly add stacked
> > file system support to EVM (which doesn't make much sense to me
> > at the moment).
> >
> > I plan on adding in self test infrastructure for the v1, but I would
> > like to get some early feedback on the approach first.
> > ---
> > include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> > security/integrity/evm/evm_crypto.c | 2 +-
> > security/integrity/evm/evm_main.c | 2 +-
> > security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> > security/integrity/ima/ima_main.c | 17 ++++++++++-------
> > 5 files changed, 51 insertions(+), 20 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
> >
> > /* An inode's attributes for detection of changes */
> > struct integrity_inode_attributes {
> > + u64 ctime_guard;
> > u64 version; /* track inode changes */
> > unsigned long ino;
> > dev_t dev;
> > };
> >
> > +static inline u64 integrity_ctime_guard(struct kstat stat)
> > +{
> > + return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > +}
> > +
> > /*
> > * On stacked filesystems the i_version alone is not enough to detect file data
> > * or metadata change. Additional metadata is required.
> > */
> > static inline void
> > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > - u64 i_version, const struct inode *inode)
> > + u64 i_version, u64 ctime_guard,
> > + const struct inode *inode)
> > {
> > + attrs->ctime_guard = ctime_guard;
> > attrs->version = i_version;
> > attrs->dev = inode->i_sb->s_dev;
> > attrs->ino = inode->i_ino;
> > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > */
> > static inline bool
> > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > - const struct inode *inode)
> > + struct file *file, struct inode *inode)
> > {
> > - return (inode->i_sb->s_dev != attrs->dev ||
> > - inode->i_ino != attrs->ino ||
> > - !inode_eq_iversion(inode, attrs->version));
> > + struct kstat stat;
> > +
> > + if (inode->i_sb->s_dev != attrs->dev ||
> > + inode->i_ino != attrs->ino)
> > + return true;
> > +
> > + if (inode_eq_iversion(inode, attrs->version))
> > + return false;
> > +
> > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > + AT_STATX_SYNC_AS_STAT))
> > + return true;
> > +
>
> This is rather odd. You're sampling the i_version field directly, but
> if it's not equal then you go through ->getattr() to get the ctime.
>
> It's particularly odd since you don't know whether the i_version field
> is even implemented on the fs. On filesystems where it isn't, the
> i_version field generally stays at 0, so won't this never fall through
> to do the vfs_getattr_nosec() call on those filesystems?
>
You're totally right. I didn't consider FS's caching the value at zero.
> Ideally, you should just call vfs_getattr_nosec() early on with
> STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
> STATX_CHANGE_COOKIE if it's set in the returned mask.
>
Yes, that makes sense.
I'll spin that in v1, thanks!
> > + return attrs->ctime_guard != integrity_ctime_guard(stat);
> > }
> >
> >
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > if (IS_I_VERSION(inode))
> > i_version = inode_query_iversion(inode);
> > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > - inode);
> > + 0, inode);
> > }
> >
> > /* Portable EVM signatures must include an IMA hash */
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..0712802628fd6533383f9855687e19bef7b771c7 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > if (iint) {
> > ret = (!IS_I_VERSION(metadata_inode) ||
> > integrity_inode_attrs_changed(&iint->metadata_inode,
> > - metadata_inode));
> > + NULL, metadata_inode));
> > if (ret)
> > iint->evm_status = INTEGRITY_UNKNOWN;
> > }
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..72bba8daa588a0f4e45e4249276edb54ca3d77ef 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > int length;
> > void *tmpbuf;
> > u64 i_version = 0;
> > + u64 ctime_guard = 0;
> >
> > /*
> > * Always collect the modsig, because IMA might have already collected
> > @@ -272,10 +273,16 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > +
> > + if (stat.result_mask & STATX_CTIME)
> > + ctime_guard = integrity_ctime_guard(stat);
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
> > @@ -305,11 +312,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> >
> > iint->ima_hash = tmpbuf;
> > memcpy(iint->ima_hash, &hash, length);
> > - if (real_inode == inode)
> > + if (real_inode == inode) {
> > iint->real_inode.version = i_version;
> > - else
> > + iint->real_inode.ctime_guard = ctime_guard;
> > + } else {
> > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > - real_inode);
> > + ctime_guard, real_inode);
> > + }
> >
> > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > if (!result)
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 5770cf691912aa912fc65280c59f5baac35dd725..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -22,6 +22,7 @@
> > #include <linux/mount.h>
> > #include <linux/mman.h>
> > #include <linux/slab.h>
> > +#include <linux/stat.h>
> > #include <linux/xattr.h>
> > #include <linux/ima.h>
> > #include <linux/fs.h>
> > @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > {
> > fmode_t mode = file->f_mode;
> > bool update;
> > + int ret;
> >
> > if (!(mode & FMODE_WRITE))
> > return;
> > @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> >
> > update = test_and_clear_bit(IMA_UPDATE_XATTR,
> > &iint->atomic_flags);
> > - if ((iint->flags & IMA_NEW_FILE) ||
> > - vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > - AT_STATX_SYNC_AS_STAT) ||
> > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > - stat.change_cookie != iint->real_inode.version) {
> > + ret = vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > + AT_STATX_SYNC_AS_STAT);
> > + if ((iint->flags & IMA_NEW_FILE) || ret ||
> > + (!ret && stat.change_cookie != iint->real_inode.version) ||
> > + (!ret && integrity_ctime_guard(stat) !=
> > + iint->real_inode.ctime_guard)) {
> > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > iint->measured_pcrs = 0;
> > if (update)
> > @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > if (!IS_I_VERSION(real_inode) ||
> > integrity_inode_attrs_changed(&iint->real_inode,
> > - real_inode)) {
> > + file, real_inode)) {
> > iint->flags &= ~IMA_DONE_MASK;
> > iint->measured_pcrs = 0;
> > }
> >
> > ---
> > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> >
> > Best regards,
>
> --
> Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* [PATCH v3 6/6] docs: trusted-encryped: add PKWM as a new trust source
From: Srish Srinivasan @ 2026-01-06 15:05 UTC (permalink / raw)
To: linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, nayna, rnsastry, linux-kernel, linux-security-module,
ssrish
In-Reply-To: <20260106150527.446525-1-ssrish@linux.ibm.com>
From: Nayna Jain <nayna@linux.ibm.com>
Update Documentation/security/keys/trusted-encrypted.rst and Documentation/
admin-guide/kernel-parameters.txt with PowerVM Key Wrapping Module (PKWM)
as a new trust source
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
.../admin-guide/kernel-parameters.txt | 1 +
.../security/keys/trusted-encrypted.rst | 50 +++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a8d0afde7f85..ccb9c2f502fb 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -7755,6 +7755,7 @@ Kernel parameters
- "tee"
- "caam"
- "dcp"
+ - "pkwm"
If not specified then it defaults to iterating through
the trust source list starting with TPM and assigns the
first trust source as a backend which is initialized
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index eae6a36b1c9a..ddff7c7c2582 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -81,6 +81,14 @@ safe.
and the UNIQUE key. Default is to use the UNIQUE key, but selecting
the OTP key can be done via a module parameter (dcp_use_otp_key).
+ (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+ Rooted to a unique, per-LPAR key, which is derived from a system-wide,
+ randomly generated LPAR root key. Both the per-LPAR keys and the LPAR
+ root key are stored in hypervisor-owned secure memory at runtime,
+ and the LPAR root key is additionally persisted in secure locations
+ such as the processor SEEPROMs and encrypted NVRAM.
+
* Execution isolation
(1) TPM
@@ -102,6 +110,14 @@ safe.
environment. Only basic blob key encryption is executed there.
The actual key sealing/unsealing is done on main processor/kernel space.
+ (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+ Fixed set of cryptographic operations done on on-chip hardware
+ cryptographic acceleration unit NX. Keys for wrapping and unwrapping
+ are managed by PowerVM Platform KeyStore, which stores keys in an
+ isolated in-memory copy in secure hypervisor memory, as well as in a
+ persistent copy in hypervisor-encrypted NVRAM.
+
* Optional binding to platform integrity state
(1) TPM
@@ -129,6 +145,11 @@ safe.
Relies on Secure/Trusted boot process (called HAB by vendor) for
platform integrity.
+ (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+ Relies on secure and trusted boot process of IBM Power systems for
+ platform integrity.
+
* Interfaces and APIs
(1) TPM
@@ -149,6 +170,11 @@ safe.
Vendor-specific API that is implemented as part of the DCP crypto driver in
``drivers/crypto/mxs-dcp.c``.
+ (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+ Platform Keystore has well documented interfaces in PAPR document.
+ Refer to ``Documentation/arch/powerpc/papr_hcalls.rst``
+
* Threat model
The strength and appropriateness of a particular trust source for a given
@@ -191,6 +217,10 @@ selected trust source:
a dedicated hardware RNG that is independent from DCP which can be enabled
to back the kernel RNG.
+ * PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+ The normal kernel random number generator is used to generate keys.
+
Users may override this by specifying ``trusted.rng=kernel`` on the kernel
command-line to override the used RNG with the kernel's random number pool.
@@ -321,6 +351,26 @@ Usage::
specific to this DCP key-blob implementation. The key length for new keys is
always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
+Trusted Keys usage: PKWM
+------------------------
+
+Usage::
+
+ keyctl add trusted name "new keylen [options]" ring
+ keyctl add trusted name "load hex_blob" ring
+ keyctl print keyid
+
+ options:
+ wrap_flags= ascii hex value of security policy requirement
+ 0x00: no secure boot requirement (default)
+ 0x01: require secure boot to be in either audit or
+ enforced mode
+ 0x02: require secure boot to be in enforced mode
+
+"keyctl print" returns an ASCII hex copy of the sealed key, which is in format
+specific to PKWM key-blob implementation. The key length for new keys is
+always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
+
Encrypted Keys usage
--------------------
--
2.47.3
^ permalink raw reply related
* [PATCH v3 5/6] keys/trusted_keys: establish PKWM as a trusted source
From: Srish Srinivasan @ 2026-01-06 15:05 UTC (permalink / raw)
To: linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, nayna, rnsastry, linux-kernel, linux-security-module,
ssrish
In-Reply-To: <20260106150527.446525-1-ssrish@linux.ibm.com>
The wrapping key does not exist by default and is generated by the
hypervisor as a part of PKWM initialization. This key is then persisted by
the hypervisor and is used to wrap trusted keys. These are variable length
symmetric keys, which in the case of PowerVM Key Wrapping Module (PKWM) are
generated using the kernel RNG. PKWM can be used as a trust source through
the following example keyctl commands:
keyctl add trusted my_trusted_key "new 32" @u
Use the wrap_flags command option to set the secure boot requirement for
the wrapping request through the following keyctl commands
case1: no secure boot requirement. (default)
keyctl usage: keyctl add trusted my_trusted_key "new 32" @u
OR
keyctl add trusted my_trusted_key "new 32 wrap_flags=0x00" @u
case2: secure boot required to in either audit or enforce mode. set bit 0
keyctl usage: keyctl add trusted my_trusted_key "new 32 wrap_flags=0x01" @u
case3: secure boot required to be in enforce mode. set bit 1
keyctl usage: keyctl add trusted my_trusted_key "new 32 wrap_flags=0x02" @u
NOTE:
-> Setting the secure boot requirement is NOT a must.
-> Only either of the secure boot requirement options should be set. Not
both.
-> All the other bits are required to be not set.
-> Set the kernel parameter trusted.source=pkwm to choose PKWM as the
backend for trusted keys implementation.
-> CONFIG_PSERIES_PLPKS must be enabled to build PKWM.
Add PKWM, which is a combination of IBM PowerVM and Power LPAR Platform
KeyStore, as a new trust source for trusted keys.
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
MAINTAINERS | 9 ++
include/keys/trusted-type.h | 7 +-
include/keys/trusted_pkwm.h | 22 +++
security/keys/trusted-keys/Kconfig | 8 ++
security/keys/trusted-keys/Makefile | 2 +
security/keys/trusted-keys/trusted_core.c | 6 +-
security/keys/trusted-keys/trusted_pkwm.c | 168 ++++++++++++++++++++++
7 files changed, 220 insertions(+), 2 deletions(-)
create mode 100644 include/keys/trusted_pkwm.h
create mode 100644 security/keys/trusted-keys/trusted_pkwm.c
diff --git a/MAINTAINERS b/MAINTAINERS
index a0dd762f5648..ba51eff21a16 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14003,6 +14003,15 @@ S: Supported
F: include/keys/trusted_dcp.h
F: security/keys/trusted-keys/trusted_dcp.c
+KEYS-TRUSTED-PLPKS
+M: Srish Srinivasan <ssrish@linux.ibm.com>
+M: Nayna Jain <nayna@linux.ibm.com>
+L: linux-integrity@vger.kernel.org
+L: keyrings@vger.kernel.org
+S: Supported
+F: include/keys/trusted_plpks.h
+F: security/keys/trusted-keys/trusted_pkwm.c
+
KEYS-TRUSTED-TEE
M: Sumit Garg <sumit.garg@kernel.org>
L: linux-integrity@vger.kernel.org
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 4eb64548a74f..45c6c538df22 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -19,7 +19,11 @@
#define MIN_KEY_SIZE 32
#define MAX_KEY_SIZE 128
-#define MAX_BLOB_SIZE 512
+#if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
+#define MAX_BLOB_SIZE 1152
+#else
+#define MAX_BLOB_SIZE 512
+#endif
#define MAX_PCRINFO_SIZE 64
#define MAX_DIGEST_SIZE 64
@@ -46,6 +50,7 @@ struct trusted_key_options {
uint32_t policydigest_len;
unsigned char policydigest[MAX_DIGEST_SIZE];
uint32_t policyhandle;
+ uint16_t wrap_flags;
};
struct trusted_key_ops {
diff --git a/include/keys/trusted_pkwm.h b/include/keys/trusted_pkwm.h
new file mode 100644
index 000000000000..c7249d08b4d8
--- /dev/null
+++ b/include/keys/trusted_pkwm.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PKWM_TRUSTED_KEY_H
+#define __PKWM_TRUSTED_KEY_H
+
+#include <keys/trusted-type.h>
+
+extern struct trusted_key_ops pkwm_trusted_key_ops;
+
+static inline void dump_options(struct trusted_key_options *o)
+{
+ bool sb_audit_or_enforce_bit = o->wrap_flags & BIT(0);
+ bool sb_enforce_bit = o->wrap_flags & BIT(1);
+
+ if (sb_audit_or_enforce_bit)
+ pr_debug("secure boot mode required: audit or enforce");
+ else if (sb_enforce_bit)
+ pr_debug("secure boot mode required: enforce");
+ else
+ pr_debug("secure boot mode required: disabled");
+}
+
+#endif
diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
index 204a68c1429d..9e00482d886a 100644
--- a/security/keys/trusted-keys/Kconfig
+++ b/security/keys/trusted-keys/Kconfig
@@ -46,6 +46,14 @@ config TRUSTED_KEYS_DCP
help
Enable use of NXP's DCP (Data Co-Processor) as trusted key backend.
+config TRUSTED_KEYS_PKWM
+ bool "PKWM-based trusted keys"
+ depends on PSERIES_PLPKS >= TRUSTED_KEYS
+ default y
+ select HAVE_TRUSTED_KEYS
+ help
+ Enable use of IBM PowerVM Key Wrapping Module (PKWM) as a trusted key backend.
+
if !HAVE_TRUSTED_KEYS
comment "No trust source selected!"
endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index f0f3b27f688b..5fc053a21dad 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -16,3 +16,5 @@ trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
trusted-$(CONFIG_TRUSTED_KEYS_CAAM) += trusted_caam.o
trusted-$(CONFIG_TRUSTED_KEYS_DCP) += trusted_dcp.o
+
+trusted-$(CONFIG_TRUSTED_KEYS_PKWM) += trusted_pkwm.o
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index b1680ee53f86..2d328de170e8 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -12,6 +12,7 @@
#include <keys/trusted_caam.h>
#include <keys/trusted_dcp.h>
#include <keys/trusted_tpm.h>
+#include <keys/trusted_pkwm.h>
#include <linux/capability.h>
#include <linux/err.h>
#include <linux/init.h>
@@ -31,7 +32,7 @@ MODULE_PARM_DESC(rng, "Select trusted key RNG");
static char *trusted_key_source;
module_param_named(source, trusted_key_source, charp, 0);
-MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam or dcp)");
+MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam, dcp or pkwm)");
static const struct trusted_key_source trusted_key_sources[] = {
#if defined(CONFIG_TRUSTED_KEYS_TPM)
@@ -46,6 +47,9 @@ static const struct trusted_key_source trusted_key_sources[] = {
#if defined(CONFIG_TRUSTED_KEYS_DCP)
{ "dcp", &dcp_trusted_key_ops },
#endif
+#if defined(CONFIG_TRUSTED_KEYS_PKWM)
+ { "pkwm", &pkwm_trusted_key_ops },
+#endif
};
DEFINE_STATIC_CALL_NULL(trusted_key_seal, *trusted_key_sources[0].ops->seal);
diff --git a/security/keys/trusted-keys/trusted_pkwm.c b/security/keys/trusted-keys/trusted_pkwm.c
new file mode 100644
index 000000000000..d822b81afacf
--- /dev/null
+++ b/security/keys/trusted-keys/trusted_pkwm.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
+ */
+
+#include <keys/trusted_pkwm.h>
+#include <keys/trusted-type.h>
+#include <linux/build_bug.h>
+#include <linux/key-type.h>
+#include <linux/parser.h>
+#include <asm/plpks.h>
+
+enum {
+ Opt_err,
+ Opt_wrap_flags,
+};
+
+static const match_table_t key_tokens = {
+ {Opt_wrap_flags, "wrap_flags=%s"},
+ {Opt_err, NULL}
+};
+
+static int getoptions(char *datablob, struct trusted_key_options **opt)
+{
+ substring_t args[MAX_OPT_ARGS];
+ char *p = datablob;
+ int token;
+ int res;
+ unsigned long wrap_flags;
+ unsigned long token_mask = 0;
+
+ if (!datablob)
+ return 0;
+
+ while ((p = strsep(&datablob, " \t"))) {
+ if (*p == '\0' || *p == ' ' || *p == '\t')
+ continue;
+
+ token = match_token(p, key_tokens, args);
+ if (test_and_set_bit(token, &token_mask))
+ return -EINVAL;
+
+ switch (token) {
+ case Opt_wrap_flags:
+ res = kstrtoul(args[0].from, 16, &wrap_flags);
+ if (res < 0 || wrap_flags > 2)
+ return -EINVAL;
+ (*opt)->wrap_flags = wrap_flags;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+
+static struct trusted_key_options *trusted_options_alloc(void)
+{
+ struct trusted_key_options *options;
+
+ options = kzalloc(sizeof(*options), GFP_KERNEL);
+ return options;
+}
+
+static int trusted_pkwm_seal(struct trusted_key_payload *p, char *datablob)
+{
+ struct trusted_key_options *options = NULL;
+ u8 *input_buf, *output_buf;
+ u32 output_len, input_len;
+ int rc;
+
+ options = trusted_options_alloc();
+ if (!options)
+ return -ENOMEM;
+
+ rc = getoptions(datablob, &options);
+ if (rc < 0)
+ goto out;
+ dump_options(options);
+
+ input_len = p->key_len;
+ input_buf = kmalloc(ALIGN(input_len, 4096), GFP_KERNEL);
+ if (!input_buf) {
+ pr_err("Input buffer allocation failed. Returning -ENOMEM.");
+ return -ENOMEM;
+ }
+
+ memcpy(input_buf, p->key, p->key_len);
+
+ rc = plpks_wrap_object(&input_buf, input_len, options->wrap_flags,
+ &output_buf, &output_len);
+ if (!rc) {
+ memcpy(p->blob, output_buf, output_len);
+ p->blob_len = output_len;
+ dump_payload(p);
+ } else {
+ pr_err("Wrapping of payload key failed: %d\n", rc);
+ }
+
+ kfree(input_buf);
+ kfree(output_buf);
+
+out:
+ kfree_sensitive(options);
+ return rc;
+}
+
+static int trusted_pkwm_unseal(struct trusted_key_payload *p, char *datablob)
+{
+ u8 *input_buf, *output_buf;
+ u32 input_len, output_len;
+ int rc;
+
+ input_len = p->blob_len;
+ input_buf = kmalloc(ALIGN(input_len, 4096), GFP_KERNEL);
+ if (!input_buf) {
+ pr_err("Input buffer allocation failed. Returning -ENOMEM.");
+ return -ENOMEM;
+ }
+
+ memcpy(input_buf, p->blob, p->blob_len);
+
+ rc = plpks_unwrap_object(&input_buf, input_len, &output_buf,
+ &output_len);
+ if (!rc) {
+ memcpy(p->key, output_buf, output_len);
+ p->key_len = output_len;
+ dump_payload(p);
+ } else {
+ pr_err("Unwrapping of payload failed: %d\n", rc);
+ }
+
+ kfree(input_buf);
+ kfree(output_buf);
+
+ return rc;
+}
+
+static int trusted_pkwm_init(void)
+{
+ int ret;
+
+ if (!plpks_wrapping_is_supported()) {
+ pr_err("H_PKS_WRAP_OBJECT interface not supported\n");
+ return -ENODEV;
+ }
+
+ ret = plpks_gen_wrapping_key();
+ if (ret) {
+ pr_err("Failed to generate default wrapping key\n");
+ return -EINVAL;
+ }
+
+ return register_key_type(&key_type_trusted);
+}
+
+static void trusted_pkwm_exit(void)
+{
+ unregister_key_type(&key_type_trusted);
+}
+
+struct trusted_key_ops pkwm_trusted_key_ops = {
+ .migratable = 0, /* non-migratable */
+ .init = trusted_pkwm_init,
+ .seal = trusted_pkwm_seal,
+ .unseal = trusted_pkwm_unseal,
+ .exit = trusted_pkwm_exit,
+};
--
2.47.3
^ permalink raw reply related
* [PATCH v3 4/6] pseries/plpks: add HCALLs for PowerVM Key Wrapping Module
From: Srish Srinivasan @ 2026-01-06 15:05 UTC (permalink / raw)
To: linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, nayna, rnsastry, linux-kernel, linux-security-module,
ssrish
In-Reply-To: <20260106150527.446525-1-ssrish@linux.ibm.com>
The hypervisor generated wrapping key is an AES-GCM-256 symmetric key which
is stored in a non-volatile, secure, and encrypted storage called the Power
LPAR Platform KeyStore. It has policy based protections that prevent it
from being read out or exposed to the user.
Implement H_PKS_GEN_KEY, H_PKS_WRAP_OBJECT, and H_PKS_UNWRAP_OBJECT HCALLs
to enable using the PowerVM Key Wrapping Module (PKWM) as a new trust
source for trusted keys. Disallow H_PKS_READ_OBJECT, H_PKS_SIGNED_UPDATE,
and H_PKS_WRITE_OBJECT for objects with the 'wrapping key' policy set.
Capture the availability status for the H_PKS_WRAP_OBJECT interface.
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
---
Documentation/arch/powerpc/papr_hcalls.rst | 43 +++
arch/powerpc/include/asm/plpks.h | 10 +
arch/powerpc/platforms/pseries/plpks.c | 342 ++++++++++++++++++++-
3 files changed, 393 insertions(+), 2 deletions(-)
diff --git a/Documentation/arch/powerpc/papr_hcalls.rst b/Documentation/arch/powerpc/papr_hcalls.rst
index 805e1cb9bab9..14e39f095a1c 100644
--- a/Documentation/arch/powerpc/papr_hcalls.rst
+++ b/Documentation/arch/powerpc/papr_hcalls.rst
@@ -300,6 +300,49 @@ H_HTM supports setup, configuration, control and dumping of Hardware Trace
Macro (HTM) function and its data. HTM buffer stores tracing data for functions
like core instruction, core LLAT and nest.
+**H_PKS_GEN_KEY**
+
+| Input: authorization, objectlabel, objectlabellen, policy, out, outlen
+| Out: *Hypervisor Generated Key, or None when the wrapping key policy is set*
+| Return Value: *H_SUCCESS, H_Function, H_State, H_R_State, H_Parameter, H_P2,
+ H_P3, H_P4, H_P5, H_P6, H_Authority, H_Nomem, H_Busy, H_Resource,
+ H_Aborted*
+
+H_PKS_GEN_KEY is used to have the hypervisor generate a new random key.
+This key is stored as an object in the Power LPAR Platform KeyStore with
+the provided object label. With the wrapping key policy set the key is only
+visible to the hypervisor, while the key's label would still be visible to
+the user. Generation of wrapping keys is supported only for a key size of
+32 bytes.
+
+**H_PKS_WRAP_OBJECT**
+
+| Input: authorization, wrapkeylabel, wrapkeylabellen, objectwrapflags, in,
+| inlen, out, outlen, continue-token
+| Out: *continue-token, byte size of wrapped object, wrapped object*
+| Return Value: *H_SUCCESS, H_Function, H_State, H_R_State, H_Parameter, H_P2,
+ H_P3, H_P4, H_P5, H_P6, H_P7, H_P8, H_P9, H_Authority, H_Invalid_Key,
+ H_NOT_FOUND, H_Busy, H_LongBusy, H_Aborted*
+
+H_PKS_WRAP_OBJECT is used to wrap an object using a wrapping key stored in the
+Power LPAR Platform KeyStore and return the wrapped object to the caller. The
+caller provides a label to a wrapping key with the 'wrapping key' policy set,
+which must have been previously created with H_PKS_GEN_KEY. The provided object
+is then encrypted with the wrapping key and additional metadata and the result
+is returned to the caller.
+
+
+**H_PKS_UNWRAP_OBJECT**
+
+| Input: authorization, objectwrapflags, in, inlen, out, outlen, continue-token
+| Out: *continue-token, byte size of unwrapped object, unwrapped object*
+| Return Value: *H_SUCCESS, H_Function, H_State, H_R_State, H_Parameter, H_P2,
+ H_P3, H_P4, H_P5, H_P6, H_P7, H_Authority, H_Unsupported, H_Bad_Data,
+ H_NOT_FOUND, H_Invalid_Key, H_Busy, H_LongBusy, H_Aborted*
+
+H_PKS_UNWRAP_OBJECT is used to unwrap an object that was previously warapped with
+H_PKS_WRAP_OBJECT.
+
References
==========
.. [1] "Power Architecture Platform Reference"
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 8f034588fdf7..e87f90e40d4e 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -113,6 +113,16 @@ void plpks_early_init_devtree(void);
int plpks_populate_fdt(void *fdt);
int plpks_config_create_softlink(struct kobject *from);
+
+bool plpks_wrapping_is_supported(void);
+
+int plpks_gen_wrapping_key(void);
+
+int plpks_wrap_object(u8 **input_buf, u32 input_len, u16 wrap_flags,
+ u8 **output_buf, u32 *output_len);
+
+int plpks_unwrap_object(u8 **input_buf, u32 input_len,
+ u8 **output_buf, u32 *output_len);
#else // CONFIG_PSERIES_PLPKS
static inline bool plpks_is_available(void) { return false; }
static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index 4a08f51537c8..b97b7750f6a8 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -9,6 +9,32 @@
#define pr_fmt(fmt) "plpks: " fmt
+#define PLPKS_WRAPKEY_COMPONENT "PLPKSWR"
+#define PLPKS_WRAPKEY_NAME "default-wrapping-key"
+
+/*
+ * To 4K align the {input, output} buffers to the {UN}WRAP H_CALLs
+ */
+#define PLPKS_WRAPPING_BUF_ALIGN 4096
+
+/*
+ * To ensure the output buffer's length is at least 1024 bytes greater
+ * than the input buffer's length during the WRAP H_CALL
+ */
+#define PLPKS_WRAPPING_BUF_DIFF 1024
+
+#define PLPKS_WRAP_INTERFACE_BIT 3
+#define PLPKS_WRAPPING_KEY_LENGTH 32
+
+#define WRAPFLAG_BE_BIT_SET(be_bit) \
+ BIT_ULL(63 - (be_bit))
+
+#define WRAPFLAG_BE_GENMASK(be_bit_hi, be_bit_lo) \
+ GENMASK_ULL(63 - (be_bit_hi), 63 - (be_bit_lo))
+
+#define WRAPFLAG_BE_FIELD_PREP(be_bit_hi, be_bit_lo, val) \
+ FIELD_PREP(WRAPFLAG_BE_GENMASK(be_bit_hi, be_bit_lo), (val))
+
#include <linux/delay.h>
#include <linux/errno.h>
#include <linux/io.h>
@@ -39,6 +65,7 @@ static u32 supportedpolicies;
static u32 maxlargeobjectsize;
static u64 signedupdatealgorithms;
static u64 wrappingfeatures;
+static bool wrapsupport;
struct plpks_auth {
u8 version;
@@ -283,6 +310,7 @@ static int _plpks_get_config(void)
maxlargeobjectsize = be32_to_cpu(config->maxlargeobjectsize);
signedupdatealgorithms = be64_to_cpu(config->signedupdatealgorithms);
wrappingfeatures = be64_to_cpu(config->wrappingfeatures);
+ wrapsupport = config->flags & PPC_BIT8(PLPKS_WRAP_INTERFACE_BIT);
// Validate that the numbers we get back match the requirements of the spec
if (maxpwsize < 32) {
@@ -614,6 +642,9 @@ int plpks_signed_update_var(struct plpks_var *var, u64 flags)
if (!(var->policy & PLPKS_SIGNEDUPDATE))
return -EINVAL;
+ if (var->policy & PLPKS_WRAPPINGKEY)
+ return -EINVAL;
+
// Signed updates need the component to be NULL.
if (var->component)
return -EINVAL;
@@ -696,6 +727,9 @@ int plpks_write_var(struct plpks_var var)
if (var.policy & PLPKS_SIGNEDUPDATE)
return -EINVAL;
+ if (var.policy & PLPKS_WRAPPINGKEY)
+ return -EINVAL;
+
auth = construct_auth(PLPKS_OS_OWNER);
if (IS_ERR(auth))
return PTR_ERR(auth);
@@ -790,6 +824,9 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var)
if (var->namelen > PLPKS_MAX_NAME_SIZE)
return -EINVAL;
+ if (var->policy & PLPKS_WRAPPINGKEY)
+ return -EINVAL;
+
auth = construct_auth(consumer);
if (IS_ERR(auth))
return PTR_ERR(auth);
@@ -845,8 +882,309 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var)
}
/**
- * plpks_read_os_var() - Fetch the data for the specified variable that is
- * owned by the OS consumer.
+ * plpks_wrapping_is_supported() - Get the H_PKS_WRAP_OBJECT interface
+ * availability status for the LPAR.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * sets bit 3 of the flags variable in the PLPKS config structure if the
+ * H_PKS_WRAP_OBJECT interface is supported.
+ *
+ * Returns: true if the H_PKS_WRAP_OBJECT interface is supported, false if not.
+ */
+bool plpks_wrapping_is_supported(void)
+{
+ return wrapsupport;
+}
+
+/**
+ * plpks_gen_wrapping_key() - Generate a new random key with the 'wrapping key'
+ * policy set.
+ *
+ * The H_PKS_GEN_KEY HCALL makes the hypervisor generate a new random key and
+ * store the key in a PLPKS object with the provided object label. With the
+ * 'wrapping key' policy set, only the label to the newly generated random key
+ * would be visible to the user.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if PLPKS modification is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid object label parameter
+ * if invalid object label len parameter
+ * if invalid or unsupported policy declaration
+ * if invalid output buffer parameter
+ * if invalid output buffer length parameter
+ * -EPERM if access is denied
+ * -ENOMEM if there is inadequate memory to perform this operation
+ * -EBUSY if unable to handle the request
+ * -EEXIST if the object label already exists
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
+int plpks_gen_wrapping_key(void)
+{
+ unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+ struct plpks_auth *auth;
+ struct label *label;
+ int rc = 0, pseries_status = 0;
+ struct plpks_var var = {
+ .name = PLPKS_WRAPKEY_NAME,
+ .namelen = strlen(var.name),
+ .policy = PLPKS_WRAPPINGKEY,
+ .os = PLPKS_VAR_LINUX,
+ .component = PLPKS_WRAPKEY_COMPONENT
+ };
+
+ auth = construct_auth(PLPKS_OS_OWNER);
+ if (IS_ERR(auth))
+ return PTR_ERR(auth);
+
+ label = construct_label(var.component, var.os, var.name, var.namelen);
+ if (IS_ERR(label)) {
+ rc = PTR_ERR(label);
+ goto out;
+ }
+
+ rc = plpar_hcall(H_PKS_GEN_KEY, retbuf,
+ virt_to_phys(auth), virt_to_phys(label),
+ label->size, var.policy,
+ NULL, PLPKS_WRAPPING_KEY_LENGTH);
+
+ if (!rc)
+ rc = plpks_confirm_object_flushed(label, auth);
+
+ pseries_status = rc;
+ rc = pseries_status_to_err(rc);
+
+ if (rc && rc != -EEXIST) {
+ pr_err("H_PKS_GEN_KEY failed. pseries_status=%d, rc=%d",
+ pseries_status, rc);
+ } else {
+ rc = 0;
+ }
+
+ kfree(label);
+out:
+ kfree(auth);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(plpks_gen_wrapping_key);
+
+/**
+ * plpks_wrap_object() - Wrap an object using the default wrapping key stored in
+ * the PLPKS.
+ * @input_buf: buffer containing the data to be wrapped
+ * @input_len: length of the input buffer
+ * @wrap_flags: object wrapping flags
+ * @output_buf: buffer to store the wrapped data
+ * @output_len: length of the output buffer
+ *
+ * The H_PKS_WRAP_OBJECT HCALL wraps an object using a wrapping key stored in
+ * the PLPKS and returns the wrapped object to the caller. The caller provides a
+ * label to the wrapping key with the 'wrapping key' policy set that must have
+ * been previously created with the H_PKS_GEN_KEY HCALL. The provided object is
+ * then encrypted with the wrapping key and additional metadata and the result
+ * is returned to the user. The metadata includes the wrapping algorithm and the
+ * wrapping key name so those parameters are not required during unwrap.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if PLPKS modification is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid wrapping key label parameter
+ * if invalid wrapping key label length parameter
+ * if invalid or unsupported object wrapping flags
+ * if invalid input buffer parameter
+ * if invalid input buffer length parameter
+ * if invalid output buffer parameter
+ * if invalid output buffer length parameter
+ * if invalid continue token parameter
+ * if the wrapping key is not compatible with the wrapping
+ * algorithm
+ * -EPERM if access is denied
+ * -ENOENT if the requested wrapping key was not found
+ * -EBUSY if unable to handle the request or long running operation
+ * initiated, retry later.
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
+int plpks_wrap_object(u8 **input_buf, u32 input_len, u16 wrap_flags,
+ u8 **output_buf, u32 *output_len)
+{
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = { 0 };
+ struct plpks_auth *auth;
+ struct label *label;
+ u64 continuetoken = 0;
+ u64 objwrapflags = 0;
+ int rc = 0, pseries_status = 0;
+ bool sb_audit_or_enforce_bit = wrap_flags & BIT(0);
+ bool sb_enforce_bit = wrap_flags & BIT(1);
+ struct plpks_var var = {
+ .name = PLPKS_WRAPKEY_NAME,
+ .namelen = strlen(var.name),
+ .os = PLPKS_VAR_LINUX,
+ .component = PLPKS_WRAPKEY_COMPONENT
+ };
+
+ auth = construct_auth(PLPKS_OS_OWNER);
+ if (IS_ERR(auth))
+ return PTR_ERR(auth);
+
+ label = construct_label(var.component, var.os, var.name, var.namelen);
+ if (IS_ERR(label)) {
+ rc = PTR_ERR(label);
+ goto out;
+ }
+
+ /* Set the consumer password requirement bit. A must have. */
+ objwrapflags |= WRAPFLAG_BE_BIT_SET(3);
+
+ /* Set the wrapping algorithm bit. Just one algorithm option for now */
+ objwrapflags |= WRAPFLAG_BE_FIELD_PREP(60, 63, 0x1);
+
+ if (sb_audit_or_enforce_bit & sb_enforce_bit) {
+ pr_err("Cannot set both audit/enforce and enforce bits.");
+ rc = -EINVAL;
+ goto out_free_label;
+ } else if (sb_audit_or_enforce_bit) {
+ objwrapflags |= WRAPFLAG_BE_BIT_SET(1);
+ } else if (sb_enforce_bit) {
+ objwrapflags |= WRAPFLAG_BE_BIT_SET(2);
+ }
+
+ *output_len = input_len + PLPKS_WRAPPING_BUF_DIFF;
+
+ *output_buf = kzalloc(ALIGN(*output_len, PLPKS_WRAPPING_BUF_ALIGN),
+ GFP_KERNEL);
+ if (!(*output_buf)) {
+ pr_err("Output buffer allocation failed. Returning -ENOMEM.");
+ rc = -ENOMEM;
+ goto out_free_label;
+ }
+
+ do {
+ rc = plpar_hcall9(H_PKS_WRAP_OBJECT, retbuf,
+ virt_to_phys(auth), virt_to_phys(label),
+ label->size, objwrapflags,
+ virt_to_phys(*input_buf), input_len,
+ virt_to_phys(*output_buf), *output_len,
+ continuetoken);
+
+ continuetoken = retbuf[0];
+ pseries_status = rc;
+ rc = pseries_status_to_err(rc);
+ } while (rc == -EBUSY);
+
+ if (rc) {
+ pr_err("H_PKS_WRAP_OBJECT failed. pseries_status=%d, rc=%d",
+ pseries_status, rc);
+ kfree(*output_buf);
+ *output_buf = NULL;
+ } else {
+ *output_len = retbuf[1];
+ }
+
+out_free_label:
+ kfree(label);
+out:
+ kfree(auth);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(plpks_wrap_object);
+
+/**
+ * plpks_unwrap_object() - Unwrap an object using the default wrapping key
+ * stored in the PLPKS.
+ * @input_buf: buffer containing the data to be unwrapped
+ * @input_len: length of the input buffer
+ * @output_buf: buffer to store the unwrapped data
+ * @output_len: length of the output buffer
+ *
+ * The H_PKS_UNWRAP_OBJECT HCALL unwraps an object that was previously wrapped
+ * using the H_PKS_WRAP_OBJECT HCALL.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if PLPKS modification is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid or unsupported object unwrapping flags
+ * if invalid input buffer parameter
+ * if invalid input buffer length parameter
+ * if invalid output buffer parameter
+ * if invalid output buffer length parameter
+ * if invalid continue token parameter
+ * if the wrapping key is not compatible with the wrapping
+ * algorithm
+ * if the wrapped object's format is not supported
+ * if the wrapped object is invalid
+ * -EPERM if access is denied
+ * -ENOENT if the wrapping key for the provided object was not found
+ * -EBUSY if unable to handle the request or long running operation
+ * initiated, retry later.
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
+int plpks_unwrap_object(u8 **input_buf, u32 input_len, u8 **output_buf,
+ u32 *output_len)
+{
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = { 0 };
+ struct plpks_auth *auth;
+ u64 continuetoken = 0;
+ u64 objwrapflags = 0;
+ int rc = 0, pseries_status = 0;
+
+ auth = construct_auth(PLPKS_OS_OWNER);
+ if (IS_ERR(auth))
+ return PTR_ERR(auth);
+
+ *output_len = input_len - PLPKS_WRAPPING_BUF_DIFF;
+ *output_buf = kzalloc(ALIGN(*output_len, PLPKS_WRAPPING_BUF_ALIGN),
+ GFP_KERNEL);
+ if (!(*output_buf)) {
+ pr_err("Output buffer allocation failed. Returning -ENOMEM.");
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ do {
+ rc = plpar_hcall9(H_PKS_UNWRAP_OBJECT, retbuf,
+ virt_to_phys(auth), objwrapflags,
+ virt_to_phys(*input_buf), input_len,
+ virt_to_phys(*output_buf), *output_len,
+ continuetoken);
+
+ continuetoken = retbuf[0];
+ pseries_status = rc;
+ rc = pseries_status_to_err(rc);
+ } while (rc == -EBUSY);
+
+ if (rc) {
+ pr_err("H_PKS_UNWRAP_OBJECT failed. pseries_status=%d, rc=%d",
+ pseries_status, rc);
+ kfree(*output_buf);
+ *output_buf = NULL;
+ } else {
+ *output_len = retbuf[1];
+ }
+
+out:
+ kfree(auth);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(plpks_unwrap_object);
+
+/**
+ * plpks_read_os_var() - Fetch the data for the specified variable that is owned
+ * by the OS consumer.
* @var: variable to be read from the PLPKS
*
* The consumer or the owner of the object is the os kernel. The
--
2.47.3
^ permalink raw reply related
* [PATCH v3 3/6] pseries/plpks: expose PowerVM wrapping features via the sysfs
From: Srish Srinivasan @ 2026-01-06 15:05 UTC (permalink / raw)
To: linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, nayna, rnsastry, linux-kernel, linux-security-module,
ssrish
In-Reply-To: <20260106150527.446525-1-ssrish@linux.ibm.com>
Starting with Power11, PowerVM supports a new feature called "Key Wrapping"
that protects user secrets by wrapping them using a hypervisor generated
wrapping key. The status of this feature can be read by the
H_PKS_GET_CONFIG HCALL.
Expose the Power LPAR Platform KeyStore (PLPKS) wrapping features config
via the sysfs file /sys/firmware/plpks/config/wrapping_features.
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
---
.../ABI/testing/sysfs-firmware-plpks | 8 ++++++++
arch/powerpc/include/asm/hvcall.h | 4 +++-
arch/powerpc/include/asm/plpks.h | 3 +++
arch/powerpc/platforms/pseries/plpks-sysfs.c | 2 ++
arch/powerpc/platforms/pseries/plpks.c | 20 +++++++++++++++++++
5 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-firmware-plpks b/Documentation/ABI/testing/sysfs-firmware-plpks
index af0353f34115..cba061e4eee2 100644
--- a/Documentation/ABI/testing/sysfs-firmware-plpks
+++ b/Documentation/ABI/testing/sysfs-firmware-plpks
@@ -48,3 +48,11 @@ Description: Bitmask of flags indicating which algorithms the hypervisor
supports for signed update of objects, represented as a 16 byte
hexadecimal ASCII string. Consult the hypervisor documentation
for what these flags mean.
+
+What: /sys/firmware/plpks/config/wrapping_features
+Date: November 2025
+Contact: Srish Srinivasan <ssrish@linux.ibm.com>
+Description: Bitmask of the wrapping features indicating the wrapping
+ algorithms that are supported for the H_PKS_WRAP_OBJECT requests
+ , represented as a 8 byte hexadecimal ASCII string. Consult the
+ hypervisor documentation for what these flags mean.
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 9aef16149d92..dff90a7d7f70 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -360,7 +360,9 @@
#define H_GUEST_RUN_VCPU 0x480
#define H_GUEST_COPY_MEMORY 0x484
#define H_GUEST_DELETE 0x488
-#define MAX_HCALL_OPCODE H_GUEST_DELETE
+#define H_PKS_WRAP_OBJECT 0x490
+#define H_PKS_UNWRAP_OBJECT 0x494
+#define MAX_HCALL_OPCODE H_PKS_UNWRAP_OBJECT
/* Scope args for H_SCM_UNBIND_ALL */
#define H_UNBIND_SCOPE_ALL (0x1)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 8829a13bfda0..8f034588fdf7 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -23,6 +23,7 @@
#define PLPKS_IMMUTABLE PPC_BIT32(5) // Once written, object cannot be removed
#define PLPKS_TRANSIENT PPC_BIT32(6) // Object does not persist through reboot
#define PLPKS_SIGNEDUPDATE PPC_BIT32(7) // Object can only be modified by signed updates
+#define PLPKS_WRAPPINGKEY PPC_BIT32(8) // Object contains a wrapping key
#define PLPKS_HVPROVISIONED PPC_BIT32(28) // Hypervisor has provisioned this object
// Signature algorithm flags from signed_update_algorithms
@@ -103,6 +104,8 @@ u32 plpks_get_maxlargeobjectsize(void);
u64 plpks_get_signedupdatealgorithms(void);
+u64 plpks_get_wrappingfeatures(void);
+
u16 plpks_get_passwordlen(void);
void plpks_early_init_devtree(void);
diff --git a/arch/powerpc/platforms/pseries/plpks-sysfs.c b/arch/powerpc/platforms/pseries/plpks-sysfs.c
index 01d526185783..c2ebcbb41ae3 100644
--- a/arch/powerpc/platforms/pseries/plpks-sysfs.c
+++ b/arch/powerpc/platforms/pseries/plpks-sysfs.c
@@ -30,6 +30,7 @@ PLPKS_CONFIG_ATTR(used_space, "%u\n", plpks_get_usedspace);
PLPKS_CONFIG_ATTR(supported_policies, "%08x\n", plpks_get_supportedpolicies);
PLPKS_CONFIG_ATTR(signed_update_algorithms, "%016llx\n",
plpks_get_signedupdatealgorithms);
+PLPKS_CONFIG_ATTR(wrapping_features, "%016llx\n", plpks_get_wrappingfeatures);
static const struct attribute *config_attrs[] = {
&attr_version.attr,
@@ -38,6 +39,7 @@ static const struct attribute *config_attrs[] = {
&attr_used_space.attr,
&attr_supported_policies.attr,
&attr_signed_update_algorithms.attr,
+ &attr_wrapping_features.attr,
NULL,
};
diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index 03722fabf9c3..4a08f51537c8 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -38,6 +38,7 @@ static u32 usedspace;
static u32 supportedpolicies;
static u32 maxlargeobjectsize;
static u64 signedupdatealgorithms;
+static u64 wrappingfeatures;
struct plpks_auth {
u8 version;
@@ -248,6 +249,7 @@ static int _plpks_get_config(void)
__be32 supportedpolicies;
__be32 maxlargeobjectsize;
__be64 signedupdatealgorithms;
+ __be64 wrappingfeatures;
u8 rsvd1[476];
} __packed * config;
size_t size;
@@ -280,6 +282,7 @@ static int _plpks_get_config(void)
supportedpolicies = be32_to_cpu(config->supportedpolicies);
maxlargeobjectsize = be32_to_cpu(config->maxlargeobjectsize);
signedupdatealgorithms = be64_to_cpu(config->signedupdatealgorithms);
+ wrappingfeatures = be64_to_cpu(config->wrappingfeatures);
// Validate that the numbers we get back match the requirements of the spec
if (maxpwsize < 32) {
@@ -472,6 +475,23 @@ u64 plpks_get_signedupdatealgorithms(void)
return signedupdatealgorithms;
}
+/**
+ * plpks_get_wrappingfeatures() - Returns a bitmask of the wrapping features
+ * supported by the hypervisor.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads a bitmask of the wrapping features supported by the hypervisor into the
+ * file local static wrappingfeatures variable. This is valid only when the
+ * PLPKS config structure version >= 3.
+ *
+ * Return:
+ * bitmask of the wrapping features supported by the hypervisor
+ */
+u64 plpks_get_wrappingfeatures(void)
+{
+ return wrappingfeatures;
+}
+
/**
* plpks_get_passwordlen() - Get the length of the PLPKS password in bytes.
*
--
2.47.3
^ permalink raw reply related
* [PATCH v3 2/6] powerpc/pseries: move the PLPKS config inside its own sysfs directory
From: Srish Srinivasan @ 2026-01-06 15:05 UTC (permalink / raw)
To: linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, nayna, rnsastry, linux-kernel, linux-security-module,
ssrish
In-Reply-To: <20260106150527.446525-1-ssrish@linux.ibm.com>
The /sys/firmware/secvar/config directory represents Power LPAR Platform
KeyStore (PLPKS) configuration properties such as max_object_size, signed_
update_algorithms, supported_policies, total_size, used_space, and version.
These attributes describe the PLPKS, and not the secure boot variables
(secvars).
Create /sys/firmware/plpks directory and move the PLPKS config inside this
directory. For backwards compatibility, create a soft link from the secvar
sysfs directory to this config and emit a warning stating that the older
sysfs path has been deprecated. Separate out the plpks specific
documentation from secvar.
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
.../ABI/testing/sysfs-firmware-plpks | 50 ++++++++++
Documentation/ABI/testing/sysfs-secvar | 65 -------------
arch/powerpc/include/asm/plpks.h | 5 +
arch/powerpc/include/asm/secvar.h | 1 -
arch/powerpc/kernel/secvar-sysfs.c | 21 ++---
arch/powerpc/platforms/pseries/Makefile | 2 +-
arch/powerpc/platforms/pseries/plpks-secvar.c | 29 ------
arch/powerpc/platforms/pseries/plpks-sysfs.c | 94 +++++++++++++++++++
8 files changed, 156 insertions(+), 111 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-plpks
create mode 100644 arch/powerpc/platforms/pseries/plpks-sysfs.c
diff --git a/Documentation/ABI/testing/sysfs-firmware-plpks b/Documentation/ABI/testing/sysfs-firmware-plpks
new file mode 100644
index 000000000000..af0353f34115
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-plpks
@@ -0,0 +1,50 @@
+What: /sys/firmware/plpks/config
+Date: February 2023
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description: This optional directory contains read-only config attributes as
+ defined by the PLPKS implementation. All data is in ASCII
+ format.
+
+What: /sys/firmware/plpks/config/version
+Date: February 2023
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description: Config version as reported by the hypervisor in ASCII decimal
+ format.
+
+What: /sys/firmware/plpks/config/max_object_size
+Date: February 2023
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description: Maximum allowed size of objects in the keystore in bytes,
+ represented in ASCII decimal format.
+
+ This is not necessarily the same as the max size that can be
+ written to an update file as writes can contain more than
+ object data, you should use the size of the update file for
+ that purpose.
+
+What: /sys/firmware/plpks/config/total_size
+Date: February 2023
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description: Total size of the PLPKS in bytes, represented in ASCII decimal
+ format.
+
+What: /sys/firmware/plpks/config/used_space
+Date: February 2023
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description: Current space consumed by the key store, in bytes, represented
+ in ASCII decimal format.
+
+What: /sys/firmware/plpks/config/supported_policies
+Date: February 2023
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description: Bitmask of supported policy flags by the hypervisor, represented
+ as an 8 byte hexadecimal ASCII string. Consult the hypervisor
+ documentation for what these flags are.
+
+What: /sys/firmware/plpks/config/signed_update_algorithms
+Date: February 2023
+Contact: Nayna Jain <nayna@linux.ibm.com>
+Description: Bitmask of flags indicating which algorithms the hypervisor
+ supports for signed update of objects, represented as a 16 byte
+ hexadecimal ASCII string. Consult the hypervisor documentation
+ for what these flags mean.
diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
index 1016967a730f..c52a5fd15709 100644
--- a/Documentation/ABI/testing/sysfs-secvar
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -63,68 +63,3 @@ Contact: Nayna Jain <nayna@linux.ibm.com>
Description: A write-only file that is used to submit the new value for the
variable. The size of the file represents the maximum size of
the variable data that can be written.
-
-What: /sys/firmware/secvar/config
-Date: February 2023
-Contact: Nayna Jain <nayna@linux.ibm.com>
-Description: This optional directory contains read-only config attributes as
- defined by the secure variable implementation. All data is in
- ASCII format. The directory is only created if the backing
- implementation provides variables to populate it, which at
- present is only PLPKS on the pseries platform.
-
-What: /sys/firmware/secvar/config/version
-Date: February 2023
-Contact: Nayna Jain <nayna@linux.ibm.com>
-Description: Config version as reported by the hypervisor in ASCII decimal
- format.
-
- Currently only provided by PLPKS on the pseries platform.
-
-What: /sys/firmware/secvar/config/max_object_size
-Date: February 2023
-Contact: Nayna Jain <nayna@linux.ibm.com>
-Description: Maximum allowed size of objects in the keystore in bytes,
- represented in ASCII decimal format.
-
- This is not necessarily the same as the max size that can be
- written to an update file as writes can contain more than
- object data, you should use the size of the update file for
- that purpose.
-
- Currently only provided by PLPKS on the pseries platform.
-
-What: /sys/firmware/secvar/config/total_size
-Date: February 2023
-Contact: Nayna Jain <nayna@linux.ibm.com>
-Description: Total size of the PLPKS in bytes, represented in ASCII decimal
- format.
-
- Currently only provided by PLPKS on the pseries platform.
-
-What: /sys/firmware/secvar/config/used_space
-Date: February 2023
-Contact: Nayna Jain <nayna@linux.ibm.com>
-Description: Current space consumed by the key store, in bytes, represented
- in ASCII decimal format.
-
- Currently only provided by PLPKS on the pseries platform.
-
-What: /sys/firmware/secvar/config/supported_policies
-Date: February 2023
-Contact: Nayna Jain <nayna@linux.ibm.com>
-Description: Bitmask of supported policy flags by the hypervisor,
- represented as an 8 byte hexadecimal ASCII string. Consult the
- hypervisor documentation for what these flags are.
-
- Currently only provided by PLPKS on the pseries platform.
-
-What: /sys/firmware/secvar/config/signed_update_algorithms
-Date: February 2023
-Contact: Nayna Jain <nayna@linux.ibm.com>
-Description: Bitmask of flags indicating which algorithms the hypervisor
- supports for signed update of objects, represented as a 16 byte
- hexadecimal ASCII string. Consult the hypervisor documentation
- for what these flags mean.
-
- Currently only provided by PLPKS on the pseries platform.
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index f303922bf622..8829a13bfda0 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -13,6 +13,7 @@
#include <linux/types.h>
#include <linux/list.h>
+#include <linux/kobject.h>
// Object policy flags from supported_policies
#define PLPKS_OSSECBOOTAUDIT PPC_BIT32(1) // OS secure boot must be audit/enforce
@@ -107,11 +108,15 @@ u16 plpks_get_passwordlen(void);
void plpks_early_init_devtree(void);
int plpks_populate_fdt(void *fdt);
+
+int plpks_config_create_softlink(struct kobject *from);
#else // CONFIG_PSERIES_PLPKS
static inline bool plpks_is_available(void) { return false; }
static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
static inline void plpks_early_init_devtree(void) { }
static inline int plpks_populate_fdt(void *fdt) { BUILD_BUG(); }
+static inline int plpks_config_create_softlink(struct kobject *from)
+ { return 0; }
#endif // CONFIG_PSERIES_PLPKS
#endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
index 4828e0ab7e3c..fd5006307f2a 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -20,7 +20,6 @@ struct secvar_operations {
int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
ssize_t (*format)(char *buf, size_t bufsize);
int (*max_size)(u64 *max_size);
- const struct attribute **config_attrs;
// NULL-terminated array of fixed variable names
// Only used if get_next() isn't provided
diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
index ec900bce0257..4111b21962eb 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -12,6 +12,7 @@
#include <linux/string.h>
#include <linux/of.h>
#include <asm/secvar.h>
+#include <asm/plpks.h>
#define NAME_MAX_SIZE 1024
@@ -145,19 +146,6 @@ static __init int update_kobj_size(void)
return 0;
}
-static __init int secvar_sysfs_config(struct kobject *kobj)
-{
- struct attribute_group config_group = {
- .name = "config",
- .attrs = (struct attribute **)secvar_ops->config_attrs,
- };
-
- if (secvar_ops->config_attrs)
- return sysfs_create_group(kobj, &config_group);
-
- return 0;
-}
-
static __init int add_var(const char *name)
{
struct kobject *kobj;
@@ -260,12 +248,15 @@ static __init int secvar_sysfs_init(void)
goto err;
}
- rc = secvar_sysfs_config(secvar_kobj);
+ rc = plpks_config_create_softlink(secvar_kobj);
if (rc) {
- pr_err("Failed to create config directory\n");
+ pr_err("Failed to create softlink to PLPKS config directory");
goto err;
}
+ pr_info("/sys/firmware/secvar/config is now deprecated.\n");
+ pr_info("Will be removed in future versions.\n");
+
if (secvar_ops->get_next)
rc = secvar_sysfs_load();
else
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 931ebaa474c8..3ced289a675b 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -30,7 +30,7 @@ obj-$(CONFIG_PAPR_SCM) += papr_scm.o
obj-$(CONFIG_PPC_SPLPAR) += vphn.o
obj-$(CONFIG_PPC_SVM) += svm.o
obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
-obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
+obj-$(CONFIG_PSERIES_PLPKS) += plpks.o plpks-sysfs.o
obj-$(CONFIG_PPC_SECURE_BOOT) += plpks-secvar.o
obj-$(CONFIG_PSERIES_PLPKS_SED) += plpks_sed_ops.o
obj-$(CONFIG_SUSPEND) += suspend.o
diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
index f9e9cc40c9d0..a50ff6943d80 100644
--- a/arch/powerpc/platforms/pseries/plpks-secvar.c
+++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
@@ -20,33 +20,6 @@
#include <asm/secvar.h>
#include <asm/plpks.h>
-// Config attributes for sysfs
-#define PLPKS_CONFIG_ATTR(name, fmt, func) \
- static ssize_t name##_show(struct kobject *kobj, \
- struct kobj_attribute *attr, \
- char *buf) \
- { \
- return sysfs_emit(buf, fmt, func()); \
- } \
- static struct kobj_attribute attr_##name = __ATTR_RO(name)
-
-PLPKS_CONFIG_ATTR(version, "%u\n", plpks_get_version);
-PLPKS_CONFIG_ATTR(max_object_size, "%u\n", plpks_get_maxobjectsize);
-PLPKS_CONFIG_ATTR(total_size, "%u\n", plpks_get_totalsize);
-PLPKS_CONFIG_ATTR(used_space, "%u\n", plpks_get_usedspace);
-PLPKS_CONFIG_ATTR(supported_policies, "%08x\n", plpks_get_supportedpolicies);
-PLPKS_CONFIG_ATTR(signed_update_algorithms, "%016llx\n", plpks_get_signedupdatealgorithms);
-
-static const struct attribute *config_attrs[] = {
- &attr_version.attr,
- &attr_max_object_size.attr,
- &attr_total_size.attr,
- &attr_used_space.attr,
- &attr_supported_policies.attr,
- &attr_signed_update_algorithms.attr,
- NULL,
-};
-
static u32 get_policy(const char *name)
{
if ((strcmp(name, "db") == 0) ||
@@ -225,7 +198,6 @@ static const struct secvar_operations plpks_secvar_ops_static = {
.set = plpks_set_variable,
.format = plpks_secvar_format,
.max_size = plpks_max_size,
- .config_attrs = config_attrs,
.var_names = plpks_var_names_static,
};
@@ -234,7 +206,6 @@ static const struct secvar_operations plpks_secvar_ops_dynamic = {
.set = plpks_set_variable,
.format = plpks_secvar_format,
.max_size = plpks_max_size,
- .config_attrs = config_attrs,
.var_names = plpks_var_names_dynamic,
};
diff --git a/arch/powerpc/platforms/pseries/plpks-sysfs.c b/arch/powerpc/platforms/pseries/plpks-sysfs.c
new file mode 100644
index 000000000000..01d526185783
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks-sysfs.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
+ *
+ * This code exposes PLPKS config to user via sysfs
+ */
+
+#define pr_fmt(fmt) "plpks-sysfs: "fmt
+
+#include <linux/init.h>
+#include <linux/printk.h>
+#include <linux/types.h>
+#include <asm/machdep.h>
+#include <asm/plpks.h>
+
+/* config attributes for sysfs */
+#define PLPKS_CONFIG_ATTR(name, fmt, func) \
+ static ssize_t name##_show(struct kobject *kobj, \
+ struct kobj_attribute *attr, \
+ char *buf) \
+ { \
+ return sysfs_emit(buf, fmt, func()); \
+ } \
+ static struct kobj_attribute attr_##name = __ATTR_RO(name)
+
+PLPKS_CONFIG_ATTR(version, "%u\n", plpks_get_version);
+PLPKS_CONFIG_ATTR(max_object_size, "%u\n", plpks_get_maxobjectsize);
+PLPKS_CONFIG_ATTR(total_size, "%u\n", plpks_get_totalsize);
+PLPKS_CONFIG_ATTR(used_space, "%u\n", plpks_get_usedspace);
+PLPKS_CONFIG_ATTR(supported_policies, "%08x\n", plpks_get_supportedpolicies);
+PLPKS_CONFIG_ATTR(signed_update_algorithms, "%016llx\n",
+ plpks_get_signedupdatealgorithms);
+
+static const struct attribute *config_attrs[] = {
+ &attr_version.attr,
+ &attr_max_object_size.attr,
+ &attr_total_size.attr,
+ &attr_used_space.attr,
+ &attr_supported_policies.attr,
+ &attr_signed_update_algorithms.attr,
+ NULL,
+};
+
+static struct kobject *plpks_kobj, *plpks_config_kobj;
+
+int plpks_config_create_softlink(struct kobject *from)
+{
+ if (!plpks_config_kobj)
+ return -EINVAL;
+ return sysfs_create_link(from, plpks_config_kobj, "config");
+}
+
+static __init int plpks_sysfs_config(struct kobject *kobj)
+{
+ struct attribute_group config_group = {
+ .name = NULL,
+ .attrs = (struct attribute **)config_attrs,
+ };
+
+ return sysfs_create_group(kobj, &config_group);
+}
+
+static __init int plpks_sysfs_init(void)
+{
+ int rc;
+
+ if (!plpks_is_available())
+ return -ENODEV;
+
+ plpks_kobj = kobject_create_and_add("plpks", firmware_kobj);
+ if (!plpks_kobj) {
+ pr_err("Failed to create plpks kobj\n");
+ return -ENOMEM;
+ }
+
+ plpks_config_kobj = kobject_create_and_add("config", plpks_kobj);
+ if (!plpks_config_kobj) {
+ pr_err("Failed to create plpks config kobj\n");
+ kobject_put(plpks_kobj);
+ return -ENOMEM;
+ }
+
+ rc = plpks_sysfs_config(plpks_config_kobj);
+ if (rc) {
+ pr_err("Failed to create attribute group for plpks config\n");
+ kobject_put(plpks_config_kobj);
+ kobject_put(plpks_kobj);
+ return rc;
+ }
+
+ return 0;
+}
+
+machine_subsys_initcall(pseries, plpks_sysfs_init);
--
2.47.3
^ permalink raw reply related
* [PATCH v3 1/6] pseries/plpks: fix kernel-doc comment inconsistencies
From: Srish Srinivasan @ 2026-01-06 15:05 UTC (permalink / raw)
To: linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, nayna, rnsastry, linux-kernel, linux-security-module,
ssrish
In-Reply-To: <20260106150527.446525-1-ssrish@linux.ibm.com>
Fix issues with comments for all the applicable functions to be
consistent with kernel-doc format. Move them before the function
definition as opposed to the function prototype.
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
---
arch/powerpc/include/asm/plpks.h | 77 ------
arch/powerpc/platforms/pseries/plpks.c | 328 ++++++++++++++++++++++++-
2 files changed, 318 insertions(+), 87 deletions(-)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 7a84069759b0..f303922bf622 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -67,122 +67,45 @@ struct plpks_var_name_list {
struct plpks_var_name varlist[];
};
-/**
- * Updates the authenticated variable. It expects NULL as the component.
- */
int plpks_signed_update_var(struct plpks_var *var, u64 flags);
-/**
- * Writes the specified var and its data to PKS.
- * Any caller of PKS driver should present a valid component type for
- * their variable.
- */
int plpks_write_var(struct plpks_var var);
-/**
- * Removes the specified var and its data from PKS.
- */
int plpks_remove_var(char *component, u8 varos,
struct plpks_var_name vname);
-/**
- * Returns the data for the specified os variable.
- *
- * Caller must allocate a buffer in var->data with length in var->datalen.
- * If no buffer is provided, var->datalen will be populated with the object's
- * size.
- */
int plpks_read_os_var(struct plpks_var *var);
-/**
- * Returns the data for the specified firmware variable.
- *
- * Caller must allocate a buffer in var->data with length in var->datalen.
- * If no buffer is provided, var->datalen will be populated with the object's
- * size.
- */
int plpks_read_fw_var(struct plpks_var *var);
-/**
- * Returns the data for the specified bootloader variable.
- *
- * Caller must allocate a buffer in var->data with length in var->datalen.
- * If no buffer is provided, var->datalen will be populated with the object's
- * size.
- */
int plpks_read_bootloader_var(struct plpks_var *var);
-/**
- * Returns if PKS is available on this LPAR.
- */
bool plpks_is_available(void);
-/**
- * Returns version of the Platform KeyStore.
- */
u8 plpks_get_version(void);
-/**
- * Returns hypervisor storage overhead per object, not including the size of
- * the object or label. Only valid for config version >= 2
- */
u16 plpks_get_objoverhead(void);
-/**
- * Returns maximum password size. Must be >= 32 bytes
- */
u16 plpks_get_maxpwsize(void);
-/**
- * Returns maximum object size supported by Platform KeyStore.
- */
u16 plpks_get_maxobjectsize(void);
-/**
- * Returns maximum object label size supported by Platform KeyStore.
- */
u16 plpks_get_maxobjectlabelsize(void);
-/**
- * Returns total size of the configured Platform KeyStore.
- */
u32 plpks_get_totalsize(void);
-/**
- * Returns used space from the total size of the Platform KeyStore.
- */
u32 plpks_get_usedspace(void);
-/**
- * Returns bitmask of policies supported by the hypervisor.
- */
u32 plpks_get_supportedpolicies(void);
-/**
- * Returns maximum byte size of a single object supported by the hypervisor.
- * Only valid for config version >= 3
- */
u32 plpks_get_maxlargeobjectsize(void);
-/**
- * Returns bitmask of signature algorithms supported for signed updates.
- * Only valid for config version >= 3
- */
u64 plpks_get_signedupdatealgorithms(void);
-/**
- * Returns the length of the PLPKS password in bytes.
- */
u16 plpks_get_passwordlen(void);
-/**
- * Called in early init to retrieve and clear the PLPKS password from the DT.
- */
void plpks_early_init_devtree(void);
-/**
- * Populates the FDT with the PLPKS password to prepare for kexec.
- */
int plpks_populate_fdt(void *fdt);
#else // CONFIG_PSERIES_PLPKS
static inline bool plpks_is_available(void) { return false; }
diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index b1667ed05f98..03722fabf9c3 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -312,40 +312,107 @@ static int _plpks_get_config(void)
return rc;
}
+/**
+ * plpks_get_version() - Get the version of the PLPKS config structure.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads the PLPKS config structure version and saves it in a file local static
+ * version variable.
+ *
+ * Returns: On success the saved PLPKS config structure version is returned, 0
+ * if not.
+ */
u8 plpks_get_version(void)
{
return version;
}
+/**
+ * plpks_get_objoverhead() - Get the hypervisor storage overhead per object.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads the per object hypervisor storage overhead in bytes into the local
+ * static objoverhead variable, excluding the size of the object or the label.
+ * This value can be treated as valid only when the PLPKS config structure
+ * version >= 2.
+ *
+ * Returns: If PLPKS config structure version >= 2 then the storage overhead is
+ * returned, 0 otherwise.
+ */
u16 plpks_get_objoverhead(void)
{
return objoverhead;
}
+/**
+ * plpks_get_maxpwsize() - Get the maximum password size.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads the maximum password size and checks if it is 32 bytes at the least
+ * before storing it in the local static maxpwsize variable.
+ *
+ * Returns: On success the maximum password size is returned, 0 if not.
+ */
u16 plpks_get_maxpwsize(void)
{
return maxpwsize;
}
+/**
+ * plpks_get_maxobjectsize() - Get the maximum object size supported by the
+ * PLPKS.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads the maximum object size into the file local static maxobjsize variable.
+ *
+ * Returns: On success the maximum object size is returned, 0 if not.
+ */
u16 plpks_get_maxobjectsize(void)
{
return maxobjsize;
}
+/**
+ * plpks_get_maxobjectlabelsize() - Get the maximum object label size supported
+ * by the PLPKS.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads the maximum object label size into the local static maxobjlabelsize
+ * variable.
+ *
+ * Returns: On success the maximum object label size is returned, 0 if not.
+ */
u16 plpks_get_maxobjectlabelsize(void)
{
return maxobjlabelsize;
}
+/**
+ * plpks_get_totalsize() - Get the total size of the PLPKS that is configured.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads the total size of the PLPKS that is configured for the LPAR into the
+ * file local static totalsize variable.
+ *
+ * Returns: On success the total size of the PLPKS configured is returned, 0 if
+ * not.
+ */
u32 plpks_get_totalsize(void)
{
return totalsize;
}
+/**
+ * plpks_get_usedspace() - Get the used space from the total size of the PLPKS.
+ *
+ * Invoke the H_PKS_GET_CONFIG HCALL to refresh the latest value for the used
+ * space as this keeps changing with the creation and removal of objects in the
+ * PLPKS.
+ *
+ * Returns: On success the used space is returned, 0 if not.
+ */
u32 plpks_get_usedspace(void)
{
- // Unlike other config values, usedspace regularly changes as objects
- // are updated, so we need to refresh.
int rc = _plpks_get_config();
if (rc) {
pr_err("Couldn't get config, rc: %d\n", rc);
@@ -354,26 +421,84 @@ u32 plpks_get_usedspace(void)
return usedspace;
}
+/**
+ * plpks_get_supportedpolicies() - Get a bitmask of the policies supported by
+ * the hypervisor.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads a bitmask of the policies supported by the hypervisor into the file
+ * local static supportedpolicies variable.
+ *
+ * Returns: On success the bitmask of the policies supported by the hypervisor
+ * are returned, 0 if not.
+ */
u32 plpks_get_supportedpolicies(void)
{
return supportedpolicies;
}
+/**
+ * plpks_get_maxlargeobjectsize() - Get the maximum object size supported for
+ * PLPKS config structure version >= 3
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads the maximum object size into the local static maxlargeobjectsize
+ * variable for PLPKS config structure version >= 3. This was introduced
+ * starting with PLPKS config structure version 3 to allow for objects of
+ * size >= 64K.
+ *
+ * Returns: If PLPKS config structure version >= 3 then the new maximum object
+ * size is returned, 0 if not.
+ */
u32 plpks_get_maxlargeobjectsize(void)
{
return maxlargeobjectsize;
}
+/**
+ * plpks_get_signedupdatealgorithms() - Get a bitmask of the signature
+ * algorithms supported for signed updates.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * reads a bitmask of the signature algorithms supported for signed updates into
+ * the file local static signedupdatealgorithms variable. This is valid only
+ * when the PLPKS config structure version >= 3.
+ *
+ * Returns: On success the bitmask of the signature algorithms supported for
+ * signed updates is returned, 0 if not.
+ */
u64 plpks_get_signedupdatealgorithms(void)
{
return signedupdatealgorithms;
}
+/**
+ * plpks_get_passwordlen() - Get the length of the PLPKS password in bytes.
+ *
+ * The H_PKS_GEN_PASSWORD HCALL makes the hypervisor generate a random password
+ * for the specified consumer, apply that password to the PLPKS and return it to
+ * the caller. In this process, the password length for the OS consumer is
+ * stored in the local static ospasswordlength variable.
+ *
+ * Returns: On success the password length for the OS consumer in bytes is
+ * returned, 0 if not.
+ */
u16 plpks_get_passwordlen(void)
{
return ospasswordlength;
}
+/**
+ * plpks_is_available() - Get the PLPKS availability status for the LPAR.
+ *
+ * The availability of PLPKS is inferred based upon the successful execution of
+ * the H_PKS_GET_CONFIG HCALL provided the firmware supports this feature. The
+ * H_PKS_GET_CONFIG HCALL reads the configuration and status information related
+ * to the PLPKS. The configuration structure provides a version number to inform
+ * the caller of the supported features.
+ *
+ * Returns: true is returned if PLPKS is available, false if not.
+ */
bool plpks_is_available(void)
{
int rc;
@@ -425,6 +550,35 @@ static int plpks_confirm_object_flushed(struct label *label,
return pseries_status_to_err(rc);
}
+/**
+ * plpks_signed_update_var() - Update the specified authenticated variable.
+ * @var: authenticated variable to be updated
+ * @flags: signed update request operation flags
+ *
+ * The H_PKS_SIGNED_UPDATE HCALL performs a signed update to an object in the
+ * PLPKS. The object must have the signed update policy flag set.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if PLPKS modification is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid object label parameter
+ * if invalid object label len parameter
+ * if invalid or unsupported policy declaration
+ * if invalid signed update flags
+ * if invalid input data parameter
+ * if invalid input data len parameter
+ * if invalid continue token parameter
+ * -EPERM if access is denied
+ * -ENOMEM if there is inadequate memory to perform the operation
+ * -EBUSY if unable to handle the request or long running operation
+ * initiated, retry later
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
int plpks_signed_update_var(struct plpks_var *var, u64 flags)
{
unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
@@ -481,6 +635,33 @@ int plpks_signed_update_var(struct plpks_var *var, u64 flags)
return rc;
}
+/**
+ * plpks_write_var() - Write the specified variable and its data to PLPKS.
+ * @var: variable to be written into the PLPKS
+ *
+ * The H_PKS_WRITE_OBJECT HCALL writes an object into the PLPKS. The caller must
+ * provide a valid component type for the variable, and the signed update policy
+ * flag must not be set.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if PLPKS modification is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid object label parameter
+ * if invalid object label len parameter
+ * if invalid or unsupported policy declaration
+ * if invalid input data parameter
+ * if invalid input data len parameter
+ * -EPERM if access is denied
+ * -ENOMEM if unable to store the requested object in the space available
+ * -EBUSY if unable to handle the request
+ * -EEXIST if the object label already exists
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
int plpks_write_var(struct plpks_var var)
{
unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
@@ -520,6 +701,30 @@ int plpks_write_var(struct plpks_var var)
return rc;
}
+/**
+ * plpks_remove_var() - Remove the specified variable and its data from PLPKS.
+ * @component: metadata prefix in the object label metadata structure
+ * @varos: metadata OS flags in the object label metadata structure
+ * @vname: object label for the object that needs to be removed
+ *
+ * The H_PKS_REMOVE_OBJECT HCALL removes an object from the PLPKS. The removal
+ * is independent of the policy bits that are set.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if PLPKS modification is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid object label parameter
+ * if invalid object label len parameter
+ * -EPERM if access is denied
+ * -ENOENT if the requested object was not found
+ * -EBUSY if unable to handle the request
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
int plpks_remove_var(char *component, u8 varos, struct plpks_var_name vname)
{
unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
@@ -619,21 +824,119 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var)
return rc;
}
+/**
+ * plpks_read_os_var() - Fetch the data for the specified variable that is
+ * owned by the OS consumer.
+ * @var: variable to be read from the PLPKS
+ *
+ * The consumer or the owner of the object is the os kernel. The
+ * H_PKS_READ_OBJECT HCALL reads an object from the PLPKS. The caller must
+ * allocate the buffer var->data and specify the length for this buffer in
+ * var->datalen. If no buffer is provided, var->datalen will be populated with
+ * the requested object's size.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid object label parameter
+ * if invalid object label len parameter
+ * if invalid output data parameter
+ * if invalid output data len parameter
+ * -EPERM if access is denied
+ * -ENOENT if the requested object was not found
+ * -EFBIG if the requested object couldn't be
+ * stored in the buffer provided
+ * -EBUSY if unable to handle the request
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
int plpks_read_os_var(struct plpks_var *var)
{
return plpks_read_var(PLPKS_OS_OWNER, var);
}
+/**
+ * plpks_read_fw_var() - Fetch the data for the specified variable that is
+ * owned by the firmware consumer.
+ * @var: variable to be read from the PLPKS
+ *
+ * The consumer or the owner of the object is the firmware. The
+ * H_PKS_READ_OBJECT HCALL reads an object from the PLPKS. The caller must
+ * allocate the buffer var->data and specify the length for this buffer in
+ * var->datalen. If no buffer is provided, var->datalen will be populated with
+ * the requested object's size.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid object label parameter
+ * if invalid object label len parameter
+ * if invalid output data parameter
+ * if invalid output data len parameter
+ * -EPERM if access is denied
+ * -ENOENT if the requested object was not found
+ * -EFBIG if the requested object couldn't be
+ * stored in the buffer provided
+ * -EBUSY if unable to handle the request
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
int plpks_read_fw_var(struct plpks_var *var)
{
return plpks_read_var(PLPKS_FW_OWNER, var);
}
+/**
+ * plpks_read_bootloader_var() - Fetch the data for the specified variable
+ * owned by the bootloader consumer.
+ * @var: variable to be read from the PLPKS
+ *
+ * The consumer or the owner of the object is the bootloader. The
+ * H_PKS_READ_OBJECT HCALL reads an object from the PLPKS. The caller must
+ * allocate the buffer var->data and specify the length for this buffer in
+ * var->datalen. If no buffer is provided, var->datalen will be populated with
+ * the requested object's size.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO if PLPKS is not supported
+ * -EIO if PLPKS access is blocked due to the LPAR's state
+ * if an error occurred while processing the request
+ * -EINVAL if invalid authorization parameter
+ * if invalid object label parameter
+ * if invalid object label len parameter
+ * if invalid output data parameter
+ * if invalid output data len parameter
+ * -EPERM if access is denied
+ * -ENOENT if the requested object was not found
+ * -EFBIG if the requested object couldn't be
+ * stored in the buffer provided
+ * -EBUSY if unable to handle the request
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
int plpks_read_bootloader_var(struct plpks_var *var)
{
return plpks_read_var(PLPKS_BOOTLOADER_OWNER, var);
}
+/**
+ * plpks_populate_fdt(): Populates the FDT with the PLPKS password to prepare
+ * for kexec.
+ * @fdt: pointer to the device tree blob
+ *
+ * Upon confirming the existence of the chosen node, invoke fdt_setprop to
+ * populate the device tree with the PLPKS password in order to prepare for
+ * kexec.
+ *
+ * Returns: On success 0 is returned, a negative value if not.
+ */
int plpks_populate_fdt(void *fdt)
{
int chosen_offset = fdt_path_offset(fdt, "/chosen");
@@ -647,14 +950,19 @@ int plpks_populate_fdt(void *fdt)
return fdt_setprop(fdt, chosen_offset, "ibm,plpks-pw", ospassword, ospasswordlength);
}
-// Once a password is registered with the hypervisor it cannot be cleared without
-// rebooting the LPAR, so to keep using the PLPKS across kexec boots we need to
-// recover the previous password from the FDT.
-//
-// There are a few challenges here. We don't want the password to be visible to
-// users, so we need to clear it from the FDT. This has to be done in early boot.
-// Clearing it from the FDT would make the FDT's checksum invalid, so we have to
-// manually cause the checksum to be recalculated.
+/**
+ * plpks_early_init_devtree() - Retrieves and clears the PLPKS password from the
+ * DT in early init.
+ *
+ * Once a password is registered with the hypervisor it cannot be cleared
+ * without rebooting the LPAR, so to keep using the PLPKS across kexec boots we
+ * need to recover the previous password from the FDT.
+ *
+ * There are a few challenges here. We don't want the password to be visible to
+ * users, so we need to clear it from the FDT. This has to be done in early
+ * boot. Clearing it from the FDT would make the FDT's checksum invalid, so we
+ * have to manually cause the checksum to be recalculated.
+ */
void __init plpks_early_init_devtree(void)
{
void *fdt = initial_boot_params;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 0/6] Extend "trusted" keys to support a new trust source named the PowerVM Key Wrapping Module (PKWM)
From: Srish Srinivasan @ 2026-01-06 15:05 UTC (permalink / raw)
To: linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, nayna, rnsastry, linux-kernel, linux-security-module,
ssrish
Power11 has introduced a feature called the PowerVM Key Wrapping Module
(PKWM), where PowerVM in combination with Power LPAR Platform KeyStore
(PLPKS) [1] supports a new feature called "Key Wrapping" [2] to protect
user secrets by wrapping them using a hypervisor generated wrapping key.
This wrapping key is an AES-GCM-256 symmetric key that is stored as an
object in the PLPKS. It has policy based protections that prevents it from
being read out or exposed to the user. This wrapping key can then be used
by the OS to wrap or unwrap secrets via hypervisor calls.
This patchset intends to add the PKWM, which is a combination of IBM
PowerVM and PLPKS, as a new trust source for trusted keys. The wrapping key
does not exist by default and its generation is requested by the kernel at
the time of PKWM initialization. This key is then persisted by the PKWM and
is used for wrapping any kernel provided key, and is never exposed to the
user. The kernel is aware of only the label to this wrapping key.
Along with the PKWM implementation, this patchset includes two preparatory
patches: one fixing the kernel-doc inconsistencies in the PLPKS code and
another reorganizing PLPKS config variables in the sysfs.
Changelog:
v3:
* Patch 2:
- Add Mimi's Reviewed-by tag
* Patch 4:
- Minor tweaks to some print statements
- Fix typos
* Patch 5:
- Fix typos
- Add Mimi's Reviewed-by tag
* Patch 6:
- Add Mimi's Reviewed-by tag
v2:
* Patch 2:
- Fix build warning detected by the kernel test bot
* Patch 5:
- Use pr_debug inside dump_options
- Replace policyhande with wrap_flags inside dump_options
- Provide meaningful error messages with error codes
Nayna Jain (1):
docs: trusted-encryped: add PKWM as a new trust source
Srish Srinivasan (5):
pseries/plpks: fix kernel-doc comment inconsistencies
powerpc/pseries: move the PLPKS config inside its own sysfs directory
pseries/plpks: expose PowerVM wrapping features via the sysfs
pseries/plpks: add HCALLs for PowerVM Key Wrapping Module
keys/trusted_keys: establish PKWM as a trusted source
.../ABI/testing/sysfs-firmware-plpks | 58 ++
Documentation/ABI/testing/sysfs-secvar | 65 --
.../admin-guide/kernel-parameters.txt | 1 +
Documentation/arch/powerpc/papr_hcalls.rst | 43 ++
.../security/keys/trusted-encrypted.rst | 50 ++
MAINTAINERS | 9 +
arch/powerpc/include/asm/hvcall.h | 4 +-
arch/powerpc/include/asm/plpks.h | 95 +--
arch/powerpc/include/asm/secvar.h | 1 -
arch/powerpc/kernel/secvar-sysfs.c | 21 +-
arch/powerpc/platforms/pseries/Makefile | 2 +-
arch/powerpc/platforms/pseries/plpks-secvar.c | 29 -
arch/powerpc/platforms/pseries/plpks-sysfs.c | 96 +++
arch/powerpc/platforms/pseries/plpks.c | 686 +++++++++++++++++-
include/keys/trusted-type.h | 7 +-
include/keys/trusted_pkwm.h | 22 +
security/keys/trusted-keys/Kconfig | 8 +
security/keys/trusted-keys/Makefile | 2 +
security/keys/trusted-keys/trusted_core.c | 6 +-
security/keys/trusted-keys/trusted_pkwm.c | 168 +++++
20 files changed, 1172 insertions(+), 201 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-plpks
create mode 100644 arch/powerpc/platforms/pseries/plpks-sysfs.c
create mode 100644 include/keys/trusted_pkwm.h
create mode 100644 security/keys/trusted-keys/trusted_pkwm.c
--
2.47.3
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sudeep Holla @ 2026-01-06 13:40 UTC (permalink / raw)
To: Jens Wiklander
Cc: Alexandre Belloni, Sudeep Holla, Uwe Kleine-König,
Jonathan Corbet, Sumit Garg, Olivia Mackall, Herbert Xu,
Clément Léger, Ard Biesheuvel, Maxime Coquelin,
Alexandre Torgue, Sumit Garg, Ilias Apalodimas, Jan Kiszka,
Christophe JAILLET, Rafał Miłecki, Michael Chan,
Pavan Chebbi, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
Peter Huewe, op-tee, linux-kernel, linux-doc, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, linux-mips, netdev, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <CAHUa44HqRbCJTXsrTCm0G5iwtkQtq+Si=yOspCjpAn-N2uVSVg@mail.gmail.com>
On Mon, Jan 05, 2026 at 10:16:09AM +0100, Jens Wiklander wrote:
> Hi,
>
> On Thu, Dec 18, 2025 at 5:29 PM Jens Wiklander
> <jens.wiklander@linaro.org> wrote:
> >
> > On Thu, Dec 18, 2025 at 2:53 PM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> > >
> > > On 18/12/2025 08:21:27+0100, Jens Wiklander wrote:
> > > > Hi,
> > > >
> > > > On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
> > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > the objective of this series is to make tee driver stop using callbacks
> > > > > in struct device_driver. These were superseded by bus methods in 2006
> > > > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > > > methods.")) but nobody cared to convert all subsystems accordingly.
> > > > >
> > > > > Here the tee drivers are converted. The first commit is somewhat
> > > > > unrelated, but simplifies the conversion (and the drivers). It
> > > > > introduces driver registration helpers that care about setting the bus
> > > > > and owner. (The latter is missing in all drivers, so by using these
> > > > > helpers the drivers become more correct.)
> > > > >
> > > > > v1 of this series is available at
> > > > > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> > > > >
> > > > > Changes since v1:
> > > > >
> > > > > - rebase to v6.19-rc1 (no conflicts)
> > > > > - add tags received so far
> > > > > - fix whitespace issues pointed out by Sumit Garg
> > > > > - fix shutdown callback to shutdown and not remove
> > > > >
> > > > > As already noted in v1's cover letter, this series should go in during a
> > > > > single merge window as there are runtime warnings when the series is
> > > > > only applied partially. Sumit Garg suggested to apply the whole series
> > > > > via Jens Wiklander's tree.
> > > > > If this is done the dependencies in this series are honored, in case the
> > > > > plan changes: Patches #4 - #17 depend on the first two.
> > > > >
> > > > > Note this series is only build tested.
> > > > >
> > > > > Uwe Kleine-König (17):
> > > > > tee: Add some helpers to reduce boilerplate for tee client drivers
> > > > > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > > > > tee: Adapt documentation to cover recent additions
> > > > > hwrng: optee - Make use of module_tee_client_driver()
> > > > > hwrng: optee - Make use of tee bus methods
> > > > > rtc: optee: Migrate to use tee specific driver registration function
> > > > > rtc: optee: Make use of tee bus methods
> > > > > efi: stmm: Make use of module_tee_client_driver()
> > > > > efi: stmm: Make use of tee bus methods
> > > > > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > > > > firmware: arm_scmi: Make use of tee bus methods
> > > > > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > > > > firmware: tee_bnxt: Make use of tee bus methods
> > > > > KEYS: trusted: Migrate to use tee specific driver registration
> > > > > function
> > > > > KEYS: trusted: Make use of tee bus methods
> > > > > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > > > > tpm/tpm_ftpm_tee: Make use of tee bus methods
> > > > >
> > > > > Documentation/driver-api/tee.rst | 18 +----
> > > > > drivers/char/hw_random/optee-rng.c | 26 ++----
> > > > > drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> > > > > drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> > > > > drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> > > > > drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> > > > > drivers/rtc/rtc-optee.c | 27 ++-----
> > > > > drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> > > > > include/linux/tee_drv.h | 12 +++
> > > > > security/keys/trusted-keys/trusted_tee.c | 17 ++--
> > > > > 10 files changed, 164 insertions(+), 138 deletions(-)
> > > > >
> > > > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > > > --
> > > > > 2.47.3
> > > > >
> > > >
> > > > Thank you for the nice cleanup, Uwe.
> > > >
> > > > I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
> > > > tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
> > > >
> > > > The branch is based on v6.19-rc1, and I'll try to keep it stable for
> > > > others to depend on, if needed. Let's see if we can agree on taking
> > > > the remaining patches via that branch.
> > >
> > > 6 and 7 can go through your branch.
> >
> > Good, I've added them to my branch now.
>
> This entire patch set should go in during a single merge window. I
> will not send any pull request until I'm sure all patches will be
> merged.
>
> So far (if I'm not mistaken), only the patches I've already added to
> next have appeared next. I can take the rest of the patches, too, but
> I need OK for the following:
>
[...]
>
> Sudeep, you seem happy with the following patches
> - firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> - firmware: arm_scmi: Make use of tee bus methods
> OK if I take them via my tree, or would you rather take them yourself?
>
I am happy if you want to take all of them in one go. I think I have
already acked it. Please shout if you need anything else from me, happy to
help in anyway to make it easier to handle this change set.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Jeff Layton @ 2026-01-06 12:01 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com>
On Mon, 2025-12-29 at 11:52 -0600, Frederick Lawler wrote:
> Since commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps"), IMA
> is no longer able to correctly track inode.i_version due to the struct
> kstat.change_cookie no longer containing an updated i_version.
>
> Introduce a fallback mechanism for IMA that instead tracks a
> integrity_ctime_guard() in absence of or outdated i_version
> for stacked file systems.
>
> EVM is left alone since it mostly cares about the backing inode.
>
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> The motivation behind this was that file systems that use the
> cookie to set the i_version for stacked file systems may still do so.
> Then add in the ctime_guard as a fallback if there's a detected change.
> The assumption is that the ctime will be different if the i_version is
> different anyway for non-stacked file systems.
>
> I'm not too pleased with passing in struct file* to
> integrity_inode_attrs_changed() since EVM doesn't currently use
> that for now, but I couldn't come up with another idea to get the
> stat without coming up with a new stat function to accommodate just
> the file path, fully separate out IMA/EVM checks, or lastly add stacked
> file system support to EVM (which doesn't make much sense to me
> at the moment).
>
> I plan on adding in self test infrastructure for the v1, but I would
> like to get some early feedback on the approach first.
> ---
> include/linux/integrity.h | 29 ++++++++++++++++++++++++-----
> security/integrity/evm/evm_crypto.c | 2 +-
> security/integrity/evm/evm_main.c | 2 +-
> security/integrity/ima/ima_api.c | 21 +++++++++++++++------
> security/integrity/ima/ima_main.c | 17 ++++++++++-------
> 5 files changed, 51 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index f5842372359be5341b6870a43b92e695e8fc78af..4964c0f2bbda0ca450d135b9b738bc92256c375a 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -31,19 +31,27 @@ static inline void integrity_load_keys(void)
>
> /* An inode's attributes for detection of changes */
> struct integrity_inode_attributes {
> + u64 ctime_guard;
> u64 version; /* track inode changes */
> unsigned long ino;
> dev_t dev;
> };
>
> +static inline u64 integrity_ctime_guard(struct kstat stat)
> +{
> + return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> +}
> +
> /*
> * On stacked filesystems the i_version alone is not enough to detect file data
> * or metadata change. Additional metadata is required.
> */
> static inline void
> integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> - u64 i_version, const struct inode *inode)
> + u64 i_version, u64 ctime_guard,
> + const struct inode *inode)
> {
> + attrs->ctime_guard = ctime_guard;
> attrs->version = i_version;
> attrs->dev = inode->i_sb->s_dev;
> attrs->ino = inode->i_ino;
> @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> */
> static inline bool
> integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> - const struct inode *inode)
> + struct file *file, struct inode *inode)
> {
> - return (inode->i_sb->s_dev != attrs->dev ||
> - inode->i_ino != attrs->ino ||
> - !inode_eq_iversion(inode, attrs->version));
> + struct kstat stat;
> +
> + if (inode->i_sb->s_dev != attrs->dev ||
> + inode->i_ino != attrs->ino)
> + return true;
> +
> + if (inode_eq_iversion(inode, attrs->version))
> + return false;
> +
> + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT))
> + return true;
> +
This is rather odd. You're sampling the i_version field directly, but
if it's not equal then you go through ->getattr() to get the ctime.
It's particularly odd since you don't know whether the i_version field
is even implemented on the fs. On filesystems where it isn't, the
i_version field generally stays at 0, so won't this never fall through
to do the vfs_getattr_nosec() call on those filesystems?
Ideally, you should just call vfs_getattr_nosec() early on with
STATX_CHANGE_COOKIE|STATX_CTIME to get both at once, and only trust
STATX_CHANGE_COOKIE if it's set in the returned mask.
> + return attrs->ctime_guard != integrity_ctime_guard(stat);
> }
>
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index a5e730ffda57fbc0a91124adaa77b946a12d08b4..2d89c0e8d9360253f8dad52d2a8168127bb4d3b8 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -300,7 +300,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> if (IS_I_VERSION(inode))
> i_version = inode_query_iversion(inode);
> integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> - inode);
> + 0, inode);
> }
>
> /* Portable EVM signatures must include an IMA hash */
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..0712802628fd6533383f9855687e19bef7b771c7 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -754,7 +754,7 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> if (iint) {
> ret = (!IS_I_VERSION(metadata_inode) ||
> integrity_inode_attrs_changed(&iint->metadata_inode,
> - metadata_inode));
> + NULL, metadata_inode));
> if (ret)
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8d404ba4886e3b736c3bab29d1668..72bba8daa588a0f4e45e4249276edb54ca3d77ef 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> int length;
> void *tmpbuf;
> u64 i_version = 0;
> + u64 ctime_guard = 0;
>
> /*
> * Always collect the modsig, because IMA might have already collected
> @@ -272,10 +273,16 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> +
> + if (stat.result_mask & STATX_CTIME)
> + ctime_guard = integrity_ctime_guard(stat);
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> @@ -305,11 +312,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>
> iint->ima_hash = tmpbuf;
> memcpy(iint->ima_hash, &hash, length);
> - if (real_inode == inode)
> + if (real_inode == inode) {
> iint->real_inode.version = i_version;
> - else
> + iint->real_inode.ctime_guard = ctime_guard;
> + } else {
> integrity_inode_attrs_store(&iint->real_inode, i_version,
> - real_inode);
> + ctime_guard, real_inode);
> + }
>
> /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> if (!result)
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5770cf691912aa912fc65280c59f5baac35dd725..6051ea4a472fc0b0dd7b4e81da36eff8bd048c62 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -22,6 +22,7 @@
> #include <linux/mount.h>
> #include <linux/mman.h>
> #include <linux/slab.h>
> +#include <linux/stat.h>
> #include <linux/xattr.h>
> #include <linux/ima.h>
> #include <linux/fs.h>
> @@ -185,6 +186,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> {
> fmode_t mode = file->f_mode;
> bool update;
> + int ret;
>
> if (!(mode & FMODE_WRITE))
> return;
> @@ -197,12 +199,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
>
> update = test_and_clear_bit(IMA_UPDATE_XATTR,
> &iint->atomic_flags);
> - if ((iint->flags & IMA_NEW_FILE) ||
> - vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> - AT_STATX_SYNC_AS_STAT) ||
> - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> - stat.change_cookie != iint->real_inode.version) {
> + ret = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT);
> + if ((iint->flags & IMA_NEW_FILE) || ret ||
> + (!ret && stat.change_cookie != iint->real_inode.version) ||
> + (!ret && integrity_ctime_guard(stat) !=
> + iint->real_inode.ctime_guard)) {
> iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> iint->measured_pcrs = 0;
> if (update)
> @@ -330,7 +333,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> if (!IS_I_VERSION(real_inode) ||
> integrity_inode_attrs_changed(&iint->real_inode,
> - real_inode)) {
> + file, real_inode)) {
> iint->flags &= ~IMA_DONE_MASK;
> iint->measured_pcrs = 0;
> }
>
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251212-xfs-ima-fixup-931780a62c2c
>
> Best regards,
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jon Hunter @ 2026-01-06 9:39 UTC (permalink / raw)
To: Uwe Kleine-König, Jens Wiklander, Jonathan Corbet,
Sumit Garg, Olivia Mackall, Herbert Xu, Clément Léger,
Alexandre Belloni, Ard Biesheuvel, Maxime Coquelin,
Alexandre Torgue, Sumit Garg, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Rafał Miłecki,
Michael Chan, Pavan Chebbi, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe
Cc: op-tee, linux-kernel, linux-doc, linux-crypto, linux-rtc,
linux-efi, linux-stm32, linux-arm-kernel, Cristian Marussi,
arm-scmi, linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe,
linux-tegra@vger.kernel.org
In-Reply-To: <cover.1765791463.git.u.kleine-koenig@baylibre.com>
Hi Uwe,
On 15/12/2025 14:16, Uwe Kleine-König wrote:
> Hello,
>
> the objective of this series is to make tee driver stop using callbacks
> in struct device_driver. These were superseded by bus methods in 2006
> (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> methods.")) but nobody cared to convert all subsystems accordingly.
>
> Here the tee drivers are converted. The first commit is somewhat
> unrelated, but simplifies the conversion (and the drivers). It
> introduces driver registration helpers that care about setting the bus
> and owner. (The latter is missing in all drivers, so by using these
> helpers the drivers become more correct.)
>
> v1 of this series is available at
> https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
>
> Changes since v1:
>
> - rebase to v6.19-rc1 (no conflicts)
> - add tags received so far
> - fix whitespace issues pointed out by Sumit Garg
> - fix shutdown callback to shutdown and not remove
>
> As already noted in v1's cover letter, this series should go in during a
> single merge window as there are runtime warnings when the series is
> only applied partially. Sumit Garg suggested to apply the whole series
> via Jens Wiklander's tree.
> If this is done the dependencies in this series are honored, in case the
> plan changes: Patches #4 - #17 depend on the first two.
>
> Note this series is only build tested.
>
> Uwe Kleine-König (17):
> tee: Add some helpers to reduce boilerplate for tee client drivers
> tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> tee: Adapt documentation to cover recent additions
> hwrng: optee - Make use of module_tee_client_driver()
> hwrng: optee - Make use of tee bus methods
> rtc: optee: Migrate to use tee specific driver registration function
> rtc: optee: Make use of tee bus methods
> efi: stmm: Make use of module_tee_client_driver()
> efi: stmm: Make use of tee bus methods
> firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> firmware: arm_scmi: Make use of tee bus methods
> firmware: tee_bnxt: Make use of module_tee_client_driver()
> firmware: tee_bnxt: Make use of tee bus methods
> KEYS: trusted: Migrate to use tee specific driver registration
> function
> KEYS: trusted: Make use of tee bus methods
> tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> tpm/tpm_ftpm_tee: Make use of tee bus methods
On the next-20260105 I am seeing the following warnings ...
WARNING KERN Driver 'optee-rng' needs updating - please use bus_type methods
WARNING KERN Driver 'scmi-optee' needs updating - please use bus_type methods
WARNING KERN Driver 'tee_bnxt_fw' needs updating - please use bus_type methods
I bisected the first warning and this point to the following
commit ...
# first bad commit: [a707eda330b932bcf698be9460e54e2f389e24b7] tee: Add some helpers to reduce boilerplate for tee client drivers
I have not bisected the others, but guess they are related
to this series. Do you observe the same?
Thanks
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Herbert Xu @ 2026-01-06 5:04 UTC (permalink / raw)
To: Jens Wiklander
Cc: Alexandre Belloni, Uwe Kleine-König, Jonathan Corbet,
Sumit Garg, Olivia Mackall, Clément Léger,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Sudeep Holla, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <CAHUa44HqRbCJTXsrTCm0G5iwtkQtq+Si=yOspCjpAn-N2uVSVg@mail.gmail.com>
On Mon, Jan 05, 2026 at 10:16:09AM +0100, Jens Wiklander wrote:
>
> Herbert, you seem happy with the following patches
> - hwrng: optee - Make use of module_tee_client_driver()
> - hwrng: optee - Make use of tee bus methods
> OK if I take them via my tree, or would you rather take them yourself?
Feel free to take them through your tree.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ 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