Re: [PATCH] keys: set persistent keyring timeout before destination linking

[Jarkko Sakkinen]

On Sun, Aug 30, 2026 at 08:20:26PM +0200, Karl Mehltretter wrote:
> keyring_alloc() links a newly created persistent keyring into the
> namespace's hidden register with TIME64_MAX expiry.
> key_get_persistent() sets the configured timeout only after linking the
> keyring into the caller's destination.
> 
> If the destination rejects the link, the registered keyring retains
> infinite expiry. It is exempt from key quota and can remain until namespace
> teardown or reboot even though the syscall returned an error. At most one
> such keyring exists per UID per namespace. Repeated failures for the same
> UID therefore strand only one small allocation.
> 
> This is reachable from unprivileged userspace: KEYCTL_RESTRICT_KEYRING on
> the destination makes key_link() return -EPERM via restrict_link_reject(),
> after key_create_persistent() has already registered the keyring.
> 
> Set the timeout immediately after creating and registering the keyring. A
> later permission or destination-link failure then leaves a collectible
> persistent keyring. The successful path may refresh the same timeout again
> without changing its semantics.
> 
> Fixes: f36f8c75ae2e ("KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches")
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
> ---
> Tested with QEMU 10.2.1 TCG. The reproducer set
> /proc/sys/kernel/keys/persistent_keyring_expiry to 60 seconds, restricted
> the destination with KEYCTL_RESTRICT_KEYRING, and then called
> KEYCTL_GET_PERSISTENT. No LSM policy was loaded.
> 
>                       syscall result  /proc/keys expiry
>   i386 baseline       -EPERM          perm
>   i386 patched        -EPERM          1m
>   x86_64 patched      -EPERM          1m
> 
> Full kernel builds completed for i386 and ARM926, and for x86_64 with
> CONFIG_PROVE_LOCKING=y. The x86_64 reproducer completed without lockdep
> reports, warnings, or bugs.

Please describe this in the commit message and delete most of
the text it has. This is the motivation part.

> 
>  security/keys/persistent.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/security/keys/persistent.c b/security/keys/persistent.c
> index 97af230aa4b22..0eca8c5758434 100644
> --- a/security/keys/persistent.c
> +++ b/security/keys/persistent.c
> @@ -63,6 +63,12 @@ static key_ref_t key_create_persistent(struct user_namespace *ns, kuid_t uid,
>  	if (IS_ERR(persistent))
>  		return ERR_CAST(persistent);
>  
> +	/* Set the expiry now: if the caller then fails to link the keyring to
> +	 * its destination, the register is left holding a collectible key
> +	 * rather than a permanent, quota-exempt one.
> +	 */
> +	key_set_timeout(persistent, persistent_keyring_expiry);
> +
>  	return make_key_ref(persistent, true);
>  }
>  
> 
> base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72
> -- 
> 2.53.0

BR, Jarkko