Re: [PATCH] KEYS: reject descriptions exceeding U16_MAX in __key_create_or_update()

[Jarkko Sakkinen <jarkko@kernel.org>]

On Sat, Sep 19, 2026 at 08:35:11PM +0000, Hui Peng wrote:
> struct keyring_index_key stores the key description length in a 16-bit
> field (u16 desc_len), and __key_link_begin() enforces
> BUG_ON(index_key->desc_len == 0).
> 
> While add_key() bounds explicit userspace description strings to
> KEY_MAX_DESC_SIZE (4096), when the userspace description is NULL or
> empty __key_create_or_update() delegates description generation to
> index_key.type->preparse(&prep) on the payload (up to 1 MiB). Key types
> such as "asymmetric" (x509_key_preparse()) construct prep.description
> from the certificate issuer, subject, and hex-encoded serial number,
> which can exceed 65535 bytes.
> 
> __key_create_or_update() then assigns:
> 
>         index_key.desc_len = strlen(index_key.description);
> 
> without checking whether the length fits in u16. When
> strlen(index_key.description) is 65536 (0x10000), index_key.desc_len
> truncates to 0 and immediately triggers BUG_ON(index_key->desc_len == 0)
> in __key_link_begin(). Lengths above 65536 truncate modulo 65536,
> corrupting keyring hash and comparison lengths.
> 
> Reject empty descriptions or descriptions longer than U16_MAX with
> -EINVAL in __key_create_or_update() and key_alloc().
> 
> Fixes: 16feef434017 ("KEYS: Consolidate the concept of an 'index key' for key access")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>

Were you able to construct X.509 certificate that triggers the bug?

I do think range checks should exist and current implementation leaves
staying within appropriate range somewhat open.

> ---
> diff --git a/security/keys/key.c b/security/keys/key.c
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -245,6 +245,8 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>  	}
>  
>  	desclen = strlen(desc);
> +	if (desclen > U16_MAX)
> +		goto error;
>  	quotalen = desclen + 1 + type->def_datalen;
>  
>  	/* get hold of the key tracking for this user */
> @@ -820,6 +822,7 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
>  	const struct cred *cred = current_cred();
>  	struct key *keyring, *key = NULL;
>  	key_ref_t key_ref;
> +	size_t desc_len;
>  	int ret;
>  	struct key_restriction *restrict_link = NULL;
>  
> @@ -865,7 +868,11 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
>  		if (!index_key.description)
>  			goto error_free_prep;
>  	}
> -	index_key.desc_len = strlen(index_key.description);
> +	desc_len = strlen(index_key.description);
> +	key_ref = ERR_PTR(-EINVAL);

Why key_ref is assigned here?

> +	if (!desc_len || desc_len > U16_MAX)
> +		goto error_free_prep;
> +	index_key.desc_len = desc_len;
>  	key_set_index_key(&index_key);
>  
>  	ret = __key_link_lock(keyring, &index_key);
> -- 
> 2.43.0

Br, Jarkko