Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency
@ 2026-07-12  1:44 Michael Bommarito
  2026-07-12  1:44 ` [PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk() Michael Bommarito
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Michael Bommarito @ 2026-07-12  1:44 UTC (permalink / raw)
  To: David Howells, Jarkko Sakkinen
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Andrew Morton,
	keyrings, linux-security-module, linux-kernel

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk()
  2026-07-12  1:44 [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Michael Bommarito
@ 2026-07-12  1:44 ` Michael Bommarito
  2026-07-12  1:44 ` [PATCH 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects() Michael Bommarito
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Bommarito @ 2026-07-12  1:44 UTC (permalink / raw)
  To: David Howells, Jarkko Sakkinen
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Andrew Morton,
	keyrings, linux-security-module, linux-kernel

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects()
  2026-07-12  1:44 [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Michael Bommarito
  2026-07-12  1:44 ` [PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk() Michael Bommarito
@ 2026-07-12  1:44 ` Michael Bommarito
  2026-07-12  1:45 ` [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned Michael Bommarito
  2026-07-14  2:11 ` [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Andrew Morton
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Bommarito @ 2026-07-12  1:44 UTC (permalink / raw)
  To: David Howells, Jarkko Sakkinen
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Andrew Morton,
	keyrings, linux-security-module, linux-kernel

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned
  2026-07-12  1:44 [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Michael Bommarito
  2026-07-12  1:44 ` [PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk() Michael Bommarito
  2026-07-12  1:44 ` [PATCH 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects() Michael Bommarito
@ 2026-07-12  1:45 ` Michael Bommarito
  2026-07-14  2:11 ` [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Andrew Morton
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Bommarito @ 2026-07-12  1:45 UTC (permalink / raw)
  To: David Howells, Jarkko Sakkinen
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Andrew Morton,
	keyrings, linux-security-module, linux-kernel

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency
  2026-07-12  1:44 [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Michael Bommarito
                   ` (2 preceding siblings ...)
  2026-07-12  1:45 ` [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned Michael Bommarito
@ 2026-07-14  2:11 ` Andrew Morton
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2026-07-14  2:11 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
	Serge E . Hallyn, keyrings, linux-security-module, linux-kernel

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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-14  2:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-12  1:44 [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Michael Bommarito
2026-07-12  1:44 ` [PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk() Michael Bommarito
2026-07-12  1:44 ` [PATCH 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects() Michael Bommarito
2026-07-12  1:45 ` [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned Michael Bommarito
2026-07-14  2:11 ` [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox