[PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency

[PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency

[Michael Bommarito]

keyring_get_key_chunk() advances the description read pointer by
level * sizeof(long) past the inline prefix but only bounds-checks the
prefix, so once the associative-array walk reaches a description-level
chunk it reads past the kmemdup(desc, desc_len + 1) description
allocation.  Reaching that depth needs two keys that collide through the
hash, x, type and domain_tag chunks, which an unprivileged add_key(2)
can arrange with a crafted pair of same-type keys.

An unprivileged user can thus read up to sizeof(long) bytes past a
keyring key's description; on kernels built without init-on-alloc the
same collision, read back with KEYCTL_READ, returns uninitialized kernel
slab.

Patch 1 is the memory-safety fix and stands alone.  Patches 2 and 3 fix
two index-key consistency bugs that let the crafted keys collide into a
single malformed node in the first place, which is what enables the
KEYCTL_READ disclosure.

The KASAN reproduction is on patch 1. Trigger is available off-list.

Michael Bommarito (3):
  keys: fix out-of-bounds read in keyring_get_key_chunk()
  keys: make keyring key-chunk byte order agree with
    keyring_diff_objects()
  assoc_array: trim the final shortcut word when skip_to_level is
    chunk-aligned

 lib/assoc_array.c       |  2 +-
 security/keys/keyring.c | 15 ++++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)


base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309
--
2.53.0

[PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk()

[Michael Bommarito]

For description-level chunks keyring_get_key_chunk() advances the read
pointer by level * sizeof(long) past the inline prefix but only
bounds-checks the prefix, so a long enough key description is read past
its kmemdup(desc, desc_len + 1) allocation.  Compute the full byte
offset and bounds-check the description against it before reading.

The walk only reaches a description-level chunk when two keys collide
through the hash, x, type and domain_tag chunks, so this is reached from
an unprivileged add_key(2) with a crafted pair of same-type keys whose
index hashes collide; KASAN reports a slab-out-of-bounds read.

Fixes: f771fde82051 ("keys: Simplify key description management")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 security/keys/keyring.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

KASAN, x86_64: add_key(2) of a crafted hash-colliding "user"-key pair
(~63-byte descriptions) reports

  BUG: KASAN: slab-out-of-bounds in keyring_get_key_chunk
  keyring_get_key_chunk <- assoc_array_insert <- __key_link_begin
    <- __do_sys_add_key

reading one byte past the description allocation; the same trigger is
KASAN-clean with this patch.  On a kernel built without init-on-alloc,
reading the colliding keyring back with KEYCTL_READ returns
uninitialized slab until patches 2 and 3 are applied too.  Trigger
available off-list.

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 7a2ee0ded7c93..1739373172ad5 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -270,7 +270,7 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 	const struct keyring_index_key *index_key = data;
 	unsigned long chunk = 0;
 	const u8 *d;
-	int desc_len = index_key->desc_len, n = sizeof(chunk);
+	int desc_len = index_key->desc_len, n = sizeof(chunk), offset;
 
 	level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
 	switch (level) {
@@ -284,12 +284,12 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 		return (unsigned long)index_key->domain_tag;
 	default:
 		level -= 4;
-		if (desc_len <= sizeof(index_key->desc))
+		offset = sizeof(index_key->desc) + level * sizeof(long);
+		if (desc_len <= offset)
 			return 0;
 
-		d = index_key->description + sizeof(index_key->desc);
-		d += level * sizeof(long);
-		desc_len -= sizeof(index_key->desc);
+		d = index_key->description + offset;
+		desc_len -= offset;
 		if (desc_len > n)
 			desc_len = n;
 		do {
-- 
2.53.0

[PATCH 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects()

[Michael Bommarito]

keyring_get_key_chunk() loads description bytes into the index chunk low
address first, while keyring_diff_objects() numbers the first differing
bit from the low end and folds the absolute byte index into the level
without removing the inline-prefix offset the level already carries.
The two disagree on byte order and bit position, so the array can be
told two keys first differ at a bit that does not differ in the chunk
the walker uses, letting crafted descriptions collide into one node.

Load the chunk in the order keyring_diff_objects() assumes and drop the
inline-prefix length when folding the byte index into the level.  This
only changes the in-memory ordering used to place keys within a keyring;
add, search and read of non-colliding keys are unaffected.

Fixes: f771fde82051 ("keys: Simplify key description management")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 security/keys/keyring.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 1739373172ad5..e7066893e6ffc 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -292,9 +292,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 		desc_len -= offset;
 		if (desc_len > n)
 			desc_len = n;
+		d += desc_len;
 		do {
 			chunk <<= 8;
-			chunk |= *d++;
+			chunk |= *--d;
 		} while (--desc_len > 0);
 		return chunk;
 	}
@@ -375,7 +376,7 @@ static int keyring_diff_objects(const void *object, const void *data)
 	return -1;
 
 differ_plus_i:
-	level += i;
+	level += i - (int)sizeof(a->desc);
 differ:
 	i = level * 8 + __ffs(seg_a ^ seg_b);
 	return i;
-- 
2.53.0

[PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned

[Michael Bommarito]

assoc_array_walk() masks off the bits past shortcut->skip_to_level in
the final word of a shortcut before testing it, gated on
round_up(sc_level, chunk_size) > skip_to_level.  Once sc_level is
word-aligned (every word after the first) round_up() is a no-op and the
guard never fires for the word that contains skip_to_level, so its stale
high bits leak into the dissimilarity word and can steer the walk down
the wrong descendant.

Test sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > skip_to_level directly; an
exact-multiple skip_to_level ends on the boundary and stays untrimmed.

Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 lib/assoc_array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/assoc_array.c b/lib/assoc_array.c
index bcc6e0a013eb8..1de2c337f8fcf 100644
--- a/lib/assoc_array.c
+++ b/lib/assoc_array.c
@@ -255,7 +255,7 @@ assoc_array_walk(const struct assoc_array *array,
 		sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
 		dissimilarity = segments ^ sc_segments;
 
-		if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
+		if (sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > shortcut->skip_to_level) {
 			/* Trim segments that are beyond the shortcut */
 			int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
 			dissimilarity &= ~(ULONG_MAX << shift);
-- 
2.53.0

Re: [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency

[Andrew Morton]

On Sat, 11 Jul 2026 21:44:57 -0400 Michael Bommarito <michael.bommarito@gmail.com> wrote:

> keyring_get_key_chunk() advances the description read pointer by
> level * sizeof(long) past the inline prefix but only bounds-checks the
> prefix, so once the associative-array walk reaches a description-level
> chunk it reads past the kmemdup(desc, desc_len + 1) description
> allocation.  Reaching that depth needs two keys that collide through the
> hash, x, type and domain_tag chunks, which an unprivileged add_key(2)
> can arrange with a crafted pair of same-type keys.
> 
> An unprivileged user can thus read up to sizeof(long) bytes past a
> keyring key's description; on kernels built without init-on-alloc the
> same collision, read back with KEYCTL_READ, returns uninitialized kernel
> slab.
> 
> Patch 1 is the memory-safety fix and stands alone.  Patches 2 and 3 fix
> two index-key consistency bugs that let the crafted keys collide into a
> single malformed node in the first place, which is what enables the
> KEYCTL_READ disclosure.
> 
> The KASAN reproduction is on patch 1. Trigger is available off-list.

fyi, AI review might have found things, some pre-existing:
	https://sashiko.dev/#/patchset/20260712014500.480410-1-michael.bommarito@gmail.com

David, you might wish to take a look at the first assoc_array.c issue.

Re: [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency

[Jarkko Sakkinen]

On Sat, Jul 11, 2026 at 09:44:57PM -0400, Michael Bommarito wrote:
> keyring_get_key_chunk() advances the description read pointer by
> level * sizeof(long) past the inline prefix but only bounds-checks the
> prefix, so once the associative-array walk reaches a description-level
> chunk it reads past the kmemdup(desc, desc_len + 1) description
> allocation.  Reaching that depth needs two keys that collide through the
> hash, x, type and domain_tag chunks, which an unprivileged add_key(2)
> can arrange with a crafted pair of same-type keys.

Thanks for the description. I fully get the scenario from this.

> 
> An unprivileged user can thus read up to sizeof(long) bytes past a
> keyring key's description; on kernels built without init-on-alloc the
> same collision, read back with KEYCTL_READ, returns uninitialized kernel
> slab.
> 
> Patch 1 is the memory-safety fix and stands alone.  Patches 2 and 3 fix
> two index-key consistency bugs that let the crafted keys collide into a
> single malformed node in the first place, which is what enables the
> KEYCTL_READ disclosure.
> 
> The KASAN reproduction is on patch 1. Trigger is available off-list.
> 
> Michael Bommarito (3):
>   keys: fix out-of-bounds read in keyring_get_key_chunk()
>   keys: make keyring key-chunk byte order agree with
>     keyring_diff_objects()
>   assoc_array: trim the final shortcut word when skip_to_level is
>     chunk-aligned
> 
>  lib/assoc_array.c       |  2 +-
>  security/keys/keyring.c | 15 ++++++++-------
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> 
> base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309
> --
> 2.53.0
> 

BR, Jarkko

Re: [PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk()

[Jarkko Sakkinen]

On Sat, Jul 11, 2026 at 09:44:58PM -0400, Michael Bommarito wrote:
> For description-level chunks keyring_get_key_chunk() advances the read
> pointer by level * sizeof(long) past the inline prefix but only
> bounds-checks the prefix, so a long enough key description is read past
> its kmemdup(desc, desc_len + 1) allocation.  Compute the full byte
> offset and bounds-check the description against it before reading.
> 
> The walk only reaches a description-level chunk when two keys collide
> through the hash, x, type and domain_tag chunks, so this is reached from
> an unprivileged add_key(2) with a crafted pair of same-type keys whose
> index hashes collide; KASAN reports a slab-out-of-bounds read.
> 
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  security/keys/keyring.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> KASAN, x86_64: add_key(2) of a crafted hash-colliding "user"-key pair
> (~63-byte descriptions) reports
> 
>   BUG: KASAN: slab-out-of-bounds in keyring_get_key_chunk
>   keyring_get_key_chunk <- assoc_array_insert <- __key_link_begin
>     <- __do_sys_add_key
> 
> reading one byte past the description allocation; the same trigger is
> KASAN-clean with this patch.  On a kernel built without init-on-alloc,
> reading the colliding keyring back with KEYCTL_READ returns
> uninitialized slab until patches 2 and 3 are applied too.  Trigger
> available off-list.
> 
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 7a2ee0ded7c93..1739373172ad5 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -270,7 +270,7 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
>  	const struct keyring_index_key *index_key = data;
>  	unsigned long chunk = 0;
>  	const u8 *d;
> -	int desc_len = index_key->desc_len, n = sizeof(chunk);
> +	int desc_len = index_key->desc_len, n = sizeof(chunk), offset;

Just a nut but this is quite nasty looking statement to begin with, so
at least I would not extend it.

I'd add "int offset" to its own line. Maybe for the sake of clarity it
should be also unsigned int unless negative values hold some merit.

>  
>  	level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
>  	switch (level) {
> @@ -284,12 +284,12 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
>  		return (unsigned long)index_key->domain_tag;
>  	default:
>  		level -= 4;
> -		if (desc_len <= sizeof(index_key->desc))
> +		offset = sizeof(index_key->desc) + level * sizeof(long);
> +		if (desc_len <= offset)
>  			return 0;
>  
> -		d = index_key->description + sizeof(index_key->desc);
> -		d += level * sizeof(long);
> -		desc_len -= sizeof(index_key->desc);
> +		d = index_key->description + offset;
> +		desc_len -= offset;
>  		if (desc_len > n)
>  			desc_len = n;
>  		do {
> -- 
> 2.53.0
> 

BR, Jarkko

Re: [PATCH 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects()

[Jarkko Sakkinen]

On Sat, Jul 11, 2026 at 09:44:59PM -0400, Michael Bommarito wrote:
> keyring_get_key_chunk() loads description bytes into the index chunk low
> address first, while keyring_diff_objects() numbers the first differing
> bit from the low end and folds the absolute byte index into the level
> without removing the inline-prefix offset the level already carries.
> The two disagree on byte order and bit position, so the array can be
> told two keys first differ at a bit that does not differ in the chunk
> the walker uses, letting crafted descriptions collide into one node.
> 
> Load the chunk in the order keyring_diff_objects() assumes and drop the
> inline-prefix length when folding the byte index into the level.  This
> only changes the in-memory ordering used to place keys within a keyring;
> add, search and read of non-colliding keys are unaffected.
> 
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  security/keys/keyring.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 1739373172ad5..e7066893e6ffc 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -292,9 +292,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
>  		desc_len -= offset;
>  		if (desc_len > n)
>  			desc_len = n;
> +		d += desc_len;
>  		do {
>  			chunk <<= 8;
> -			chunk |= *d++;
> +			chunk |= *--d;
>  		} while (--desc_len > 0);
>  		return chunk;
>  	}
> @@ -375,7 +376,7 @@ static int keyring_diff_objects(const void *object, const void *data)
>  	return -1;
>  
>  differ_plus_i:
> -	level += i;
> +	level += i - (int)sizeof(a->desc);
>  differ:
>  	i = level * 8 + __ffs(seg_a ^ seg_b);
>  	return i;
> -- 
> 2.53.0
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Add to +1 version.

BR, Jarkko

Re: [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned

[Jarkko Sakkinen]

On Sat, Jul 11, 2026 at 09:45:00PM -0400, Michael Bommarito wrote:
> assoc_array_walk() masks off the bits past shortcut->skip_to_level in
> the final word of a shortcut before testing it, gated on
> round_up(sc_level, chunk_size) > skip_to_level.  Once sc_level is
> word-aligned (every word after the first) round_up() is a no-op and the
> guard never fires for the word that contains skip_to_level, so its stale
> high bits leak into the dissimilarity word and can steer the walk down
> the wrong descendant.
> 
> Test sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > skip_to_level directly; an
> exact-multiple skip_to_level ends on the boundary and stays untrimmed.
> 
> Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  lib/assoc_array.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/assoc_array.c b/lib/assoc_array.c
> index bcc6e0a013eb8..1de2c337f8fcf 100644
> --- a/lib/assoc_array.c
> +++ b/lib/assoc_array.c
> @@ -255,7 +255,7 @@ assoc_array_walk(const struct assoc_array *array,
>  		sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
>  		dissimilarity = segments ^ sc_segments;
>  
> -		if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
> +		if (sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > shortcut->skip_to_level) {
>  			/* Trim segments that are beyond the shortcut */
>  			int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
>  			dissimilarity &= ~(ULONG_MAX << shift);
> -- 
> 2.53.0
> 

Ditto.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned

[Jarkko Sakkinen]

On Sat, Jul 18, 2026 at 09:32:46PM +0300, Jarkko Sakkinen wrote:
> On Sat, Jul 11, 2026 at 09:45:00PM -0400, Michael Bommarito wrote:
> > assoc_array_walk() masks off the bits past shortcut->skip_to_level in
> > the final word of a shortcut before testing it, gated on
> > round_up(sc_level, chunk_size) > skip_to_level.  Once sc_level is
> > word-aligned (every word after the first) round_up() is a no-op and the
> > guard never fires for the word that contains skip_to_level, so its stale
> > high bits leak into the dissimilarity word and can steer the walk down
> > the wrong descendant.
> > 
> > Test sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > skip_to_level directly; an
> > exact-multiple skip_to_level ends on the boundary and stays untrimmed.
> > 
> > Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> > ---
> >  lib/assoc_array.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/assoc_array.c b/lib/assoc_array.c
> > index bcc6e0a013eb8..1de2c337f8fcf 100644
> > --- a/lib/assoc_array.c
> > +++ b/lib/assoc_array.c
> > @@ -255,7 +255,7 @@ assoc_array_walk(const struct assoc_array *array,
> >  		sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
> >  		dissimilarity = segments ^ sc_segments;
> >  
> > -		if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
> > +		if (sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > shortcut->skip_to_level) {
> >  			/* Trim segments that are beyond the shortcut */
> >  			int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
> >  			dissimilarity &= ~(ULONG_MAX << shift);
> > -- 
> > 2.53.0
> > 
> 
> Ditto.
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> BR, Jarkko

On holiday up until end of this month so expect some delay in my
responses.

BR, Jarkko