[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

[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

[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 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