[PATCH v2 0/6] KEYS: rhashtable bug fixes

[PATCH v2 0/6] 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
=========

- v2: There was enough tweaks that a sanity check round makes sense just
  in case.
- v2: Addressed issues pointed out by David.
- v1: https://lore.kernel.org/keyrings/20261019142516.300040-1-jarkko@kernel.org/

Jarkko Sakkinen (6):
  KEYS: Expand key->{datalen,quotalen} to unsigned int
  KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()
  KEYS: Prevent overflow of user->{qnkeys,qnbytes} in key_alloc()
  KEYS: Prevent overflow of quota in key_alloc()
  KEYS: Reject @datalen values over INT_MAX
  KEYS: Verify the key identity in __keyctl_unlink

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

--
2.47.3

[PATCH v2 1/6] 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.

Change their types from unsigned short to unsigned int so that they can
carry the full payload and avoid 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 v2 2/6] 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 account for the NUL terminator and prevent integer overflow.

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 v2 3/6] KEYS: Prevent overflow of user->{qnkeys,qnbytes} in key_alloc()

[Jarkko Sakkinen]

Add the missing overflow checks for user->qnkeys and user->qnbytes
in order keep calculations within INT_MAX range, which is the
absolute maximum quota.

Cc: stable@vger.kernel.org
Assisted-by: LLM
Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/key.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/security/keys/key.c b/security/keys/key.c
index e9f8cacf00c8..d4e4a2e89a20 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -263,6 +263,10 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 			key_quota_root_maxbytes : key_quota_maxbytes;
 
 		spin_lock_irqsave(&user->lock, irqflags);
+		if (user->qnkeys == INT_MAX ||
+		    quotalen > INT_MAX - user->qnbytes)
+			goto no_quota;
+
 		if (!(flags & KEY_ALLOC_QUOTA_OVERRUN)) {
 			if (user->qnkeys + 1 > maxkeys ||
 			    user->qnbytes + quotalen > maxbytes ||
-- 
2.47.3

[PATCH v2 4/6] KEYS: Prevent overflow of quota in key_alloc()

[Jarkko Sakkinen]

Check that the calculated quota length does not go over INT_MAX.

Cc: stable@vger.kernel.org
Assisted-by: LLM
Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 security/keys/key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index d4e4a2e89a20..dbf5626a48b7 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -245,7 +245,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	}
 
 	desclen = strlen(desc);
-	if (desclen >= KEY_MAX_DESC_SIZE)
+	if (desclen >= KEY_MAX_DESC_SIZE || type->def_datalen > INT_MAX - desclen - 1)
 		goto error;
 	quotalen = desclen + 1 + type->def_datalen;
 
-- 
2.47.3

[PATCH v2 5/6] KEYS: Reject @datalen values over INT_MAX

[Jarkko Sakkinen]

Protect quota from overflowing by rejecting @datalen values that
surpass over INT_MAX.

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>
---
 security/keys/key.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/security/keys/key.c b/security/keys/key.c
index dbf5626a48b7..fbd608634bde 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -384,6 +384,9 @@ int key_payload_reserve(struct key *key, size_t datalen)
 
 	key_check(key);
 
+	if (datalen > INT_MAX)
+		return -EINVAL;
+
 	/* contemplate the quota adjustment */
 	if (delta != 0 && test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
 		unsigned maxbytes = uid_eq(key->user->uid, GLOBAL_ROOT_UID) ?
-- 
2.47.3

[PATCH v2 6/6] KEYS: Verify the key identity in __keyctl_unlink

[Jarkko Sakkinen]

Let's define the following preconditions:

- Two keys: A and B.
- R is a keyring.
- Both have the same type/description/domain.

Then:

1. KEYCTL_LINK(A, R) => {A}
1. KEYCTL_LINK(B, R) => {B}
2. KEYCTL_UNLINK(A, R) => {}

Instead, verify key tidentity before unlinking, and return -ENOENT on
mismatch.

Assisted-by: LLM
Cc: stable@vger.kernel.org
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 v2 6/6] KEYS: Verify the key identity in __keyctl_unlink

[sashiko-bot]

> Let's define the following preconditions:
> 
> - Two keys: A and B.
> - R is a keyring.
> - Both have the same type/description/domain.
> 
> Then:
> 
> 1. KEYCTL_LINK(A, R) => {A}
> 1. KEYCTL_LINK(B, R) => {B}
> 2. KEYCTL_UNLINK(A, R) => {}
> 
> Instead, verify key tidentity before unlinking, and return -ENOENT on
> mismatch.
> 
> Assisted-by: LLM
> Cc: stable@vger.kernel.org
> 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/20261019181732.449591-1-jarkko@kernel.org?part=6

Re: [PATCH v2 4/6] KEYS: Prevent overflow of quota in key_alloc()

[sashiko-bot]

> Check that the calculated quota length does not go over INT_MAX.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
> 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/20261019181732.449591-1-jarkko@kernel.org?part=4

Re: [PATCH v2 5/6] KEYS: Reject @datalen values over INT_MAX

[sashiko-bot]

> Protect quota from overflowing by rejecting @datalen values that
> surpass over INT_MAX.
> 
> 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/20261019181732.449591-1-jarkko@kernel.org?part=5

Re: [PATCH v2 1/6] 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.
> 
> Change their types from unsigned short to unsigned int so that they can
> carry the full payload and avoid 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/20261019181732.449591-1-jarkko@kernel.org?part=1

Re: [PATCH v2 3/6] KEYS: Prevent overflow of user->{qnkeys,qnbytes} in key_alloc()

[sashiko-bot]

> Add the missing overflow checks for user->qnkeys and user->qnbytes
> in order keep calculations within INT_MAX range, which is the
> absolute maximum quota.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
> 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/20261019181732.449591-1-jarkko@kernel.org?part=3

Re: [PATCH v2 0/6] KEYS: rhashtable bug fixes

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 09:17:20PM +0300, Jarkko Sakkinen wrote:
> 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
> =========
> 
> - v2: There was enough tweaks that a sanity check round makes sense just
>   in case.
> - v2: Addressed issues pointed out by David.
> - v1: https://lore.kernel.org/keyrings/20261019142516.300040-1-jarkko@kernel.org/
> 
> Jarkko Sakkinen (6):
>   KEYS: Expand key->{datalen,quotalen} to unsigned int
>   KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()
>   KEYS: Prevent overflow of user->{qnkeys,qnbytes} in key_alloc()
>   KEYS: Prevent overflow of quota in key_alloc()
>   KEYS: Reject @datalen values over INT_MAX
>   KEYS: Verify the key identity in __keyctl_unlink
> 
>  include/linux/key.h     | 6 ++++--
>  security/keys/key.c     | 9 +++++++++
>  security/keys/keyctl.c  | 2 --
>  security/keys/keyring.c | 6 ++++++
>  4 files changed, 19 insertions(+), 4 deletions(-)
> 
> --
> 2.47.3
> 

The benefit with this slicing is at least that backport conflicts should
be easy to resolve, and if I recall correctly all go to v5.10+ stable
kernels.

Br, Jarkko

Re: [PATCH v2 2/6] 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: integer truncation of desc_len in __key_create_or_update()
--

commit 40e3162313abde79a8374f519fc697c857a50981
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 adds a check
in key_alloc() to reject descriptions with a length at or above
KEY_MAX_DESC_SIZE with -EINVAL so that the 16-bit desc_len field in struct
keyring_index_key does not overflow.

> diff --git a/security/keys/key.c b/security/keys/key.c
> index b34a64d81d47a..4f52245581a4c 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 for the __key_create_or_update()
path?

In security/keys/key.c:__key_create_or_update(), strlen(index_key.description)
is assigned to the 16-bit index_key.desc_len field and passed to
key_set_index_key(), __key_link_begin(), and find_key_to_update() before
key_alloc() is ever called:

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);
	...
}

When userspace calls sys_add_key() with a NULL _description, the
strndup_user(_description, KEY_MAX_DESC_SIZE) check in sys_add_key() is
skipped and index_key.type->preparse() can generate prep.description from the
payload (for example, x509_key_preparse() in
crypto/asymmetric_keys/x509_public_key.c builds prep->description from the
certificate subject and serial/SKID without a KEY_MAX_DESC_SIZE limit).
In-kernel callers of key_create_or_update() and key_create() also pass
description directly into __key_create_or_update().

If strlen(index_key.description) is a multiple of 65536 (or 0),
index_key.desc_len truncates to 0 and triggers BUG_ON(index_key->desc_len == 0)
in security/keys/keyring.c:__key_link_begin() before key_alloc() is reached.
If strlen(index_key.description) > 65535 truncates to a non-zero 16-bit length
L, key_set_index_key() and find_key_to_update() hash and match only the first
L bytes of index_key.description, which can collide with and update an
existing key with that L-byte prefix without ever reaching key_alloc().

Should __key_create_or_update() also validate that the description length is
non-zero and less than KEY_MAX_DESC_SIZE before assigning index_key.desc_len?

[ ... ]

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

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

[Eric Biggers]

On Mon, Oct 19, 2026 at 09:17:21PM +0300, Jarkko Sakkinen wrote:
> In key_alloc() and key_payload_reserve(), key->quotalen and key->datalen
> can receive values larger than USHRT_MAX.

Could use an explanation about how this can happen.  A keyring with more
than 16383 links, I guess?

- Eric

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

[Eric Biggers]

On Mon, Oct 19, 2026 at 09:17:22PM +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 account for the NUL terminator and prevent integer overflow.

This should say something like: "in order to match the userspace limit
and avoid u16 truncation".

There are also similar truncations of desc_len to u16 in
key_create_or_update() and keyring_search().

> +#define KEY_MAX_DESC_SIZE 4096

Could use a comment mentioning that it includes the trailing NUL.

- Eric

Re: [PATCH v2 3/6] KEYS: Prevent overflow of user->{qnkeys,qnbytes} in key_alloc()

[Eric Biggers]

On Mon, Oct 19, 2026 at 09:17:23PM +0300, Jarkko Sakkinen wrote:
> Add the missing overflow checks for user->qnkeys and user->qnbytes
> in order keep calculations within INT_MAX range, which is the
> absolute maximum quota.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
> Reviewed-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Don't keyctl_chown_key() and key_payload_reserve() need similar fixes?

- Eric

Re: [PATCH v2 4/6] KEYS: Prevent overflow of quota in key_alloc()

[Eric Biggers]

On Mon, Oct 19, 2026 at 09:17:24PM +0300, Jarkko Sakkinen wrote:
> Check that the calculated quota length does not go over INT_MAX.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
> Reviewed-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Seems this should be merged into patch 3?

Also is this reachable?  I.e. do any of the key types actually specify
def_datalen > INT_MAX - KEY_MAX_DESC_SIZE + 1?

- Eric

Re: [PATCH v2 5/6] KEYS: Reject @datalen values over INT_MAX

[Eric Biggers]

On Mon, Oct 19, 2026 at 09:17:25PM +0300, Jarkko Sakkinen wrote:
> Protect quota from overflowing by rejecting @datalen values that
> surpass over INT_MAX.

Is there an example of how this can happen, and why the limit needs to
still be so large?  add_key() for example limits the payload length to
1 MiB.

- Eric

Re: [PATCH v2 6/6] KEYS: Verify the key identity in __keyctl_unlink

[Eric Biggers]

On Mon, Oct 19, 2026 at 09:17:26PM +0300, Jarkko Sakkinen wrote:
> [PATCH v2 6/6] KEYS: Verify the key identity in __keyctl_unlink

There is no __keyctl_unlink.  Seems you mean __key_unlink_begin.

> Let's define the following preconditions:
> 
> - Two keys: A and B.
> - R is a keyring.
> - Both have the same type/description/domain.
> 
> Then:
> 
> 1. KEYCTL_LINK(A, R) => {A}
> 1. KEYCTL_LINK(B, R) => {B}
> 2. KEYCTL_UNLINK(A, R) => {}

Should be 1, 2, and 3?

> Instead, verify key tidentity before unlinking, and return -ENOENT on

tidentity => identity

Also should explicitly say what the bug is.

- Eric

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

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 12:12:48PM -0700, Eric Biggers wrote:
> On Mon, Oct 19, 2026 at 09:17:21PM +0300, Jarkko Sakkinen wrote:
> > In key_alloc() and key_payload_reserve(), key->quotalen and key->datalen
> > can receive values larger than USHRT_MAX.
> 
> Could use an explanation about how this can happen.  A keyring with more
> than 16383 links, I guess?

It is good to sanity check these given the source of origin.

I tried to implement your example and I think I was successful.

Here's what I did:

	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

It is mostly safe from unprivileged user as /proc/sys/kernel/keys/maxkeys
and /proc/sys/kernel/keys/maxbytes are by default set to 200 and 20000.

When I run it as root, this what I get on first run:

252793/25000000
252793/25000000

Second run:

383862/25000000
383862/25000000

And third run for good measure:

514934/25000000
514934/25000000

So yeah, it is right on the uapi surface easily accessible :-)

> 
> - Eric

Br, Jarkko

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

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 12:17:59PM -0700, Eric Biggers wrote:
> On Mon, Oct 19, 2026 at 09:17:22PM +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 account for the NUL terminator and prevent integer overflow.
> 
> This should say something like: "in order to match the userspace limit
> and avoid u16 truncation".
> 
> There are also similar truncations of desc_len to u16 in
> key_create_or_update() and keyring_search().

Ack.

> 
> > +#define KEY_MAX_DESC_SIZE 4096
> 
> Could use a comment mentioning that it includes the trailing NUL.

OK, that improved the commit mesage somewhat:

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

    @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.

> 
> - Eric

Br, Jarkko

Re: [PATCH v2 3/6] KEYS: Prevent overflow of user->{qnkeys,qnbytes} in key_alloc()

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 12:23:32PM -0700, Eric Biggers wrote:
> On Mon, Oct 19, 2026 at 09:17:23PM +0300, Jarkko Sakkinen wrote:
> > Add the missing overflow checks for user->qnkeys and user->qnbytes
> > in order keep calculations within INT_MAX range, which is the
> > absolute maximum quota.
> > 
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
> > Reviewed-by: David Howells <dhowells@redhat.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> Don't keyctl_chown_key() and key_payload_reserve() need similar fixes?

Thanks, I'll investigate.

Unless any specific reason, I would not hold merging this series. I have
written down a follow-up backlog i.e., post-tasks once this series is
merged.

> 
> - Eric

Br, Jarkko

Re: [PATCH v2 4/6] KEYS: Prevent overflow of quota in key_alloc()

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 12:25:45PM -0700, Eric Biggers wrote:
> On Mon, Oct 19, 2026 at 09:17:24PM +0300, Jarkko Sakkinen wrote:
> > Check that the calculated quota length does not go over INT_MAX.
> > 
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
> > Reviewed-by: David Howells <dhowells@redhat.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> Seems this should be merged into patch 3?
> 
> Also is this reachable?  I.e. do any of the key types actually specify
> def_datalen > INT_MAX - KEY_MAX_DESC_SIZE + 1?

This is very good question and my *guess* is that no there is not.

That said, when not treated as a bug fix this patch is a great
safe-guard e.g., to block corraterall effects of some other
bugs.

So yeah, that's I would propose here. Let's not call it a bug fix
but still merge.

> 
> - Eric

Br, Jarkko

Re: [PATCH v2 4/6] KEYS: Prevent overflow of quota in key_alloc()

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 11:55:19PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 19, 2026 at 12:25:45PM -0700, Eric Biggers wrote:
> > On Mon, Oct 19, 2026 at 09:17:24PM +0300, Jarkko Sakkinen wrote:
> > > Check that the calculated quota length does not go over INT_MAX.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: LLM
> > > Fixes: 7e047ef5fe2d ("[PATCH] keys: sort out key quota system")
> > > Reviewed-by: David Howells <dhowells@redhat.com>
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > Seems this should be merged into patch 3?
> > 
> > Also is this reachable?  I.e. do any of the key types actually specify
> > def_datalen > INT_MAX - KEY_MAX_DESC_SIZE + 1?
> 
> This is very good question and my *guess* is that no there is not.
> 
> That said, when not treated as a bug fix this patch is a great
> safe-guard e.g., to block corraterall effects of some other
> bugs.
> 
> So yeah, that's I would propose here. Let's not call it a bug fix
> but still merge.

Or actually: no let's not do this. I'm making my life only more
difficult.

I think it is totally sane to squash this to patch 3 and it
is reasonable for that fix have this additional safe-guard.

Br, Jarkko

Re: [PATCH v2 6/6] KEYS: Verify the key identity in __keyctl_unlink

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 12:35:02PM -0700, Eric Biggers wrote:
> On Mon, Oct 19, 2026 at 09:17:26PM +0300, Jarkko Sakkinen wrote:
> > [PATCH v2 6/6] KEYS: Verify the key identity in __keyctl_unlink
> 
> There is no __keyctl_unlink.  Seems you mean __key_unlink_begin.

This is definitely my typo :-) I was looking at the implementation
in other terminal while writing this and apparently lost focus.

> 
> > Let's define the following preconditions:
> > 
> > - Two keys: A and B.
> > - R is a keyring.
> > - Both have the same type/description/domain.
> > 
> > Then:
> > 
> > 1. KEYCTL_LINK(A, R) => {A}
> > 1. KEYCTL_LINK(B, R) => {B}
> > 2. KEYCTL_UNLINK(A, R) => {}
> 
> Should be 1, 2, and 3?

I tried to describe the scenarion but apparently was not so successful.

I.e. unlinking A removes B given the match in attributes while you would
expect getting -ENOENT and B retained. Or this is the idea.

> 
> > Instead, verify key tidentity before unlinking, and return -ENOENT on
> 
> tidentity => identity
> 
> Also should explicitly say what the bug is.

I tried and failed... But yeah, the bug is illict successful removal
so there is nothing unclear on that side.

Is clearer description enough or should I supplement this keyctl
snippet?

> 
> - Eric

Br, Jarkko

Re: [PATCH v2 5/6] KEYS: Reject @datalen values over INT_MAX

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 12:31:48PM -0700, Eric Biggers wrote:
> On Mon, Oct 19, 2026 at 09:17:25PM +0300, Jarkko Sakkinen wrote:
> > Protect quota from overflowing by rejecting @datalen values that
> > surpass over INT_MAX.
> 
> Is there an example of how this can happen, and why the limit needs to
> still be so large?  add_key() for example limits the payload length to
> 1 MiB.

I did a bit scavenging, and to my knowleddge, no. So I would retain
this as safety measure but would not consider it as a bug fix.

> 
> - Eric

Br, Jarkko

Re: [PATCH v2 6/6] KEYS: Verify the key identity in __keyctl_unlink

[Jarkko Sakkinen]

On Tue, Oct 20, 2026 at 12:06:30AM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 19, 2026 at 12:35:02PM -0700, Eric Biggers wrote:
> > On Mon, Oct 19, 2026 at 09:17:26PM +0300, Jarkko Sakkinen wrote:
> > > [PATCH v2 6/6] KEYS: Verify the key identity in __keyctl_unlink
> > 
> > There is no __keyctl_unlink.  Seems you mean __key_unlink_begin.
> 
> This is definitely my typo :-) I was looking at the implementation
> in other terminal while writing this and apparently lost focus.
> 
> > 
> > > Let's define the following preconditions:
> > > 
> > > - Two keys: A and B.
> > > - R is a keyring.
> > > - Both have the same type/description/domain.
> > > 
> > > Then:
> > > 
> > > 1. KEYCTL_LINK(A, R) => {A}
> > > 1. KEYCTL_LINK(B, R) => {B}
> > > 2. KEYCTL_UNLINK(A, R) => {}
> > 
> > Should be 1, 2, and 3?
> 
> I tried to describe the scenarion but apparently was not so successful.
> 
> I.e. unlinking A removes B given the match in attributes while you would
> expect getting -ENOENT and B retained. Or this is the idea.

Here's a little bit tweaked version of David's example:

#!/bin/sh

set -e

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 show
keyctl link $A %:R
keyctl link $B %:R
keyctl unlink $A %:R
keyctl show

What I get is:

77968717
178740336
781765689
Session Keyring
 952124334 --alswrv      0     0  keyring: _ses
  77968717 --alswrv      0     0   \_ keyring: ring_a
  32558793 --alswrv      0     0   |   \_ user: A
 781765689 --alswrv      0     0   \_ keyring: R
 178740336 --alswrv      0     0   \_ keyring: ring_b
 907061159 --alswrv      0     0       \_ user: A
Session Keyring
 952124334 --alswrv      0     0  keyring: _ses
  77968717 --alswrv      0     0   \_ keyring: ring_a
  32558793 --alswrv      0     0   |   \_ user: A
 781765689 --alswrv      0     0   \_ keyring: R
 178740336 --alswrv      0     0   \_ keyring: ring_b
 907061159 --alswrv      0     0       \_ user: A

What I would expect is -ENOENT.

Br, Jarkko