From 25bf14f817168079f731975b8998fc2af380f29e Mon Sep 17 00:00:00 2001 From: Karl Mehltretter Date: Sat, 12 Sep 2026 11:54:40 +0200 Subject: [PATCH 1/2] keys: finalize persistent keyring timeout after link attempt When no keyring exists for the requested UID, KEYCTL_GET_PERSISTENT creates and registers one before linking it to the requested destination. The configured timeout is set only after the destination link succeeds. A destination restricted with KEYCTL_RESTRICT_KEYRING makes that link fail with -EPERM. With persistent_keyring_expiry set to 60 seconds, /proc/keys still reports the registered keyring's expiry as "perm". The failed call therefore leaves a quota-exempt keyring in the namespace's hidden register, where it may remain until namespace teardown. Rename the write-locked helper to key_get_or_create_persistent() and return the key reference through a result parameter. Return 0 when it creates a keyring and 1 when the retry finds an existing one. Return a negative error on failure. Another caller may create the keyring between the initial read-locked lookup and the retry under the write lock. Set the timeout after permission checking and linking. Do this on success, or on failure if the call allocated the keyring. This leaves a new keyring collectible after failure without starting its timeout while linking is still in progress. Cc: stable@vger.kernel.org # v5.10+ Fixes: f36f8c75ae2e ("KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches") Assisted-by: LLM Signed-off-by: Karl Mehltretter Link: https://lore.kernel.org/r/20260912095440.79864-1-kmehltretter@gmail.com Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- security/keys/persistent.c | 55 ++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/security/keys/persistent.c b/security/keys/persistent.c index 97af230aa4b2..bdab99539326 100644 --- a/security/keys/persistent.c +++ b/security/keys/persistent.c @@ -33,25 +33,31 @@ static int key_create_persistent_register(struct user_namespace *ns) } /* - * Create the persistent keyring for the specified user. + * Get or create the persistent keyring for the specified user. * * Called with the namespace's sem locked for writing. + * + * Returns a boolean indicating whether the keyring already existed, + * or a negative error. On success, *persistent_ref holds a reference + * to the keyring. */ -static key_ref_t key_create_persistent(struct user_namespace *ns, kuid_t uid, - struct keyring_index_key *index_key) +static int key_get_or_create_persistent(struct user_namespace *ns, kuid_t uid, + struct keyring_index_key *index_key, + key_ref_t *persistent_ref) { struct key *persistent; - key_ref_t reg_ref, persistent_ref; + key_ref_t reg_ref; if (!ns->persistent_keyring_register) { - long err = key_create_persistent_register(ns); + int err = key_create_persistent_register(ns); + if (err < 0) - return ERR_PTR(err); + return err; } else { reg_ref = make_key_ref(ns->persistent_keyring_register, true); - persistent_ref = find_key_to_update(reg_ref, index_key); - if (persistent_ref) - return persistent_ref; + *persistent_ref = find_key_to_update(reg_ref, index_key); + if (*persistent_ref) + return 1; } persistent = keyring_alloc(index_key->description, @@ -61,9 +67,10 @@ static key_ref_t key_create_persistent(struct user_namespace *ns, kuid_t uid, KEY_ALLOC_NOT_IN_QUOTA, NULL, ns->persistent_keyring_register); if (IS_ERR(persistent)) - return ERR_CAST(persistent); + return PTR_ERR(persistent); - return make_key_ref(persistent, true); + *persistent_ref = make_key_ref(persistent, true); + return 0; } /* @@ -78,6 +85,7 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid, key_ref_t reg_ref, persistent_ref; char buf[32]; long ret; + bool created = false; /* Look in the register if it exists */ memset(&index_key, 0, sizeof(index_key)); @@ -100,23 +108,24 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid, * also need to create the register. */ down_write(&ns->keyring_sem); - persistent_ref = key_create_persistent(ns, uid, &index_key); + ret = key_get_or_create_persistent(ns, uid, &index_key, + &persistent_ref); up_write(&ns->keyring_sem); - if (!IS_ERR(persistent_ref)) - goto found; - - return PTR_ERR(persistent_ref); + if (ret < 0) + return ret; + created = ret == 0; found: + persistent = key_ref_to_ptr(persistent_ref); ret = key_task_permission(persistent_ref, current_cred(), KEY_NEED_LINK); - if (ret == 0) { - persistent = key_ref_to_ptr(persistent_ref); + if (ret == 0) ret = key_link(key_ref_to_ptr(dest_ref), persistent); - if (ret == 0) { - key_set_timeout(persistent, persistent_keyring_expiry); - ret = persistent->serial; - } - } + + if (ret == 0 || created) + key_set_timeout(persistent, persistent_keyring_expiry); + + if (ret == 0) + ret = persistent->serial; key_ref_put(persistent_ref); return ret; From dd3ea3fcba7c75493760cdbccd35f029597e8d5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=84=B1=EB=B3=91=EC=B0=AC?= Date: Wed, 30 Sep 2026 13:18:01 +0900 Subject: [PATCH 2/2] KEYS: Fix add_key() race with keyring restriction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit __key_create_or_update() snapshots keyring->restrict_link before taking the destination keyring's semaphore. keyring_restrict() installs a restriction while holding that semaphore. This allows a writer to observe no restriction, wait for the keyring owner to install a reject-all restriction and return successfully, and then link a key using the stale NULL snapshot. The writer only needs write permission on the destination keyring. Move the restrict_link read after __key_link_lock() and __key_link_begin(). The read and the subsequent restriction check are then serialized with restriction installation by keyring->sem. The race was reproduced on v7.2.8 in 19 executions where restriction installation returned before the link completed. All 19 linked the key despite the reject-all restriction. With this change, 312 executions reached the same ordering and every add_key() call failed with -EPERM. The issue was found by manual concurrency analysis assisted by AI-based analysis and independently verified with a QEMU reproducer and kernel instrumentation. Fixes: 5ac7eace2d00 ("KEYS: Add a facility to restrict new links into a keyring") Cc: stable@vger.kernel.org Signed-off-by: 성병찬 Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/20260930041802.6114-1-tjdqudcks0424@naver.com Signed-off-by: Jarkko Sakkinen --- security/keys/key.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/security/keys/key.c b/security/keys/key.c index b34a64d81d47..a438c4508595 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -840,9 +840,6 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref, key_check(keyring); - if (!(flags & KEY_ALLOC_BYPASS_RESTRICTION)) - restrict_link = keyring->restrict_link; - key_ref = ERR_PTR(-ENOTDIR); if (keyring->type != &key_type_keyring) goto error_put_type; @@ -880,6 +877,9 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref, goto error_link_end; } + if (!(flags & KEY_ALLOC_BYPASS_RESTRICTION)) + restrict_link = keyring->restrict_link; + if (restrict_link && restrict_link->check) { ret = restrict_link->check(keyring, index_key.type, &prep.payload, restrict_link->key);