[PATCH v3 0/3] KEYS: rhashtable bug fixes

[PATCH v3 0/3] KEYS: rhashtable bug fixes

[Jarkko Sakkinen]

My patch set that migrates assoc_array to rhashtable had some LLM
scavegend bug fixes. Unfortunately most them where either plain wrong or
not in the right scope. The bug fixes in this patch set are by human
labour constructed fully but they are derived from what I got out of the
original LLM source. Thus, they are tagged as LLM assisted.

Changelog
=========

- v3: Added couple of example, one of my own and other derived from
  David's snippet.
- v3: Removed bunch of unnecessary INT_MAX checks (Eric).
- v2: https://lore.kernel.org/keyrings/20261019181732.449591-1-jarkko@kernel.org/
- v2: Addressed issues pointed out by David.
- v1: https://lore.kernel.org/keyrings/20261019142516.300040-1-jarkko@kernel.org/

Jarkko Sakkinen (3):
  KEYS: Expand key->{datalen,quotalen} to unsigned int
  KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()
  KEYS: Verify the key identity in __key_unlink_begin

 include/linux/key.h     | 6 ++++--
 security/keys/key.c     | 2 ++
 security/keys/keyctl.c  | 2 --
 security/keys/keyring.c | 6 ++++++
 4 files changed, 12 insertions(+), 4 deletions(-)

--
2.47.3

[PATCH v3 1/3] KEYS: Expand key->{datalen,quotalen} to unsigned int

[Jarkko Sakkinen]

In key_alloc() and key_payload_reserve(), key->quotalen and key->datalen
can receive values larger than USHRT_MAX.

Example:

  keyctl new_session >/dev/null
  owner=$(keyctl newring owner @s)
  keeper=$(keyctl newring keeper @s)

  for ((i=0; i<16384; i++)); do
      key=$(keyctl add user "k$i" x "$keeper")
      keyctl link "$key" "$owner"
  done

  awk -v uid="$(id -u):" '$1 == uid {print $NF}' /proc/key-users
  keyctl clear "$owner"
  awk -v uid="$(id -u):" '$1 == uid {print $NF}' /proc/key-users

Because key->datalen wraps around 65535, the quota refunded upon clearing
is truncated, causing quota bytes to leak in /proc/key-users.

Change the types of key->datalen and key->quotalen from unsigned short to
unsigned int so that they can carry the full payload length without
truncation.

Cc: stable@vger.kernel.org
Assisted-by: LLM
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 include/linux/key.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index e40b5267658f..26139c3adf62 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -216,8 +216,8 @@ struct key {
 	kuid_t			uid;
 	kgid_t			gid;
 	key_perm_t		perm;		/* access permissions */
-	unsigned short		quotalen;	/* length added to quota */
-	unsigned short		datalen;	/* payload data length
+	unsigned int		quotalen;	/* length added to quota */
+	unsigned int		datalen;	/* payload data length
 						 * - may not match RCU dereferenced payload
 						 * - payload should contain own length
 						 */
-- 
2.47.3

Re: [PATCH v3 1/3] KEYS: Expand key->{datalen,quotalen} to unsigned int

[sashiko-bot]

> In key_alloc() and key_payload_reserve(), key->quotalen and key->datalen
> can receive values larger than USHRT_MAX.
> 
> Example:
> 
>   keyctl new_session >/dev/null
>   owner=$(keyctl newring owner @s)
>   keeper=$(keyctl newring keeper @s)
> 
>   for ((i=0; i<16384; i++)); do
>       key=$(keyctl add user "k$i" x "$keeper")
>       keyctl link "$key" "$owner"
>   done
> 
>   awk -v uid="$(id -u):" '$1 == uid {print $NF}' /proc/key-users
>   keyctl clear "$owner"
>   awk -v uid="$(id -u):" '$1 == uid {print $NF}' /proc/key-users
> 
> Because key->datalen wraps around 65535, the quota refunded upon clearing
> is truncated, causing quota bytes to leak in /proc/key-users.
> 
> Change the types of key->datalen and key->quotalen from unsigned short to
> unsigned int so that they can carry the full payload length without
> truncation.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reviewed-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Sashiko has reviewed this patch and found no issues.

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

Re: [PATCH v3 1/3] KEYS: Expand key->{datalen,quotalen} to unsigned int

[Eric Biggers]

On Tue, Oct 20, 2026 at 04:38:55AM +0300, Jarkko Sakkinen wrote:
> diff --git a/include/linux/key.h b/include/linux/key.h
> index e40b5267658f..26139c3adf62 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -216,8 +216,8 @@ struct key {
>  	kuid_t			uid;
>  	kgid_t			gid;
>  	key_perm_t		perm;		/* access permissions */
> -	unsigned short		quotalen;	/* length added to quota */
> -	unsigned short		datalen;	/* payload data length
> +	unsigned int		quotalen;	/* length added to quota */
> +	unsigned int		datalen;	/* payload data length
>  						 * - may not match RCU dereferenced payload
>  						 * - payload should contain own length
>  						 */

fs/smb/client/cifsacl.c contains some debug logging that prints
key::datalen using the %hu format specifier.  Those need to be updated
to %u to match the new type.

- Eric

Re: [PATCH v3 1/3] KEYS: Expand key->{datalen,quotalen} to unsigned int

[Jarkko Sakkinen]

On Tue, Oct 20, 2026 at 03:47:14PM +0000, Eric Biggers wrote:
> On Tue, Oct 20, 2026 at 04:38:55AM +0300, Jarkko Sakkinen wrote:
> > diff --git a/include/linux/key.h b/include/linux/key.h
> > index e40b5267658f..26139c3adf62 100644
> > --- a/include/linux/key.h
> > +++ b/include/linux/key.h
> > @@ -216,8 +216,8 @@ struct key {
> >  	kuid_t			uid;
> >  	kgid_t			gid;
> >  	key_perm_t		perm;		/* access permissions */
> > -	unsigned short		quotalen;	/* length added to quota */
> > -	unsigned short		datalen;	/* payload data length
> > +	unsigned int		quotalen;	/* length added to quota */
> > +	unsigned int		datalen;	/* payload data length
> >  						 * - may not match RCU dereferenced payload
> >  						 * - payload should contain own length
> >  						 */
> 
> fs/smb/client/cifsacl.c contains some debug logging that prints
> key::datalen using the %hu format specifier.  Those need to be updated
> to %u to match the new type.

Ugh, my mistake i.e., we are talking about:

diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index c5e47a835f99..de2acc7234be 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -385,7 +385,7 @@ id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid)
 		goto out_revert_creds;
 	} else if (sidkey->datalen < CIFS_SID_BASE_SIZE) {
 		rc = smb_EIO1(smb_eio_trace_malformed_sid_key, sidkey->datalen);
-		cifs_dbg(FYI, "%s: Downcall contained malformed key (datalen=%hu)\n",
+		cifs_dbg(FYI, "%s: Downcall contained malformed key (datalen=%u)\n",
 			 __func__, sidkey->datalen);
 		goto invalidate_key;
 	}
@@ -403,7 +403,7 @@ id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid)
 	if (ksid_size > sidkey->datalen) {
 		rc = smb_EIO2(smb_eio_trace_malformed_ksid_key,
 			      ksid_size, sidkey->datalen);
-		cifs_dbg(FYI, "%s: Downcall contained malformed key (datalen=%hu, ksid_size=%u)\n",
+		cifs_dbg(FYI, "%s: Downcall contained malformed key (datalen=%u, ksid_size=%u)\n",
 			 __func__, sidkey->datalen, ksid_size);
 		goto invalidate_key;
 	}
@@ -498,7 +498,7 @@ sid_to_id(struct cifs_sb_info *cifs_sb, struct smb_sid *psid,
 	 */
 	BUILD_BUG_ON(sizeof(uid_t) != sizeof(gid_t));
 	if (sidkey->datalen != sizeof(uid_t)) {
-		cifs_dbg(FYI, "%s: Downcall contained malformed key (datalen=%hu)\n",
+		cifs_dbg(FYI, "%s: Downcall contained malformed key (datalen=%u)\n",
 			 __func__, sidkey->datalen);
 		key_invalidate(sidkey);
 		goto out_key_put;

I'll include this as part of the patch exactly.

> 
> - Eric

Br, Jarkko

[PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Jarkko Sakkinen]

@desclen in key_alloc() is saved to the 16-bit wide desc_len field in
struct keyring_index_key.

Reject @desclen values at or above KEY_MAX_DESC_SIZE with -EINVAL in order
to match the userspace limit. The constraints inherits from the one byte
required for the NULL character.

Cc: stable@vger.kernel.org
Assisted-by: LLM
Fixes: f771fde82051 ("keys: Simplify key description management")
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 include/linux/key.h    | 2 ++
 security/keys/key.c    | 2 ++
 security/keys/keyctl.c | 2 --
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 26139c3adf62..fa88de9b5a9b 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -37,6 +37,8 @@ struct net;
 
 #undef KEY_DEBUGGING
 
+#define KEY_MAX_DESC_SIZE 4096
+
 #define KEY_POS_VIEW	0x01000000	/* possessor can view a key's attributes */
 #define KEY_POS_READ	0x02000000	/* possessor can read key payload / view keyring */
 #define KEY_POS_WRITE	0x04000000	/* possessor can update key payload / add link to keyring */
diff --git a/security/keys/key.c b/security/keys/key.c
index 6ebb8691551f..e9f8cacf00c8 100644
--- 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 >= KEY_MAX_DESC_SIZE)
+		goto error;
 	quotalen = desclen + 1 + type->def_datalen;
 
 	/* get hold of the key tracking for this user */
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 36ffba0ebba6..d6b10fd67627 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -24,8 +24,6 @@
 #include <keys/request_key_auth-type.h>
 #include "internal.h"
 
-#define KEY_MAX_DESC_SIZE 4096
-
 static const unsigned char keyrings_capabilities[2] = {
 	[0] = (KEYCTL_CAPS0_CAPABILITIES |
 	       (IS_ENABLED(CONFIG_PERSISTENT_KEYRINGS)	? KEYCTL_CAPS0_PERSISTENT_KEYRINGS : 0) |
-- 
2.47.3

Re: [PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Jarkko Sakkinen]

On Tue, Oct 20, 2026 at 04:38:56AM +0300, Jarkko Sakkinen wrote:
> @desclen in key_alloc() is saved to the 16-bit wide desc_len field in
> struct keyring_index_key.
> 
> Reject @desclen values at or above KEY_MAX_DESC_SIZE with -EINVAL in order
> to match the userspace limit. The constraints inherits from the one byte
> required for the NULL character.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Reviewed-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

This is now the only one, which does not have keyutils script in the
commit message so in that sense I think these fixes look pretty good.

Br, Jarkko

Re: [PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Eric Biggers]

On Tue, Oct 20, 2026 at 04:47:43AM +0300, Jarkko Sakkinen wrote:
> On Tue, Oct 20, 2026 at 04:38:56AM +0300, Jarkko Sakkinen wrote:
> > @desclen in key_alloc() is saved to the 16-bit wide desc_len field in
> > struct keyring_index_key.
> > 
> > Reject @desclen values at or above KEY_MAX_DESC_SIZE with -EINVAL in order
> > to match the userspace limit. The constraints inherits from the one byte
> > required for the NULL character.
> > 
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Fixes: f771fde82051 ("keys: Simplify key description management")
> > Reviewed-by: David Howells <dhowells@redhat.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> This is now the only one, which does not have keyutils script in the
> commit message so in that sense I think these fixes look pretty good.

Sure, this fix is still incomplete though, as I mentioned.

Turns out Sashiko found this too; see the comment on
https://sashiko.dev/#/patchset/20261020013903.1244157-1-jarkko%40kernel.org

- Eric

Re: [PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 07:28:32PM -0700, Eric Biggers wrote:
> On Tue, Oct 20, 2026 at 04:47:43AM +0300, Jarkko Sakkinen wrote:
> > On Tue, Oct 20, 2026 at 04:38:56AM +0300, Jarkko Sakkinen wrote:
> > > @desclen in key_alloc() is saved to the 16-bit wide desc_len field in
> > > struct keyring_index_key.
> > > 
> > > Reject @desclen values at or above KEY_MAX_DESC_SIZE with -EINVAL in order
> > > to match the userspace limit. The constraints inherits from the one byte
> > > required for the NULL character.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: LLM
> > > Fixes: f771fde82051 ("keys: Simplify key description management")
> > > Reviewed-by: David Howells <dhowells@redhat.com>
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > This is now the only one, which does not have keyutils script in the
> > commit message so in that sense I think these fixes look pretty good.
> 
> Sure, this fix is still incomplete though, as I mentioned.
> 
> Turns out Sashiko found this too; see the comment on
> https://sashiko.dev/#/patchset/20261020013903.1244157-1-jarkko%40kernel.org

I did not ignore what you said honestly but I overlooked generated
descriptions.

By comparing your v2 response and Sashiko's response and going through the
code this is the list that I have where similar check should be placed:

- key_alloc(): before quota calculation (in place)
- __key_create_or_update():  before assigning index_key.desc_len
- keyring_search(): before match_preparse
- request_key_and_link(): before match_preparse 
- request_key_rcu(): before check_cached_key()

Some sites might require some minor changes I just wrote the around the
ballpark locations.

Did I miss any site that you are aware of?

Because there are so many, despite simple check I also wonder, should
we have a macro for this check?

> 
> - Eric

Br, Jarkko

Re: [PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Jarkko Sakkinen]

On Tue, Oct 20, 2026 at 01:57:19PM +0300, Jarkko Sakkinen wrote:
> > Sure, this fix is still incomplete though, as I mentioned.
> > 
> > Turns out Sashiko found this too; see the comment on
> > https://sashiko.dev/#/patchset/20261020013903.1244157-1-jarkko%40kernel.org
> 
> I did not ignore what you said honestly but I overlooked generated
> descriptions.
> 
> By comparing your v2 response and Sashiko's response and going through the
> code this is the list that I have where similar check should be placed:
> 
> - key_alloc(): before quota calculation (in place)
> - __key_create_or_update():  before assigning index_key.desc_len
> - keyring_search(): before match_preparse
> - request_key_and_link(): before match_preparse 
> - request_key_rcu(): before check_cached_key()
> 
> Some sites might require some minor changes I just wrote the around the
> ballpark locations.
> 
> Did I miss any site that you are aware of?
> 
> Because there are so many, despite simple check I also wonder, should
> we have a macro for this check?

It's better that I show the diff I'm talking about. With the diff below
I try to address all the sites I mentioned.

Couple of open questions:

- Would it make sense to wrap strlen + check into key_is_valid_desc()?
- Would it make sense as a safe-guard to use strnlen(), which is capped
  to KEY_MAX_DESC_SIZE?

diff --git a/include/linux/key.h b/include/linux/key.h
index 26139c3adf62..3b8af14fb323 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -37,6 +37,9 @@ struct net;

 #undef KEY_DEBUGGING

+/* Maximum length of a key description, including the trailing NUL */
+#define KEY_MAX_DESC_SIZE 4096
+
 #define KEY_POS_VIEW	0x01000000	/* possessor can view a key's attributes */
 #define KEY_POS_READ	0x02000000	/* possessor can read key payload / view keyring */
 #define KEY_POS_WRITE	0x04000000	/* possessor can update key payload / add link to keyring */
diff --git a/security/keys/key.c b/security/keys/key.c
index 6ebb8691551f..e88548f1a704 100644
--- 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 >= KEY_MAX_DESC_SIZE)
+		goto error;
 	quotalen = desclen + 1 + type->def_datalen;

 	/* get hold of the key tracking for this user */
@@ -829,6 +831,7 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
 	key_ref_t key_ref;
 	int ret;
 	struct key_restriction *restrict_link = NULL;
+	size_t desclen;

 	/* look up the key type to see if it's one of the registered kernel
 	 * types */
@@ -869,7 +872,12 @@ 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);
+	desclen = strlen(index_key.description);
+	if (desclen >= KEY_MAX_DESC_SIZE) {
+		key_ref = ERR_PTR(-EINVAL);
+		goto error_free_prep;
+	}
+	index_key.desc_len = desclen;
 	key_set_index_key(&index_key);

 	ret = __key_link_lock(keyring, &index_key);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 36ffba0ebba6..d6b10fd67627 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -24,8 +24,6 @@
 #include <keys/request_key_auth-type.h>
 #include "internal.h"

-#define KEY_MAX_DESC_SIZE 4096
-
 static const unsigned char keyrings_capabilities[2] = {
 	[0] = (KEYCTL_CAPS0_CAPABILITIES |
 	       (IS_ENABLED(CONFIG_PERSISTENT_KEYRINGS)	? KEYCTL_CAPS0_PERSISTENT_KEYRINGS : 0) |
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 7143f3079d20..2c54f7f84c55 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -957,7 +957,6 @@ key_ref_t keyring_search(key_ref_t keyring,
 	struct keyring_search_context ctx = {
 		.index_key.type		= type,
 		.index_key.description	= description,
-		.index_key.desc_len	= strlen(description),
 		.cred			= current_cred(),
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
@@ -965,8 +964,14 @@ key_ref_t keyring_search(key_ref_t keyring,
 		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
 	};
 	key_ref_t key;
+	size_t desclen;
 	int ret;

+	desclen = strlen(description);
+	if (desclen >= KEY_MAX_DESC_SIZE)
+		return ERR_PTR(-EINVAL);
+	ctx.index_key.desc_len = desclen;
+
 	if (recurse)
 		ctx.flags |= KEYRING_SEARCH_RECURSE;
 	if (type->match_preparse) {
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index fa2bb9f2f538..ef00c2b08fe0 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -584,7 +584,6 @@ struct key *request_key_and_link(struct key_type *type,
 		.index_key.type		= type,
 		.index_key.domain_tag	= domain_tag,
 		.index_key.description	= description,
-		.index_key.desc_len	= strlen(description),
 		.cred			= current_cred(),
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
@@ -595,8 +594,14 @@ struct key *request_key_and_link(struct key_type *type,
 	};
 	struct key *key;
 	key_ref_t key_ref;
+	size_t desclen;
 	int ret;

+	desclen = strlen(description);
+	if (desclen >= KEY_MAX_DESC_SIZE)
+		return ERR_PTR(-EINVAL);
+	ctx.index_key.desc_len = desclen;
+
 	kenter("%s,%s,%p,%zu,%p,%p,%lx",
 	       ctx.index_key.type->name, ctx.index_key.description,
 	       callout_info, callout_len, aux, dest_keyring, flags);
@@ -787,7 +792,6 @@ struct key *request_key_rcu(struct key_type *type,
 		.index_key.type		= type,
 		.index_key.domain_tag	= domain_tag,
 		.index_key.description	= description,
-		.index_key.desc_len	= strlen(description),
 		.cred			= current_cred(),
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
@@ -797,6 +801,12 @@ struct key *request_key_rcu(struct key_type *type,
 	};
 	struct key *key;
 	key_ref_t key_ref;
+	size_t desclen;
+
+	desclen = strlen(description);
+	if (desclen >= KEY_MAX_DESC_SIZE)
+		return ERR_PTR(-EINVAL);
+	ctx.index_key.desc_len = desclen;

 	kenter("%s,%s", type->name, description);

Br, Jarkko

Re: [PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Eric Biggers]

On Tue, Oct 20, 2026 at 04:00:31PM +0300, Jarkko Sakkinen wrote:
> On Tue, Oct 20, 2026 at 01:57:19PM +0300, Jarkko Sakkinen wrote:
> > > Sure, this fix is still incomplete though, as I mentioned.
> > > 
> > > Turns out Sashiko found this too; see the comment on
> > > https://sashiko.dev/#/patchset/20261020013903.1244157-1-jarkko%40kernel.org
> > 
> > I did not ignore what you said honestly but I overlooked generated
> > descriptions.
> > 
> > By comparing your v2 response and Sashiko's response and going through the
> > code this is the list that I have where similar check should be placed:
> > 
> > - key_alloc(): before quota calculation (in place)
> > - __key_create_or_update():  before assigning index_key.desc_len
> > - keyring_search(): before match_preparse
> > - request_key_and_link(): before match_preparse 
> > - request_key_rcu(): before check_cached_key()
> > 
> > Some sites might require some minor changes I just wrote the around the
> > ballpark locations.
> > 
> > Did I miss any site that you are aware of?

It looks okay to me.

> - Would it make sense to wrap strlen + check into key_is_valid_desc()?

I don't think it would be very helpful.

> - Would it make sense as a safe-guard to use strnlen(), which is capped
>   to KEY_MAX_DESC_SIZE?

Not really needed.  They are NUL-terminated strings anyway.

- Eric

Re: [PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Jarkko Sakkinen]

On Tue, Oct 20, 2026 at 03:50:13PM +0000, Eric Biggers wrote:
> On Tue, Oct 20, 2026 at 04:00:31PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Oct 20, 2026 at 01:57:19PM +0300, Jarkko Sakkinen wrote:
> > > > Sure, this fix is still incomplete though, as I mentioned.
> > > > 
> > > > Turns out Sashiko found this too; see the comment on
> > > > https://sashiko.dev/#/patchset/20261020013903.1244157-1-jarkko%40kernel.org
> > > 
> > > I did not ignore what you said honestly but I overlooked generated
> > > descriptions.
> > > 
> > > By comparing your v2 response and Sashiko's response and going through the
> > > code this is the list that I have where similar check should be placed:
> > > 
> > > - key_alloc(): before quota calculation (in place)
> > > - __key_create_or_update():  before assigning index_key.desc_len
> > > - keyring_search(): before match_preparse
> > > - request_key_and_link(): before match_preparse 
> > > - request_key_rcu(): before check_cached_key()
> > > 
> > > Some sites might require some minor changes I just wrote the around the
> > > ballpark locations.
> > > 
> > > Did I miss any site that you are aware of?
> 
> It looks okay to me.
> 
> > - Would it make sense to wrap strlen + check into key_is_valid_desc()?
> 
> I don't think it would be very helpful.

I tend to agree.

> 
> > - Would it make sense as a safe-guard to use strnlen(), which is capped
> >   to KEY_MAX_DESC_SIZE?
> 
> Not really needed.  They are NUL-terminated strings anyway.

This is what I thought based on that in all of the sites I could
determine where the string is create/sourced.

Just wanted to sanity check so that this does not require too
many additional iterations.

> 
> - Eric

Br, Jarkko

Re: [PATCH v3 2/3] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] keys: u16 desc_len truncation in __key_create_or_update() before key_alloc()
--

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

KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

This commit moves KEY_MAX_DESC_SIZE to include/linux/key.h and rejects
descriptions with length at or above KEY_MAX_DESC_SIZE in key_alloc() to
prevent overflowing the 16-bit desc_len field in struct keyring_index_key.

> diff --git a/security/keys/key.c b/security/keys/key.c
> index a438c4508595d..aa0112a693e68 100644
> --- 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 >= KEY_MAX_DESC_SIZE)
> +		goto error;
>  	quotalen = desclen + 1 + type->def_datalen;
>  
>  	/* get hold of the key tracking for this user */

[Severity: Critical]
Is checking desclen in key_alloc() too late when a key is created or
updated through __key_create_or_update()?

In security/keys/key.c, __key_create_or_update() can obtain
index_key.description from index_key.type->preparse(&prep) (for example,
via add_key() with a NULL description and the "asymmetric" key type, where
x509_key_preparse() in crypto/asymmetric_keys/x509_public_key.c constructs
prep.description from the X.509 subject and hex-encoded serial or skid
without bounding it to KEY_MAX_DESC_SIZE) or from an in-kernel caller of
key_create_or_update().

Before calling key_alloc(), __key_create_or_update() assigns the length
directly into the 16-bit index_key.desc_len field and passes index_key to
the keyring lookup and linking helpers:

security/keys/key.c:__key_create_or_update() {
	if (index_key.type->preparse) {
		ret = index_key.type->preparse(&prep);
		if (ret < 0) {
			key_ref = ERR_PTR(ret);
			goto error_free_prep;
		}
		if (!index_key.description)
			index_key.description = prep.description;
		key_ref = ERR_PTR(-EINVAL);
		if (!index_key.description)
			goto error_free_prep;
	}
	index_key.desc_len = strlen(index_key.description);
	key_set_index_key(&index_key);

	ret = __key_link_lock(keyring, &index_key);
	if (ret < 0) {
		key_ref = ERR_PTR(ret);
		goto error_free_prep;
	}

	ret = __key_link_begin(keyring, &index_key, &edit);
	...
	key_ref = find_key_to_update(keyring_ref, &index_key);
	if (key_ref)
		goto found_matching_key;
	...
	key = key_alloc(index_key.type, index_key.description,
			cred->fsuid, cred->fsgid, cred, perm, flags, NULL);
}

If strlen(index_key.description) is 65536 (which x509_key_preparse() can
produce from a ~33 KB certificate payload because bin2hex() doubles the
serial or skid length), truncating it to the u16 index_key.desc_len yields
0 and hits BUG_ON(index_key->desc_len == 0) in __key_link_begin() in
security/keys/keyring.c before key_alloc() is ever reached:

security/keys/keyring.c:__key_link_begin() {
	...
	BUG_ON(index_key->desc_len == 0);
}

Similarly, if strlen(index_key.description) is >= KEY_MAX_DESC_SIZE and not
a multiple of 65536, the unvalidated or truncated desc_len is used by
key_set_index_key(), __key_link_begin(), and find_key_to_update(), which
can match and update an existing key with that shorter prefix via
goto found_matching_key without calling key_alloc(). Other paths that
populate struct keyring_index_key directly, such as keyring_search() in
security/keys/keyring.c and request_key_and_link() / request_key_rcu() in
security/keys/request_key.c, also assign index_key.desc_len =
strlen(description) without checking against KEY_MAX_DESC_SIZE.

Should the KEY_MAX_DESC_SIZE check also be enforced in
__key_create_or_update() (and other struct keyring_index_key initialization
paths) before assigning index_key.desc_len?

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

[PATCH v3 3/3] KEYS: Verify the key identity in __key_unlink_begin

[Jarkko Sakkinen]

When two keys have identical type and description, linking the second key
replaces the first in a keyring. However, attempting to unlink the first
key (no longer in the keyring) unlinks the second key.

This happens because __key_unlink_begin() calls assoc_array_delete() with
&key->index_key, matching only on type and description rather than the
key's identity.

Example:

  keyctl new_session >/dev/null
  keyctl newring ring_a @s
  keyctl newring ring_b @s
  A=$(keyctl add user A ss %:ring_a)
  B=$(keyctl add user A qq %:ring_b)
  keyctl newring R @s
  keyctl link $A %:R
  keyctl link $B %:R
  keyctl unlink $A %:R
  keyctl show

Verify the key identity before unlinking, and return -ENOENT on mismatch,
instead of unlinking the key with the wrong identity.

Cc: stable@vger.kernel.org
Assisted-by: LLM
Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/keyring.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 7143f3079d20..b025331488c5 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1501,9 +1501,15 @@ static int __key_unlink_begin(struct key *keyring, struct key *key,
 			      struct assoc_array_edit **_edit)
 {
 	struct assoc_array_edit *edit;
+	const void *object;
 
 	BUG_ON(*_edit != NULL);
 
+	object = assoc_array_find(&keyring->keys, &keyring_assoc_array_ops,
+				  &key->index_key);
+	if (!object || keyring_ptr_to_key(object) != key)
+		return -ENOENT;
+
 	edit = assoc_array_delete(&keyring->keys, &keyring_assoc_array_ops,
 				  &key->index_key);
 	if (IS_ERR(edit))
-- 
2.47.3

Re: [PATCH v3 3/3] KEYS: Verify the key identity in __key_unlink_begin

[sashiko-bot]

> When two keys have identical type and description, linking the second key
> replaces the first in a keyring. However, attempting to unlink the first
> key (no longer in the keyring) unlinks the second key.
> 
> This happens because __key_unlink_begin() calls assoc_array_delete() with
> &key->index_key, matching only on type and description rather than the
> key's identity.
> 
> Example:
> 
>   keyctl new_session >/dev/null
>   keyctl newring ring_a @s
>   keyctl newring ring_b @s
>   A=$(keyctl add user A ss %:ring_a)
>   B=$(keyctl add user A qq %:ring_b)
>   keyctl newring R @s
>   keyctl link $A %:R
>   keyctl link $B %:R
>   keyctl unlink $A %:R
>   keyctl show
> 
> Verify the key identity before unlinking, and return -ENOENT on mismatch,
> instead of unlinking the key with the wrong identity.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
> Reviewed-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Sashiko has reviewed this patch and found no issues.

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