From f92d0ccb05bf9089072cb067dda0dffd5394c4ef Mon Sep 17 00:00:00 2001 From: Srish Srinivasan Date: Sat, 12 Sep 2026 11:59:49 +0530 Subject: [PATCH 01/13] keys/trusted_keys: return immediately after TPM unseal failure trusted_tpm_unseal() proceeds to pcrlock() when the TPM unseal operation fails. If pcrlock() succeeds, its return value overwrites the unseal error, causing key instantiation to succeed. Return immediately when unseal fails to preserve the original error. Cc: stable@vger.kernel.org # v5.15+ Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework") Signed-off-by: Srish Srinivasan Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/20260912062950.279104-2-ssrish@linux.ibm.com Signed-off-by: Jarkko Sakkinen --- security/keys/trusted-keys/trusted_tpm1.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c index bf0bf7f36970..9cdfeea800a3 100644 --- a/security/keys/trusted-keys/trusted_tpm1.c +++ b/security/keys/trusted-keys/trusted_tpm1.c @@ -923,8 +923,10 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob) ret = tpm2_unseal_trusted(chip, p, options); else ret = key_unseal(p, options); - if (ret < 0) + if (ret < 0) { pr_info("key_unseal failed (%d)\n", ret); + goto out; + } if (options->pcrlock) { ret = pcrlock(options->pcrlock); From 11077cbcc2695f71c944a84a5f08c6672ab759f1 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Wed, 23 Sep 2026 19:35:06 +0200 Subject: [PATCH 02/13] KEYS: trusted: Fix blob allocation size in tpm2_key_decode() tpm2_key_decode() allocates 4 bytes more than needed. The ASN.1 callbacks tpm2_key_priv() and tpm2_key_pub() provide the lengths of TPM2B_PRIVATE and TPM2B_PUBLIC, so ctx.priv_len and ctx.pub_len already account for the 2-byte `size` field each of those structures starts with. I noticed this while reviewing commit 114f00d738f1 ("KEYS: trusted: Fix tpm2_load_cmd() boundary check"), which correctly reports ctx.priv_len + ctx.pub_len as the decoded blob size [1]. Let's allocate exactly that amount, matching the data copied into the blob. [1] https://lore.kernel.org/linux-integrity/apfoKo-BdwaLXtkT@sgarzare-redhat/ Cc: stable@vger.kernel.org # v5.13+ Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs") Signed-off-by: Stefano Garzarella Link: https://lore.kernel.org/r/20260923173506.41519-1-sgarzare@redhat.com Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- security/keys/trusted-keys/trusted_tpm2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index 01f18bb37047..a9b8a31a637c 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -115,7 +115,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload, if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE) return -EINVAL; - blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL); + blob = kmalloc(ctx.priv_len + ctx.pub_len, GFP_KERNEL); if (!blob) return -ENOMEM; From 0b157166e612b70f98a332b6c7be7eac6f8e9593 Mon Sep 17 00:00:00 2001 From: Adrian Martinez Date: Sat, 10 Oct 2026 23:40:00 +0300 Subject: [PATCH 03/13] KEYS: Fix key_reject_and_link() race with keyring restriction The restrict_link check in key_reject_and_link() is done before taking the keyring semaphore, racing with a concurrent keyring_restrict() that installs the restriction while holding the semaphore in write mode. Move the check to after __key_link_lock(), so it is done under the semaphore, matching the pattern used in __key_create_or_update(). Cc: stable@vger.kernel.org Fixes: 5ac7eace2d00 ("KEYS: Add a facility to restrict new links into a keyring") Reviewed-by: Jarkko Sakkinen Signed-off-by: Adrian Martinez Link: https://lore.kernel.org/r/CACO1+RT+m==BY+ZPkE+fo536D0qu3wog4xE1dfuXQ56g+=Qv=A@mail.gmail.com Signed-off-by: Jarkko Sakkinen --- security/keys/key.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/security/keys/key.c b/security/keys/key.c index a438c4508595..6ebb8691551f 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -588,11 +588,18 @@ int key_reject_and_link(struct key *key, ret = -EBUSY; if (keyring) { - if (keyring->restrict_link) - return -EPERM; - link_ret = __key_link_lock(keyring, &key->index_key); if (link_ret == 0) { + /* + * Check the restriction under the keyring semaphore. + * keyring_restrict() installs it while holding the + * semaphore, so testing it beforehand races with a + * concurrent restriction install. + */ + if (keyring->restrict_link) { + __key_link_end(keyring, &key->index_key, edit); + return -EPERM; + } link_ret = __key_link_begin(keyring, &key->index_key, &edit); if (link_ret < 0) __key_link_end(keyring, &key->index_key, edit); From fced7238b7e80cf8aa688d5af976e4aa6e33c329 Mon Sep 17 00:00:00 2001 From: Cen Zhang Date: Fri, 9 Oct 2026 13:11:03 +0800 Subject: [PATCH 04/13] keys: Protect the type name while describing a key keyctl_describe_key() must keep the key type's name alive until kasprintf() has consumed it. The retained key reference keeps the key and its description alive, but does not pin the module providing its type. The name is passed to kasprintf() without holding key->sem. kvasprintf() uses the saved name pointer in two formatting passes, with a GFP_KERNEL allocation between them. With AF_RXRPC=m, a task that passes View permission before type retirement can overlap a privileged module unload in this order: 1. KEYCTL_DESCRIBE looks up the key, saves key->type->name in the kasprintf() arguments and completes the first formatting pass. 2. While the formatter is delayed before its second pass, af_rxrpc_exit() calls unregister_key_type(), which removes the type from the registry and waits for key_gc_keytype(). 3. The collector takes key->sem for writing, retypes the retained key to key_type_dead and completes retirement. Unregistration and module exit return, allowing free_module() to release the type and name. 4. The formatter resumes its second pass and reads the saved name from released module storage, which can fault in string(). Hold key->sem for reading across kasprintf(). The collector then cannot retype the key or complete retirement until both formatting passes have finished. If retirement wins the lock first, formatting uses the core key_type_dead name instead. Release the semaphore immediately after kasprintf(), including on allocation failure, before copying to userspace. Oops report as below: BUG: unable to handle page fault for address: fffffbfff8054794 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page RIP: 0010:string+0x23f/0x470 Call Trace: vsnprintf+0x330/0x1110 Cc: stable@vger.kernel.org Fixes: aa9d4437893f ("KEYS: Fix the size of the key description passed to/from userspace") Assisted-by: LLM Signed-off-by: Cen Zhang Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/pm-key-management-objects-candidate-0009-v4-6b537c5732fd957c33fa@gmail.com Signed-off-by: Jarkko Sakkinen --- security/keys/keyctl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index d14ace88e529..5a5efa26ed0e 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -677,12 +677,14 @@ long keyctl_describe_key(key_serial_t keyid, /* calculate how much information we're going to return */ ret = -ENOMEM; + down_read(&key->sem); infobuf = kasprintf(GFP_KERNEL, "%s;%d;%d;%08x;", key->type->name, from_kuid_munged(current_user_ns(), key->uid), from_kgid_munged(current_user_ns(), key->gid), key->perm); + up_read(&key->sem); if (!infobuf) goto error2; infolen = strlen(infobuf); From 51409aaa01f8602552c336009fd22b226b0cdf1d Mon Sep 17 00:00:00 2001 From: Cen Zhang Date: Fri, 9 Oct 2026 13:11:59 +0800 Subject: [PATCH 05/13] keys: Avoid the owner account dereference in named keyring lookup Named keyring lookup must keep the storage containing the owner UID alive through the namespace mapping check. find_keyring_by_name() reads keyring->user->uid under keyring_name_lock, but that lock protects the keyring's name entry and allocation, not its separate key_user. A named session-keyring join can overlap a privileged KEYCTL_CHOWN on another CPU. If the keyring holds the last reference to its old dynamic key_user and the ownership transfer succeeds, the following ordering is possible: Named join Chown find_keyring_by_name() keyctl_chown_key() read_lock(keyring_name_lock) down_write(key->sem) load old keyring->user replace key->user and key->uid up_write(key->sem) key_put(key) key_user_put(old user): free read old user->uid read_unlock(keyring_name_lock) Chown neither takes keyring_name_lock nor key_session_mutex, so it can free the old account between the pointer load and the UID read. The lookup then reads freed memory even though the keyring itself is alive. Use keyring->uid for the mapping check. key_alloc() initializes this inline owner UID and keyctl_chown_key() updates it on chown. The existing name lock keeps its containing keyring allocated throughout the check, so lookup no longer depends on the account's lifetime. This also matches the owner UID used by the permission check. KASAN report as below: BUG: KASAN: slab-use-after-free in find_keyring_by_name+0x577/0x5d0 Read of size 4 at addr ffff8881128ae2ec by task keycase/500 Cc: stable@vger.kernel.org Fixes: 2ea190d0a006 ("keys: skip keys from another user namespace") Assisted-by: LLM Signed-off-by: Cen Zhang Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/pm-key-management-objects-candidate-0002-v3-2baa8cad2bb554364e10@gmail.com Signed-off-by: Jarkko Sakkinen --- security/keys/keyring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/keys/keyring.c b/security/keys/keyring.c index 15bf4af8f282..46f774be7296 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -1158,7 +1158,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring) * grants Search permission and that hasn't been revoked */ list_for_each_entry(keyring, &ns->keyring_name_list, name_link) { - if (!kuid_has_mapping(ns, keyring->user->uid)) + if (!kuid_has_mapping(ns, keyring->uid)) continue; if (test_bit(KEY_FLAG_REVOKED, &keyring->flags)) From 0a0ec45bf577a9b4d4960567f20d3877ef480de9 Mon Sep 17 00:00:00 2001 From: Cen Zhang Date: Fri, 9 Oct 2026 13:52:16 +0800 Subject: [PATCH 06/13] keys: Check the authorization payload in process keyring searches An authorization payload and its saved credentials must remain valid while search_process_keyrings_rcu() searches the requestor's keyrings. The function validates the authorization key, then separately loads payload.data[0] and unconditionally reads rka->cred. Validation does not prevent request_key_auth_revoke() from clearing the payload. A request-key helper can assume authority and fork a descendant that inherits the authorization key. If the descendant misses in its own keyrings, it falls back to the saved requestor credentials. Successful target instantiation invalidates the authorization key without clearing the payload. After the helper exits, UMH_WAIT_PROC returns to the requester, which calls complete_request_key() and revokes that key. With the descendant paused after validation, the paths can interleave as follows: 1. The descendant enters search_process_keyrings_rcu() under RCU and key_validate() succeeds. 2. The helper instantiates the target, invalidates the authorization key, and exits. 3. The requester resumes from UMH_WAIT_PROC, calls complete_request_key(), and revokes the authorization key. request_key_auth_revoke() sets its payload pointer to NULL. 4. The descendant loads NULL and dereferences rka->cred. This NULL dereference can oops the kernel. RCU delays payload disposal but does not prevent the pointer from being cleared. Load the payload with dereference_key_rcu() and require a non-NULL result before validating the authorization key and searching its saved credentials. A non-NULL payload and its credentials remain alive for the caller's RCU read-side section; a cleared payload skips the fallback search and follows the existing error selection. KASAN report as below: Oops: general protection fault, probably for non-canonical address 0xdffffc0000000005: 0000 [#1] SMP KASAN NOPTI KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f] RIP: 0010:search_process_keyrings_rcu+0x32b/0x790 Cc: stable@vger.kernel.org Fixes: e59428f721ee ("keys: Move the RCU locks outwards from the keyring search functions") Assisted-by: LLM Signed-off-by: Cen Zhang Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/pm-key-management-objects-candidate-0004-v3-598361863b76bc25d58d@gmail.com Signed-off-by: Jarkko Sakkinen --- security/keys/process_keys.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index a63c46bb2d14..76da13fa07fa 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -556,9 +556,8 @@ key_ref_t search_process_keyrings_rcu(struct keyring_search_context *ctx) ) { const struct cred *cred = ctx->cred; - if (key_validate(cred->request_key_auth) == 0) { - rka = ctx->cred->request_key_auth->payload.data[0]; - + rka = dereference_key_rcu(cred->request_key_auth); + if (rka && key_validate(cred->request_key_auth) == 0) { //// was search_process_keyrings() [ie. recursive] ctx->cred = rka->cred; key_ref = search_cred_keyrings_rcu(ctx); From de891c73c1cf41ac8de12c07fa2c84d6a912ad0c Mon Sep 17 00:00:00 2001 From: Kyle Zeng Date: Tue, 13 Oct 2026 16:12:51 -0700 Subject: [PATCH 07/13] keys: make keyring indexes independent of kernel addresses KEYCTL_READ returns the serial numbers in a keyring in associative-array traversal order. The first word of the index is a public, deterministic hash of the type pointer, domain pointer and description. An unprivileged user can therefore insert keys with known descriptions, predict their order for each possible KASLR slide, and recover the kernel image address. On an x86-64 build with 16 MiB alignment, 45 user keys suffice to distinguish the candidate slides. Remove the addresses from the whole index, not just its first hash word. Assign each key type and domain a nonzero 64-bit ID and hash those IDs and the description with a once-initialized SipHash key. Use the full IDs in the later index chunks as well. Truncating or hashing the IDs there would lose the guarantee that distinct type/domain/description tuples have a differing index bit. Allocate IDs lazily since private key types need not be registered. Cache them in the index key so tree walks and copies of an existing index do not need to dereference a type descriptor that may have been unregistered. Keep the 32-bit chunk layout and diff_objects() bit offsets consistent, and retain the separate root slot for keyrings used by recursive search. The key comparison, permissions and userspace data format are unchanged; only the address-dependent in-memory ordering is replaced. Cc: stable@vger.kernel.org Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring") Assisted-by: Codex:gpt-6-astra Signed-off-by: Kyle Zeng Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/20261013231251.56170-1-kylebot@openai.com Signed-off-by: Jarkko Sakkinen --- include/linux/key-type.h | 1 + include/linux/key.h | 5 ++ security/keys/keyring.c | 148 +++++++++++++++++++++------------------ 3 files changed, 85 insertions(+), 69 deletions(-) diff --git a/include/linux/key-type.h b/include/linux/key-type.h index bb97bd3e5af4..c4f329b6ae3f 100644 --- a/include/linux/key-type.h +++ b/include/linux/key-type.h @@ -161,6 +161,7 @@ struct key_type { const void *in, const void *in2); /* internal fields */ + atomic64_t index_id; /* Opaque keyring index ID */ struct list_head link; /* link in types list */ struct lock_class_key lock_class; /* key->sem lock class */ } __randomize_layout; diff --git a/include/linux/key.h b/include/linux/key.h index 81b8f05c6898..e40b5267658f 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -107,6 +107,7 @@ struct keyring_name; struct key_tag { struct rcu_head rcu; + atomic64_t index_id; /* Opaque keyring index ID */ refcount_t usage; bool removed; /* T when subject removed */ }; @@ -129,6 +130,8 @@ struct keyring_index_key { struct key_type *type; struct key_tag *domain_tag; /* Domain of operation */ const char *description; + u64 type_id; /* Cached index IDs */ + u64 domain_id; }; union key_payload { @@ -251,6 +254,8 @@ struct key { struct key_type *type; /* type of key */ struct key_tag *domain_tag; /* Domain of operation */ char *description; + u64 type_id; + u64 domain_id; }; }; diff --git a/security/keys/keyring.c b/security/keys/keyring.c index 46f774be7296..7143f3079d20 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -14,6 +14,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -147,49 +150,46 @@ static int keyring_instantiate(struct key *keyring, } /* - * Multiply 64-bits by 32-bits to 96-bits and fold back to 64-bit. Ideally we'd - * fold the carry back too, but that requires inline asm. + * Give key types and domains address-independent identities. Zero means that + * an ID has not yet been allocated. As with inode sequence numbers used by + * futexes, this relies on the 64-bit counter not wrapping in a machine's + * lifetime. */ -static u64 mult_64x32_and_fold(u64 x, u32 y) +static u64 key_index_id(atomic64_t *id) { - u64 hi = (u64)(u32)(x >> 32) * y; - u64 lo = (u64)(u32)(x) * y; - return lo + ((u64)(u32)hi << 32) + (u32)(hi >> 32); + static atomic64_t next_id; + u64 old, new; + + old = atomic64_read(id); + if (likely(old)) + return old; + + do { + new = atomic64_inc_return(&next_id); + } while (WARN_ON_ONCE(!new)); + + old = 0; + if (!atomic64_try_cmpxchg_relaxed(id, &old, new)) + return old; + return new; } /* - * Hash a key type and description. + * Hash a key type, domain and description. KEYCTL_READ exposes the tree's + * traversal order, so no part of the index may contain kernel addresses. */ static void hash_key_type_and_desc(struct keyring_index_key *index_key) { const unsigned level_shift = ASSOC_ARRAY_LEVEL_STEP; const unsigned long fan_mask = ASSOC_ARRAY_FAN_MASK; - const char *description = index_key->description; - unsigned long hash, type; - u32 piece; + static siphash_key_t hash_key __read_mostly; + unsigned long hash; u64 acc; - int n, desc_len = index_key->desc_len; - type = (unsigned long)index_key->type; - acc = mult_64x32_and_fold(type, desc_len + 13); - acc = mult_64x32_and_fold(acc, 9207); - piece = (unsigned long)index_key->domain_tag; - acc = mult_64x32_and_fold(acc, piece); - acc = mult_64x32_and_fold(acc, 9207); - - for (;;) { - n = desc_len; - if (n <= 0) - break; - if (n > 4) - n = 4; - piece = 0; - memcpy(&piece, description, n); - description += n; - desc_len -= n; - acc = mult_64x32_and_fold(acc, piece); - acc = mult_64x32_and_fold(acc, 9207); - } + get_random_once(&hash_key, sizeof(hash_key)); + acc = siphash(index_key->description, index_key->desc_len, &hash_key); + acc = siphash_3u64(index_key->type_id, index_key->domain_id, acc, + &hash_key); /* Fold the hash down to 32 bits if need be. */ hash = acc; @@ -209,7 +209,7 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key) /* * Finalise an index key to include a part of the description actually in the - * index key, to set the domain tag and to calculate the hash. + * index key, to set the domain tag and index IDs, and to calculate the hash. */ void key_set_index_key(struct keyring_index_key *index_key) { @@ -225,6 +225,14 @@ void key_set_index_key(struct keyring_index_key *index_key) index_key->domain_tag = &default_domain_tag; } + /* Preserve the IDs when copying an existing key's index. In + * particular, its type may already have been unregistered. + */ + if (!index_key->type_id) + index_key->type_id = key_index_id(&index_key->type->index_id); + if (!index_key->domain_id) + index_key->domain_id = key_index_id(&index_key->domain_tag->index_id); + hash_key_type_and_desc(index_key); } @@ -263,15 +271,19 @@ void key_remove_domain(struct key_tag *domain_tag) /* * Build the next index key chunk. * + * The hash and inline description are followed by the 64-bit type and domain + * IDs, low word first, and then by the rest of the description. * We return it one word-sized chunk at a time. */ static unsigned long keyring_get_key_chunk(const void *data, int level) { + const unsigned int id_chunks = sizeof(u64) / sizeof(unsigned long); const struct keyring_index_key *index_key = data; + int desc_len = index_key->desc_len; unsigned long chunk = 0; - const u8 *d; - int desc_len = index_key->desc_len, n = sizeof(chunk); + int n = sizeof(chunk); unsigned int offset; + const u8 *d; level /= ASSOC_ARRAY_KEY_CHUNK_SIZE; switch (level) { @@ -279,27 +291,30 @@ static unsigned long keyring_get_key_chunk(const void *data, int level) return index_key->hash; case 1: return index_key->x; - case 2: - return (unsigned long)index_key->type; - case 3: - return (unsigned long)index_key->domain_tag; - default: - level -= 4; - offset = sizeof(index_key->desc) + level * sizeof(long); - if (desc_len <= offset) - return 0; - - d = index_key->description + offset; - desc_len -= offset; - if (desc_len > n) - desc_len = n; - d += desc_len; - do { - chunk <<= 8; - chunk |= *--d; - } while (--desc_len > 0); - return chunk; } + + level -= 2; + if (level < id_chunks) + return index_key->type_id >> (level * ASSOC_ARRAY_KEY_CHUNK_SIZE); + level -= id_chunks; + if (level < id_chunks) + return index_key->domain_id >> (level * ASSOC_ARRAY_KEY_CHUNK_SIZE); + level -= id_chunks; + + offset = sizeof(index_key->desc) + level * sizeof(long); + if (desc_len <= offset) + return 0; + + d = index_key->description + offset; + desc_len -= offset; + if (desc_len > n) + desc_len = n; + d += desc_len; + do { + chunk <<= 8; + chunk |= *--d; + } while (--desc_len > 0); + return chunk; } static unsigned long keyring_get_object_key_chunk(const void *object, int level) @@ -331,6 +346,7 @@ static int keyring_diff_objects(const void *object, const void *data) const struct keyring_index_key *b = data; unsigned long seg_a, seg_b; int level, i; + u64 diff; level = 0; seg_a = a->hash; @@ -339,28 +355,22 @@ static int keyring_diff_objects(const void *object, const void *data) goto differ; level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8; - /* The number of bits contributed by the hash is controlled by a - * constant in the assoc_array headers. Everything else thereafter we - * can deal with as being machine word-size dependent. - */ + /* The inline description contributes one machine-word-sized chunk. */ seg_a = a->x; seg_b = b->x; if ((seg_a ^ seg_b) != 0) goto differ; level += sizeof(unsigned long); - /* The next bit may not work on big endian */ - seg_a = (unsigned long)a->type; - seg_b = (unsigned long)b->type; - if ((seg_a ^ seg_b) != 0) - goto differ; - level += sizeof(unsigned long); + diff = a->type_id ^ b->type_id; + if (diff) + return level * 8 + __ffs64(diff); + level += sizeof(u64); - seg_a = (unsigned long)a->domain_tag; - seg_b = (unsigned long)b->domain_tag; - if ((seg_a ^ seg_b) != 0) - goto differ; - level += sizeof(unsigned long); + diff = a->domain_id ^ b->domain_id; + if (diff) + return level * 8 + __ffs64(diff); + level += sizeof(u64); i = sizeof(a->desc); if (a->desc_len <= i) From 30a9dac7c63e76b91cc4f17aa4cb87bf58449714 Mon Sep 17 00:00:00 2001 From: Cen Zhang Date: Sat, 17 Oct 2026 16:16:22 +0800 Subject: [PATCH 08/13] keys: Avoid owner account dereferences in /proc/keys The namespace filters in find_ge_key() and key_serial_next() read key->user->uid while traversing the serial tree under key_serial_lock. That lock keeps the key allocated, but it does not protect the separate key_user from ownership changes. A /proc/keys read reaches find_ge_key() through proc_keys_start(), and key_serial_next() through proc_keys_next(). The lock stays held until proc_keys_stop(). A successful privileged KEYCTL_CHOWN on the same key replaces key->user under key->sem and drops the old owner's reference after releasing the semaphore. If that was the old dynamic key_user's last reference, a /proc/keys reader can access freed memory: /proc/keys reader KEYCTL_CHOWN proc_keys_start() keyctl_chown_key() spin_lock(key_serial_lock) down_write(key->sem) find_ge_key() old = minkey->user zapowner = key->user key->user = newowner key->uid = uid up_write(key->sem) key_put(key) key_user_put(zapowner) kfree(zapowner) read old->uid -> use-after-free proc_keys_stop() spin_unlock(key_serial_lock) Use the inline key->uid in both filters, following the approach already used by find_keyring_by_name(). The serial-tree lock keeps the containing key alive, so filtering no longer depends on the quota owner's lifetime. This also uses the same owner UID as proc_keys_show() and the permission check. For thread keyrings, key_fsuid_changed() can update key->uid without changing the quota owner. The filters now follow that current owner UID, including when its namespace mapping differs from the quota owner's. Fixes: 454804ab0302 ("keys: make procfiles per-user-namespace") Assisted-by: LLM Suggested-by: Eric Biggers Signed-off-by: Cen Zhang Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/pm-keys-proc-owner-uid-followup-20261017-v2-8f8f34a41eb1bbb94ef0@gmail.com Signed-off-by: Jarkko Sakkinen --- security/keys/proc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/keys/proc.c b/security/keys/proc.c index 4f4e2c1824f1..aea06a74d0fe 100644 --- a/security/keys/proc.c +++ b/security/keys/proc.c @@ -68,7 +68,7 @@ static struct rb_node *key_serial_next(struct seq_file *p, struct rb_node *n) n = rb_next(n); while (n) { struct key *key = rb_entry(n, struct key, serial_node); - if (kuid_has_mapping(user_ns, key->user->uid)) + if (kuid_has_mapping(user_ns, key->uid)) break; n = rb_next(n); } @@ -100,7 +100,7 @@ static struct key *find_ge_key(struct seq_file *p, key_serial_t id) return NULL; for (;;) { - if (kuid_has_mapping(user_ns, minkey->user->uid)) + if (kuid_has_mapping(user_ns, minkey->uid)) return minkey; n = rb_next(&minkey->serial_node); if (!n) From 925c84e3cc21caf09de92d9bcafab0147790debf Mon Sep 17 00:00:00 2001 From: Cen Zhang Date: Mon, 19 Oct 2026 22:57:21 +0800 Subject: [PATCH 09/13] keys: Protect the read method check against type removal keyctl_read_key() checks key->type->read without holding key->sem. A reference keeps the key alive, but does not pin the module containing its type. A surviving RxRPC key can therefore be read while AF_RXRPC is being unloaded: KEYCTL_READ task Module unload and key GC ---------------- ------------------------ lookup_user_key() permission / possession checks load key->type af_rxrpc_exit() unregister_key_type() GC: down_write(key->sem) GC: retype to key_type_dead GC: up_write(key->sem) unregister_key_type() returns free_module() read saved type's read member The final access can read freed module storage. The read invocation in __keyctl_read_key() is already protected by key->sem, but the earlier method check is outside that critical section. Move the method check into __keyctl_read_key(), after key_validate() and under its existing read hold on key->sem. The check and invocation then use the same protected type for length queries and every read retry. The GC cannot retype the key until the semaphore is released, and type unregistration waits for that GC pass before the module can be freed. Return -EOPNOTSUPP when validation succeeds and the type has no read method. A temporary buffer may now be allocated before this result; allocation and copying to userspace remain outside the semaphore. Assisted-by: LLM Cc: stable@vger.kernel.org # v5.10+ Fixes: d3ec10aa9581 ("KEYS: Don't write out to userspace while holding key semaphore") Suggested-by: Jarkko Sakkinen Signed-off-by: Cen Zhang Reviewed-by: Jarkko Sakkinen Reviewed-by: David Howells Link: https://lore.kernel.org/r/pm-key-management-objects-candidate-0010-v7-fc85721ff5cfb9e33ac6@gmail.com Signed-off-by: Jarkko Sakkinen --- security/keys/keyctl.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 5a5efa26ed0e..36ffba0ebba6 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -808,8 +808,12 @@ static long __keyctl_read_key(struct key *key, char *buffer, size_t buflen) down_read(&key->sem); ret = key_validate(key); - if (ret == 0) - ret = key->type->read(key, buffer, buflen); + if (ret == 0) { + if (key->type->read) + ret = key->type->read(key, buffer, buflen); + else + ret = -EOPNOTSUPP; + } up_read(&key->sem); return ret; } @@ -863,11 +867,6 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) /* the key is probably readable - now try to read it */ can_read_key: - if (!key->type->read) { - ret = -EOPNOTSUPP; - goto key_put_out; - } - if (!buffer || !buflen) { /* Get the key length from the read method */ ret = __keyctl_read_key(key, NULL, 0); From d909d8390149677ac56eab0f4616362af80a848d Mon Sep 17 00:00:00 2001 From: Srish Srinivasan Date: Sat, 12 Sep 2026 11:59:50 +0530 Subject: [PATCH 10/13] keys/trusted_keys: move TPM-specific fields into struct trusted_key_tpm The trusted_key_options struct contains TPM-specific fields (keyhandle, keyauth, blobauth_len, blobauth, pcrinfo_len, pcrinfo, pcrlock, hash, policydigest_len, policydigest, and policyhandle). This leads to the accumulation of backend-specific fields in the generic options structure. Define struct trusted_key_tpm and move the TPM-specific fields there. Store a pointer to it in the private member of struct trusted_key_options. Signed-off-by: Srish Srinivasan Reviewed-by: Stefan Berger Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/20260912062950.279104-3-ssrish@linux.ibm.com Signed-off-by: Jarkko Sakkinen --- include/keys/trusted-type.h | 11 -- include/keys/trusted_tpm.h | 14 +++ security/keys/trusted-keys/trusted_tpm1.c | 123 ++++++++++++---------- security/keys/trusted-keys/trusted_tpm2.c | 50 +++++---- 4 files changed, 111 insertions(+), 87 deletions(-) diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h index 9f9940482da4..3db61b57cf73 100644 --- a/include/keys/trusted-type.h +++ b/include/keys/trusted-type.h @@ -39,17 +39,6 @@ struct trusted_key_payload { struct trusted_key_options { uint16_t keytype; - uint32_t keyhandle; - unsigned char keyauth[TPM_DIGEST_SIZE]; - uint32_t blobauth_len; - unsigned char blobauth[TPM_DIGEST_SIZE]; - uint32_t pcrinfo_len; - unsigned char pcrinfo[MAX_PCRINFO_SIZE]; - int pcrlock; - uint32_t hash; - uint32_t policydigest_len; - unsigned char policydigest[MAX_DIGEST_SIZE]; - uint32_t policyhandle; void *private; }; diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h index 3a0fa3bc8454..9a83269821e5 100644 --- a/include/keys/trusted_tpm.h +++ b/include/keys/trusted_tpm.h @@ -6,6 +6,20 @@ extern struct trusted_key_ops trusted_key_tpm_ops; +struct trusted_key_tpm { + u32 keyhandle; + unsigned char keyauth[TPM_DIGEST_SIZE]; + u32 blobauth_len; + unsigned char blobauth[TPM_DIGEST_SIZE]; + u32 pcrinfo_len; + unsigned char pcrinfo[MAX_PCRINFO_SIZE]; + int pcrlock; + u32 hash; + u32 policydigest_len; + unsigned char policydigest[MAX_DIGEST_SIZE]; + u32 policyhandle; +}; + int tpm2_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload, struct trusted_key_options *options); diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c index 9cdfeea800a3..3ec078ca3f97 100644 --- a/security/keys/trusted-keys/trusted_tpm1.c +++ b/security/keys/trusted-keys/trusted_tpm1.c @@ -48,15 +48,17 @@ enum { #ifdef CONFIG_TRUSTED_KEYS_DEBUG static inline void dump_options(struct trusted_key_options *o) { + struct trusted_key_tpm *private = o->private; + if (!trusted_debug) return; pr_debug("sealing key type %d\n", o->keytype); - pr_debug("sealing key handle %0X\n", o->keyhandle); - pr_debug("pcrlock %d\n", o->pcrlock); - pr_debug("pcrinfo %d\n", o->pcrinfo_len); + pr_debug("sealing key handle %0X\n", private->keyhandle); + pr_debug("pcrlock %d\n", private->pcrlock); + pr_debug("pcrinfo %d\n", private->pcrinfo_len); print_hex_dump_debug("pcrinfo ", DUMP_PREFIX_NONE, - 16, 1, o->pcrinfo, o->pcrinfo_len, 0); + 16, 1, private->pcrinfo, private->pcrinfo_len, 0); } static inline void dump_sess(struct osapsess *s) @@ -626,6 +628,7 @@ static int tpm_unseal(struct tpm_buf *tb, static int key_seal(struct trusted_key_payload *p, struct trusted_key_options *o) { + struct trusted_key_tpm *private = o->private; int ret; struct tpm_buf *tb __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL); @@ -637,9 +640,10 @@ static int key_seal(struct trusted_key_payload *p, /* include migratable flag at end of sealed key */ p->key[p->key_len] = p->migratable; - ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth, + ret = tpm_seal(tb, o->keytype, private->keyhandle, private->keyauth, p->key, p->key_len + 1, p->blob, &p->blob_len, - o->blobauth, o->pcrinfo, o->pcrinfo_len); + private->blobauth, private->pcrinfo, + private->pcrinfo_len); if (ret < 0) pr_info("srkseal failed (%d)\n", ret); @@ -652,6 +656,7 @@ static int key_seal(struct trusted_key_payload *p, static int key_unseal(struct trusted_key_payload *p, struct trusted_key_options *o) { + struct trusted_key_tpm *private = o->private; int ret; struct tpm_buf *tb __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL); @@ -660,8 +665,8 @@ static int key_unseal(struct trusted_key_payload *p, tpm_buf_init(tb, TPM_BUFSIZE); - ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, - o->blobauth, p->key, &p->key_len); + ret = tpm_unseal(tb, private->keyhandle, private->keyauth, p->blob, + p->blob_len, private->blobauth, p->key, &p->key_len); if (ret < 0) pr_info("srkunseal failed (%d)\n", ret); else @@ -697,6 +702,7 @@ static const match_table_t key_tokens = { static int getoptions(char *c, struct trusted_key_payload *pay, struct trusted_key_options *opt) { + struct trusted_key_tpm *private = opt->private; substring_t args[MAX_OPT_ARGS]; char *p = c; int token; @@ -712,7 +718,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay, if (tpm2 < 0) return tpm2; - opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1; + private->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1; if (!c) return 0; @@ -726,11 +732,11 @@ static int getoptions(char *c, struct trusted_key_payload *pay, switch (token) { case Opt_pcrinfo: - opt->pcrinfo_len = strlen(args[0].from) / 2; - if (opt->pcrinfo_len > MAX_PCRINFO_SIZE) + private->pcrinfo_len = strlen(args[0].from) / 2; + if (private->pcrinfo_len > MAX_PCRINFO_SIZE) return -EINVAL; - res = hex2bin(opt->pcrinfo, args[0].from, - opt->pcrinfo_len); + res = hex2bin(private->pcrinfo, args[0].from, + private->pcrinfo_len); if (res < 0) return -EINVAL; break; @@ -739,12 +745,12 @@ static int getoptions(char *c, struct trusted_key_payload *pay, if (res < 0) return -EINVAL; opt->keytype = SEAL_keytype; - opt->keyhandle = handle; + private->keyhandle = handle; break; case Opt_keyauth: if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE) return -EINVAL; - res = hex2bin(opt->keyauth, args[0].from, + res = hex2bin(private->keyauth, args[0].from, SHA1_DIGEST_SIZE); if (res < 0) return -EINVAL; @@ -755,21 +761,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay, * hex strings. TPM 2.0 authorizations are simple * passwords (although it can take a hash as well) */ - opt->blobauth_len = strlen(args[0].from); + private->blobauth_len = strlen(args[0].from); - if (opt->blobauth_len == 2 * TPM_DIGEST_SIZE) { - res = hex2bin(opt->blobauth, args[0].from, + if (private->blobauth_len == 2 * TPM_DIGEST_SIZE) { + res = hex2bin(private->blobauth, args[0].from, TPM_DIGEST_SIZE); if (res < 0) return -EINVAL; - opt->blobauth_len = TPM_DIGEST_SIZE; + private->blobauth_len = TPM_DIGEST_SIZE; break; } - if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) { - memcpy(opt->blobauth, args[0].from, - opt->blobauth_len); + if (tpm2 && + private->blobauth_len <= + sizeof(private->blobauth)) { + memcpy(private->blobauth, args[0].from, + private->blobauth_len); break; } @@ -787,14 +795,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay, res = kstrtoul(args[0].from, 10, &lock); if (res < 0) return -EINVAL; - opt->pcrlock = lock; + private->pcrlock = lock; break; case Opt_hash: if (test_bit(Opt_policydigest, &token_mask)) return -EINVAL; for (i = 0; i < HASH_ALGO__LAST; i++) { if (!strcmp(args[0].from, hash_algo_name[i])) { - opt->hash = i; + private->hash = i; break; } } @@ -806,14 +814,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay, } break; case Opt_policydigest: - digest_len = hash_digest_size[opt->hash]; + digest_len = hash_digest_size[private->hash]; if (!tpm2 || strlen(args[0].from) != (2 * digest_len)) return -EINVAL; - res = hex2bin(opt->policydigest, args[0].from, + res = hex2bin(private->policydigest, args[0].from, digest_len); if (res < 0) return -EINVAL; - opt->policydigest_len = digest_len; + private->policydigest_len = digest_len; break; case Opt_policyhandle: if (!tpm2) @@ -821,7 +829,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay, res = kstrtoul(args[0].from, 16, &handle); if (res < 0) return -EINVAL; - opt->policyhandle = handle; + private->policyhandle = handle; break; default: return -EINVAL; @@ -832,6 +840,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay, static struct trusted_key_options *trusted_options_alloc(void) { + struct trusted_key_tpm *private; struct trusted_key_options *options; int tpm2; @@ -844,15 +853,23 @@ static struct trusted_key_options *trusted_options_alloc(void) /* set any non-zero defaults */ options->keytype = SRK_keytype; - if (!tpm2) - options->keyhandle = SRKHANDLE; + private = kzalloc_obj(*private); + if (!private) { + kfree_sensitive(options); + options = NULL; + } else { + if (!tpm2) + private->keyhandle = SRKHANDLE; + options->private = private; + } } return options; } static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob) { - struct trusted_key_options *options = NULL; + struct trusted_key_options *options __free(kfree_sensitive) = NULL; + struct trusted_key_tpm *private __free(kfree_sensitive) = NULL; int ret = 0; int tpm2; @@ -864,15 +881,15 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob) if (!options) return -ENOMEM; + private = options->private; + ret = getoptions(datablob, p, options); if (ret < 0) - goto out; + return ret; dump_options(options); - if (!options->keyhandle && !tpm2) { - ret = -EINVAL; - goto out; - } + if (!private->keyhandle && !tpm2) + return -EINVAL; if (tpm2) ret = tpm2_seal_trusted(chip, p, options); @@ -880,24 +897,24 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob) ret = key_seal(p, options); if (ret < 0) { pr_info("key_seal failed (%d)\n", ret); - goto out; + return ret; } - if (options->pcrlock) { - ret = pcrlock(options->pcrlock); + if (private->pcrlock) { + ret = pcrlock(private->pcrlock); if (ret < 0) { pr_info("pcrlock failed (%d)\n", ret); - goto out; + return ret; } } -out: - kfree_sensitive(options); + return ret; } static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob) { - struct trusted_key_options *options = NULL; + struct trusted_key_options *options __free(kfree_sensitive) = NULL; + struct trusted_key_tpm *private __free(kfree_sensitive) = NULL; int ret = 0; int tpm2; @@ -908,16 +925,15 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob) options = trusted_options_alloc(); if (!options) return -ENOMEM; + private = options->private; ret = getoptions(datablob, p, options); if (ret < 0) - goto out; + return ret; dump_options(options); - if (!options->keyhandle && !tpm2) { - ret = -EINVAL; - goto out; - } + if (!private->keyhandle && !tpm2) + return -EINVAL; if (tpm2) ret = tpm2_unseal_trusted(chip, p, options); @@ -925,18 +941,17 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob) ret = key_unseal(p, options); if (ret < 0) { pr_info("key_unseal failed (%d)\n", ret); - goto out; + return ret; } - if (options->pcrlock) { - ret = pcrlock(options->pcrlock); + if (private->pcrlock) { + ret = pcrlock(private->pcrlock); if (ret < 0) { pr_info("pcrlock failed (%d)\n", ret); - goto out; + return ret; } } -out: - kfree_sensitive(options); + return ret; } diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index a9b8a31a637c..414d4d53b9e7 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -23,6 +23,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload, struct trusted_key_options *options, u8 *src, u32 len) { + struct trusted_key_tpm *private = options->private; const int SCRATCH_SIZE = PAGE_SIZE; u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL); u8 *work = scratch, *work1; @@ -45,7 +46,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload, work = asn1_encode_oid(work, end_work, tpm2key_oid, asn1_oid_len(tpm2key_oid)); - if (options->blobauth_len == 0) { + if (private->blobauth_len == 0) { unsigned char bool[3], *w = bool; /* tag 0 is emptyAuth */ w = asn1_encode_boolean(w, w + sizeof(bool), true); @@ -68,7 +69,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload, goto err; } - work = asn1_encode_integer(work, end_work, options->keyhandle); + work = asn1_encode_integer(work, end_work, private->keyhandle); work = asn1_encode_octet_string(work, end_work, pub, pub_len); work = asn1_encode_octet_string(work, end_work, priv, priv_len); @@ -101,6 +102,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload, struct trusted_key_options *options, u8 **buf, unsigned int *blob_len) { + struct trusted_key_tpm *private = options->private; int ret; struct tpm2_key_context ctx; u8 *blob; @@ -121,7 +123,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload, *buf = blob; *blob_len = ctx.priv_len + ctx.pub_len; - options->keyhandle = ctx.parent; + private->keyhandle = ctx.parent; memcpy(blob, ctx.priv, ctx.priv_len); blob += ctx.priv_len; @@ -233,6 +235,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload, struct trusted_key_options *options) { + struct trusted_key_tpm *private = options->private; off_t offset = TPM_HEADER_SIZE; struct tpm_buf *buf __free(kfree) = NULL; struct tpm_buf *sized __free(kfree) = NULL; @@ -241,11 +244,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip, u32 flags; int rc; - hash = tpm2_find_hash_alg(options->hash); + hash = tpm2_find_hash_alg(private->hash); if (hash < 0) return hash; - if (!options->keyhandle) + if (!private->keyhandle) return -EINVAL; rc = tpm_try_get_ops(chip); @@ -275,18 +278,18 @@ int tpm2_seal_trusted(struct tpm_chip *chip, tpm_buf_init_sized(sized, TPM_BUFSIZE); - rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL); + rc = tpm_buf_append_name(chip, buf, private->keyhandle, NULL); if (rc) goto out; tpm_buf_append_hmac_session(chip, buf, TPM2_SA_DECRYPT, - options->keyauth, TPM_DIGEST_SIZE); + private->keyauth, TPM_DIGEST_SIZE); /* sensitive */ - tpm_buf_append_u16(sized, options->blobauth_len); + tpm_buf_append_u16(sized, private->blobauth_len); - if (options->blobauth_len) - tpm_buf_append(sized, options->blobauth, options->blobauth_len); + if (private->blobauth_len) + tpm_buf_append(sized, private->blobauth, private->blobauth_len); tpm_buf_append_u16(sized, payload->key_len); tpm_buf_append(sized, payload->key, payload->key_len); @@ -299,14 +302,15 @@ int tpm2_seal_trusted(struct tpm_chip *chip, /* key properties */ flags = 0; - flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH; + flags |= private->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH; flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT); tpm_buf_append_u32(sized, flags); /* policy */ - tpm_buf_append_u16(sized, options->policydigest_len); - if (options->policydigest_len) - tpm_buf_append(sized, options->policydigest, options->policydigest_len); + tpm_buf_append_u16(sized, private->policydigest_len); + if (private->policydigest_len) + tpm_buf_append(sized, private->policydigest, + private->policydigest_len); /* public parameters */ tpm_buf_append_u16(sized, TPM_ALG_NULL); @@ -377,6 +381,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, u32 *blob_handle) { u8 *blob_ref __free(kfree) = NULL; + struct trusted_key_tpm *private = options->private; struct tpm_buf *buf __free(kfree) = NULL; unsigned int private_len; unsigned int public_len; @@ -397,7 +402,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, } /* new format carries keyhandle but old format doesn't */ - if (!options->keyhandle) + if (!private->keyhandle) return -EINVAL; /* must be big enough for at least the two be16 size counts */ @@ -441,11 +446,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip, tpm_buf_init(buf, TPM_BUFSIZE); tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD); - rc = tpm_buf_append_name(chip, buf, options->keyhandle, NULL); + rc = tpm_buf_append_name(chip, buf, private->keyhandle, NULL); if (rc) return rc; - tpm_buf_append_hmac_session(chip, buf, 0, options->keyauth, + tpm_buf_append_hmac_session(chip, buf, 0, private->keyauth, TPM_DIGEST_SIZE); tpm_buf_append(buf, blob, blob_len); @@ -485,6 +490,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, struct trusted_key_options *options, u32 blob_handle) { + struct trusted_key_tpm *private = options->private; struct tpm_header *head; struct tpm_buf *buf __free(kfree) = NULL; u16 data_len; @@ -509,10 +515,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, if (rc) return rc; - if (!options->policyhandle) { + if (!private->policyhandle) { tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT, - options->blobauth, - options->blobauth_len); + private->blobauth, + private->blobauth_len); } else { /* * FIXME: The policy session was generated outside the @@ -525,9 +531,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, * could repeat our actions with the exfiltrated * password. */ - tpm2_buf_append_auth(buf, options->policyhandle, + tpm2_buf_append_auth(buf, private->policyhandle, NULL /* nonce */, 0, 0, - options->blobauth, options->blobauth_len); + private->blobauth, private->blobauth_len); if (tpm2_chip_auth(chip)) { tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT, NULL, 0); From 9dbaf2346de978331f202d1a700b07af84c5795b Mon Sep 17 00:00:00 2001 From: Srish Srinivasan Date: Sun, 27 Sep 2026 00:12:03 +0530 Subject: [PATCH 11/13] keys/trusted/tpm2: Validate TPM2_Create object sizes separately TPM2_Create returns outPrivate, outPublic, creationData, creationHash and creationTicket in its response parameter area. However, only outPrivate and outPublic are included in the trusted key blob. The size of the blob is therefore not determined by the size of the complete response parameter area. tpm2_seal_trusted() currently compares the size of the complete response parameter area against MAX_BLOB_SIZE. This can reject a valid response when the remaining response outputs cause the entire response parameter area to exceed MAX_BLOB_SIZE, even though the outPrivate and outPublic TPM2B structures consumed by tpm2_key_encode() remain small enough to be encoded in the key blob. This is observed when creating larger trusted keys using the swtpm TPM 2.0 emulator backed by libtpms. For example, requesting a 113-byte key succeeds, 114 fails. ~$ keyctl add trusted trusted_key1 "new 113 keyhandle=0x81000001" @u 520504613 ~$ keyctl add trusted trusted_key2 "new 114 keyhandle=0x81000001" @u add_key: Argument list too long ~$ Remove the MAX_BLOB_SIZE check on the complete response parameter area. Instead, use the response length passed to tpm2_key_encode() to validate that the outPrivate and outPublic TPM2B structures are fully contained in the response before accessing them. Previously, a response parameter area larger than MAX_BLOB_SIZE was rejected with -E2BIG before ASN.1 encoding. With this change, if the resulting encoded blob does not fit in payload->blob, the error returned by asn1_encode_sequence() is propagated instead. Also, use scope-based cleanup to simplify resource management. Signed-off-by: Srish Srinivasan Link: https://lore.kernel.org/r/20260926184203.479203-1-ssrish@linux.ibm.com Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- security/keys/trusted-keys/trusted_tpm2.c | 44 +++++++++++++---------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index 414d4d53b9e7..30188a0ec676 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -25,24 +25,36 @@ static int tpm2_key_encode(struct trusted_key_payload *payload, { struct trusted_key_tpm *private = options->private; const int SCRATCH_SIZE = PAGE_SIZE; - u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL); - u8 *work = scratch, *work1; - u8 *end_work = scratch + SCRATCH_SIZE; + u8 *scratch __free(kfree) = NULL; + u8 *work, *work1; + u8 *end_work; u8 *priv, *pub; - u16 priv_len, pub_len; + u32 priv_len, pub_len; int ret; - priv_len = get_unaligned_be16(src) + 2; - priv = src; + if (len < 4) + return -EINVAL; + priv_len = get_unaligned_be16(src) + 2; + if (priv_len + 2 > len) + return -EIO; + + priv = src; src += priv_len; pub_len = get_unaligned_be16(src) + 2; + if (pub_len + priv_len > len) + return -EIO; + pub = src; + scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL); if (!scratch) return -ENOMEM; + work = scratch; + end_work = scratch + SCRATCH_SIZE; + work = asn1_encode_oid(work, end_work, tpm2key_oid, asn1_oid_len(tpm2key_oid)); @@ -50,10 +62,9 @@ static int tpm2_key_encode(struct trusted_key_payload *payload, unsigned char bool[3], *w = bool; /* tag 0 is emptyAuth */ w = asn1_encode_boolean(w, w + sizeof(bool), true); - if (WARN(IS_ERR(w), "BUG: Boolean failed to encode")) { - ret = PTR_ERR(w); - goto err; - } + if (WARN(IS_ERR(w), "BUG: Boolean failed to encode")) + return PTR_ERR(w); + work = asn1_encode_tag(work, end_work, 0, bool, w - bool); } @@ -65,8 +76,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload, */ if (WARN(work - scratch + pub_len + priv_len + 14 > SCRATCH_SIZE, "BUG: scratch buffer is too small")) { - ret = -EINVAL; - goto err; + return -EINVAL; } work = asn1_encode_integer(work, end_work, private->keyhandle); @@ -79,15 +89,10 @@ static int tpm2_key_encode(struct trusted_key_payload *payload, if (IS_ERR(work1)) { ret = PTR_ERR(work1); pr_err("BUG: ASN.1 encoder failed with %d\n", ret); - goto err; + return ret; } - kfree(scratch); return work1 - payload->blob; - -err: - kfree(scratch); - return ret; } struct tpm2_key_context { @@ -340,10 +345,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip, goto out; blob_len = tpm_buf_read_u32(buf, &offset); - if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_INVALID) { + if (buf->flags & TPM_BUF_INVALID) { rc = -E2BIG; goto out; } + if (buf->length - offset < blob_len) { rc = -EFAULT; goto out; From bbebc81e7e72039369b5dc6272a33c42b8bfa2d7 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 28 Sep 2026 09:20:23 +0200 Subject: [PATCH 12/13] security: keys: Fix comment of search_process_keyrings_rcu() search_my_process_keyrings() has been renamed to search_cred_keyrings_rcu() in commit e59428f721ee ("keys: Move the RCU locks outwards from the keyring search functions"), but one spot has been missed in a comment. Signed-off-by: Thomas Huth Reviewed-by: Jinjie Ruan Link: https://lore.kernel.org/r/20260928072024.91664-1-thuth@redhat.com Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- security/keys/process_keys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 76da13fa07fa..1a51a36ce32a 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -528,7 +528,7 @@ key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx) /* * Search the process keyrings attached to the supplied cred for the first - * matching key in the manner of search_my_process_keyrings(), but also search + * matching key in the manner of search_cred_keyrings_rcu(), but also search * the keys attached to the assumed authorisation key using its credentials if * one is available. * From 722658cac48a6ba49f5506fd74d010ad9dbb2f05 Mon Sep 17 00:00:00 2001 From: Srish Srinivasan Date: Mon, 5 Oct 2026 11:57:32 +0530 Subject: [PATCH 13/13] keys/trusted_keys: reuse TPM version in option handling Determine the TPM version once in the seal and unseal paths, and reuse it when allocating and parsing trusted key options. This reduces the number of fallible calls to tpm_is_tpm2() and avoids unnecessary I/O between the kernel and the TPM. Signed-off-by: Srish Srinivasan Reviewed-by: Stefan Berger Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/20261005062732.140733-1-ssrish@linux.ibm.com Signed-off-by: Jarkko Sakkinen --- security/keys/trusted-keys/trusted_tpm1.c | 35 +++++++++-------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c index 3ec078ca3f97..6dbce058368f 100644 --- a/security/keys/trusted-keys/trusted_tpm1.c +++ b/security/keys/trusted-keys/trusted_tpm1.c @@ -700,7 +700,7 @@ static const match_table_t key_tokens = { /* can have zero or more token= options */ static int getoptions(char *c, struct trusted_key_payload *pay, - struct trusted_key_options *opt) + struct trusted_key_options *opt, bool is_tpm2) { struct trusted_key_tpm *private = opt->private; substring_t args[MAX_OPT_ARGS]; @@ -712,13 +712,8 @@ static int getoptions(char *c, struct trusted_key_payload *pay, unsigned long token_mask = 0; unsigned int digest_len; int i; - int tpm2; - tpm2 = tpm_is_tpm2(chip); - if (tpm2 < 0) - return tpm2; - - private->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1; + private->hash = is_tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1; if (!c) return 0; @@ -773,7 +768,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay, break; } - if (tpm2 && + if (is_tpm2 && private->blobauth_len <= sizeof(private->blobauth)) { memcpy(private->blobauth, args[0].from, @@ -808,14 +803,15 @@ static int getoptions(char *c, struct trusted_key_payload *pay, } if (i == HASH_ALGO__LAST) return -EINVAL; - if (!tpm2 && i != HASH_ALGO_SHA1) { + if (!is_tpm2 && i != HASH_ALGO_SHA1) { pr_info("TPM 1.x only supports SHA-1.\n"); return -EINVAL; } break; case Opt_policydigest: digest_len = hash_digest_size[private->hash]; - if (!tpm2 || strlen(args[0].from) != (2 * digest_len)) + if (!is_tpm2 || + strlen(args[0].from) != (2 * digest_len)) return -EINVAL; res = hex2bin(private->policydigest, args[0].from, digest_len); @@ -824,7 +820,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay, private->policydigest_len = digest_len; break; case Opt_policyhandle: - if (!tpm2) + if (!is_tpm2) return -EINVAL; res = kstrtoul(args[0].from, 16, &handle); if (res < 0) @@ -838,15 +834,10 @@ static int getoptions(char *c, struct trusted_key_payload *pay, return 0; } -static struct trusted_key_options *trusted_options_alloc(void) +static struct trusted_key_options *trusted_options_alloc(bool is_tpm2) { struct trusted_key_tpm *private; struct trusted_key_options *options; - int tpm2; - - tpm2 = tpm_is_tpm2(chip); - if (tpm2 < 0) - return NULL; options = kzalloc_obj(*options); if (options) { @@ -858,7 +849,7 @@ static struct trusted_key_options *trusted_options_alloc(void) kfree_sensitive(options); options = NULL; } else { - if (!tpm2) + if (!is_tpm2) private->keyhandle = SRKHANDLE; options->private = private; } @@ -877,13 +868,13 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob) if (tpm2 < 0) return tpm2; - options = trusted_options_alloc(); + options = trusted_options_alloc(tpm2); if (!options) return -ENOMEM; private = options->private; - ret = getoptions(datablob, p, options); + ret = getoptions(datablob, p, options, tpm2); if (ret < 0) return ret; dump_options(options); @@ -922,12 +913,12 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob) if (tpm2 < 0) return tpm2; - options = trusted_options_alloc(); + options = trusted_options_alloc(tpm2); if (!options) return -ENOMEM; private = options->private; - ret = getoptions(datablob, p, options); + ret = getoptions(datablob, p, options, tpm2); if (ret < 0) return ret; dump_options(options);