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

[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

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

[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

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 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 2/6] KEYS: Reject @desclen values >= KEY_MAX_DESC_SIZE in key_alloc()

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 11:46:58PM +0300, Jarkko Sakkinen wrote:
> 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.

Please correct if I'm wrong but I think these don't need anything 
special given that both are capped by strndup().

Br, Jarkko

[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

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

[Jarkko Sakkinen]

On Mon, Oct 19, 2026 at 11:53:12PM +0300, Jarkko Sakkinen wrote:
> 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.

OK so this is me misleading as my description is missing one important
detail: KEY_ALLOC_QUOTA_OVERRUN bypasses the maxkeys and maxbytes
checks.

This is the exact reason why this fix is needed and commit message
does not properly address that i.e. lacks the actual root cause.

Also:

1. keyctl_chown_key() checks against maxkeys and maxbytes so it 
   is clear.
2. key_payload_reserve() checs against maxbytes.

Br, Jarkko

[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

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

[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

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 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 5/6] KEYS: Reject @datalen values over INT_MAX

[Jarkko Sakkinen]

On Tue, Oct 20, 2026 at 12:07:32AM +0300, Jarkko Sakkinen wrote:
> 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.

While it is true that add_key() has 1 MiB limit there's also
root quota. That was what I was originally thinking about
now that I scratched my head a bit.

But...

However, it is capped by /proc/sys/kernel/keys/root_maxbytes, 
SYSCTL_INT_MAX already.

So that makes the check this patch adds irrelevant, and thus
it should be dropped.

Phew... At least it is now investigated in-depth enough, I
think.

Br, Jarkko

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

[Jarkko Sakkinen]

On Tue, Oct 20, 2026 at 02:55:41AM +0300, Jarkko Sakkinen wrote:
> On Tue, Oct 20, 2026 at 12:07:32AM +0300, Jarkko Sakkinen wrote:
> > 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.
> 
> While it is true that add_key() has 1 MiB limit there's also
> root quota. That was what I was originally thinking about
> now that I scratched my head a bit.
> 
> But...
> 
> However, it is capped by /proc/sys/kernel/keys/root_maxbytes, 
> SYSCTL_INT_MAX already.
> 
> So that makes the check this patch adds irrelevant, and thus
> it should be dropped.
> 
> Phew... At least it is now investigated in-depth enough, I
> think.

This also makes the other safe-guard useless, so I'll drop
that too.

And likely expand the patch set to address the entry points
that you mentioned.

Br, Jarkko

[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

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

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

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>
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Link: https://lore.kernel.org/r/20261020174325.2229448-2-jarkko@kernel.org
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 fs/smb/client/cifsacl.c | 6 +++---
 include/linux/key.h     | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index d1a92bb4d9a5..e7f7ccf2a158 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;
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