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