Re: [PATCH v5 2/3] KEYS: Replace assoc_array with rhashtable

[Hannes Reinecke <hare@suse.de>]

On 11/5/26 4:14 AM, Jarkko Sakkinen wrote:
> The keyrings subsystem is the sole user of assoc_array. It has over 1700
> lines of code, no test suite, and a long history of bugs, with LLMs now
> finding more.
> 
> This unfortunate outcome has evolved over time by large from the shortage
> of in-kernel callers, time bandwidth and 3rd party contributors.
> 
> What the keyrings needs is just a generic key-value store, which performs
> well with sparse key sets.
> 
> Replace struct assoc_array in struct key with struct keyring_index:
> 
> - keyring_index: holds an rhashtable for direct lookups, an RCU-protected
>    list of keyring_link entries for stable walks, the active link count, and
>    an rcu_work item for deferred teardown.
> - keyring_link: links a key into a keyring's hash table and list, caching
>    its 32-bit SipHash digest rehashed with jhash by rhashtable.
> 
> Correspondingly, struct assoc_array_edit is replaced by struct keyring_edit
> for delivering transient state across link operations.
> 
> Other behavioral changes:
> 
> - KEYCTL_READ and sibling-keyring searches follow list order instead of
>    hash order: keyrings are prepended, other keys are appended, and
>    replacing a matching key retains its position.
> - key_unlink(), keyring_clear(), keyring_revoke(), and keyring_gc() no
>    longer allocate memory.
> - keyring_index_free() uses queue_rcu_work() instead of call_rcu() because
>    rhashtable_destroy() may sleep.
> 
> Suggested-by: Eric Biggers <ebiggers@kernel.org>
> Co-developed-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>   include/keys/keyring-type.h |   1 -
>   include/linux/key-type.h    |   1 -
>   include/linux/key.h         |  28 +-
>   security/keys/Kconfig       |   1 -
>   security/keys/internal.h    |  14 +-
>   security/keys/key.c         |  22 +-
>   security/keys/keyring.c     | 986 ++++++++++++++++--------------------
>   security/keys/request_key.c |   8 +-
>   8 files changed, 465 insertions(+), 596 deletions(-)
> 
While I am pretty much in favour with this approach, is there a specific
reason why siphash has been chosen?
Would there be a chance to make this configurable?
I do see slight complications when running in secure environments
(ie FIPS-certified environments) where most of the hash functions
might not be available ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich