Re: [PATCH v4 10/11] KEYS: Remove key_type_dead

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] keys: broken garbage collection timer calculation in
  key_garbage_collector() [1]
--

commit 685cce051d4099c4b86015bc6b82bcd593f0652f
Author: Jarkko Sakkinen <jarkko@kernel.org>

KEYS: Remove key_type_dead

This commit removes key_type_dead and its three-phase keytype reaping logic
from the key garbage collector and unregister_key_type().

> diff --git a/security/keys/gc.c b/security/keys/gc.c
> index 7b2f32998f58b..859f331549cef 100644
> --- a/security/keys/gc.c
> +++ b/security/keys/gc.c

[ ... ]

> @@ -246,18 +192,6 @@ static void key_garbage_collector(struct work_struct *work)
>  			goto found_unreferenced_key;
>  		}
>  
> -		if (unlikely(gc_state & KEY_GC_REAPING_DEAD_1)) {
> -			if (key->type == key_gc_dead_keytype) {
> -				gc_state |= KEY_GC_FOUND_DEAD_KEY;
> -				set_bit(KEY_FLAG_DEAD, &key->flags);
> -				key->perm = 0;
> -				goto skip_dead_key;
> -			} else if (key->type == &key_type_keyring &&
> -				   key->restrict_link) {
> -				goto found_restricted_keyring;
> -			}
> -		}
> -
>  		expiry = key->expiry;
>  		if (expiry != TIME64_MAX) {
>  			if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))

[Severity: Medium]
This problem wasn't introduced by this patch, but does the timer calculation
in key_garbage_collector() mix delay-adjusted and unadjusted expiration
timestamps [1]?

security/keys/gc.c:key_garbage_collector() {
	...
		expiry = key->expiry;
		if (expiry != TIME64_MAX) {
			if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))
				expiry += key_gc_delay;
			if (expiry > limit && expiry < new_timer) {
				kdebug("will expire %x in %lld",
				       key_serial(key), key->expiry - limit);
				new_timer = key->expiry;
			}
		}
	...
	if (new_timer != TIME64_MAX) {
		new_timer += key_gc_delay;
		key_schedule_gc(new_timer);
	}
}

When KEY_TYPE_INSTANT_REAP is not set, expiry has key_gc_delay added before
it is compared against new_timer, but new_timer is then assigned the
unadjusted key->expiry.  Subsequent keys whose adjusted expiry falls at or
after a previously recorded unadjusted key->expiry fail the
expiry < new_timer check even when their actual reap time is earlier.  After
the tree scan finishes, key_gc_delay is also added unconditionally to
new_timer even if new_timer came from a key with KEY_TYPE_INSTANT_REAP set,
delaying background reaping of expired keys and dead keyring links by up to
key_gc_delay.

[1] https://sashiko.dev/#/bug/linux-327c9a06-109f-4001-9a3e-a6092c989462

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107131512.3603168-1-jarkko@kernel.org?part=10