All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] keys: reject descriptions that exceed the index length
@ 2026-08-24 11:30 Daehyeon Ko
  2026-08-28  5:02 ` Jarkko Sakkinen
  0 siblings, 1 reply; 5+ messages in thread
From: Daehyeon Ko @ 2026-08-24 11:30 UTC (permalink / raw)
  To: dhowells, jarkko, lukas, ignat
  Cc: paul, jmorris, serge, herbert, davem, keyrings,
	linux-security-module, linux-crypto, linux-kernel, Daehyeon Ko

struct keyring_index_key::desc_len is a u16.  User-provided key
descriptions are limited to 4095 bytes, but a key type preparser can
generate a longer description when the caller passes NULL.

The X.509 parser forms a description from the certificate subject and
twice the raw serial length.  A certificate with a two-byte subject and a
32766-byte serial therefore produces a 65536-byte description.  Assigning
strlen() to desc_len wraps it to zero, after which __key_link_begin() hits:

    BUG_ON(index_key->desc_len == 0);

This is reachable through add_key() by an unprivileged user and can panic
the kernel when oopses are fatal.

Measure generated descriptions before narrowing the length and reject
values that cannot be represented.  The boundary input now returns EINVAL,
while the one-byte-short control still reaches the normal quota check.

Fixes: f771fde82051 ("keys: Simplify key description management")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
 security/keys/key.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index b34a64d81d47ab..f2f472b45f4eee 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -14,6 +14,7 @@
 #include <linux/workqueue.h>
 #include <linux/random.h>
 #include <linux/err.h>
+#include <linux/limits.h>
 #include "internal.h"
 
 struct kmem_cache *key_jar;
@@ -820,6 +821,7 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
 	const struct cred *cred = current_cred();
 	struct key *keyring, *key = NULL;
 	key_ref_t key_ref;
+	size_t desc_len;
 	int ret;
 	struct key_restriction *restrict_link = NULL;
 
@@ -865,7 +867,12 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
 		if (!index_key.description)
 			goto error_free_prep;
 	}
-	index_key.desc_len = strlen(index_key.description);
+	desc_len = strlen(index_key.description);
+	if (desc_len > U16_MAX) {
+		key_ref = ERR_PTR(-EINVAL);
+		goto error_free_prep;
+	}
+	index_key.desc_len = desc_len;
 	key_set_index_key(&index_key);
 
 	ret = __key_link_lock(keyring, &index_key);

base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b

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

* Re: [PATCH] keys: reject descriptions that exceed the index length
  2026-08-24 11:30 [PATCH] keys: reject descriptions that exceed the index length Daehyeon Ko
@ 2026-08-28  5:02 ` Jarkko Sakkinen
  2026-08-28  5:38   ` Daehyeon Ko
  0 siblings, 1 reply; 5+ messages in thread
From: Jarkko Sakkinen @ 2026-08-28  5:02 UTC (permalink / raw)
  To: Daehyeon Ko
  Cc: dhowells, lukas, ignat, paul, jmorris, serge, herbert, davem,
	keyrings, linux-security-module, linux-crypto, linux-kernel

On Mon, Aug 24, 2026 at 08:30:04PM +0900, Daehyeon Ko wrote:
> struct keyring_index_key::desc_len is a u16.  User-provided key
> descriptions are limited to 4095 bytes, but a key type preparser can
> generate a longer description when the caller passes NULL.
> 
> The X.509 parser forms a description from the certificate subject and
> twice the raw serial length.  A certificate with a two-byte subject and a
> 32766-byte serial therefore produces a 65536-byte description.  Assigning
> strlen() to desc_len wraps it to zero, after which __key_link_begin() hits:
> 
>     BUG_ON(index_key->desc_len == 0);
> 
> This is reachable through add_key() by an unprivileged user and can panic
> the kernel when oopses are fatal.
> 
> Measure generated descriptions before narrowing the length and reject
> values that cannot be represented.  The boundary input now returns EINVAL,
> while the one-byte-short control still reaches the normal quota check.

This lacks smoking gun type of evidence, and I don't understand why
as it requires an extremely low effort.

If I understood the code correctly, I'd start with a key that I would
craft along the lines of:

    openssl genpkey -algorithm Ed25519 -out key.bin

    openssl req -x509 -key key.bin \
                      -outform DER \
                      -out certificate.bin \
                      -subj "/CN=xx" \
                      -set_serial "0x7f$(head -c 65530 /dev/zero | tr '\0' 'f')"

Then I would simply expect this to crash my system:

    keyctl padd asymmetric %:s @s < certificate.bin

Will this happen? Have not tried it in a VM yet. I just feel a bit
confused that this was not already demontrated in the commit message,
which makes me think that I get something wrong?

BR, Jarkko

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

* Re: [PATCH] keys: reject descriptions that exceed the index length
  2026-08-28  5:02 ` Jarkko Sakkinen
@ 2026-08-28  5:38   ` Daehyeon Ko
  2026-08-28  8:07     ` [PATCH v2] " Daehyeon Ko
  0 siblings, 1 reply; 5+ messages in thread
From: Daehyeon Ko @ 2026-08-28  5:38 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: dhowells, lukas, ignat, paul, jmorris, serge, herbert, davem,
	keyrings, linux-security-module, linux-crypto, linux-kernel

Hi Jarkko,

Thanks.  You are right that the commit message should have included the
observed runtime evidence.  I had tested the bug, but omitting that evidence
made the report unnecessarily hard to assess.  Sorry about that.

No, the exact command above does not crash in my test.  I ran it as uid 1000
with no capabilities on the vulnerable v6.12.105 kernel, and it returned
ENOPKG.  There are three relevant details:

  1. The in-tree X.509 parser does not support an Ed25519 public-key OID.

  2. keyctl passes "%:s" as a literal non-empty description.  It does not
     request a generated description in that argument position.  An empty
     string is needed; add_key() normalizes that to NULL.

  3. On my system, the default OpenSSL configuration adds a Subject Key
     Identifier.  x509_key_preparse() prefers the SKID over the raw serial, so
     that also keeps the generated description short.

I repeated the test with a supported RSA certificate, no SKID, the same
32766-byte positive serial and two-byte subject, and an empty keyctl
description.  The keyctl process recorded uid 1000 and zero inheritable,
permitted and effective capabilities, then hit:

  kernel BUG at security/keys/keyring.c:1308
  __key_link_begin
  __key_create_or_update
  key_create_or_update
  __do_sys_add_key
  Kernel panic - not syncing: Fatal exception

For comparison, the same RSA/no-SKID certificate with "%:s" as the explicit
description created the key normally and produced no splat.  An RSA
certificate generated with the default SKID also created the key normally,
even with an empty description.

The original source reproducer, which constructs the DER without a SKID,
already produced the registered BUG and panic on 3/3 fresh v6.12.105 KASAN
boots as uid 1000.  The 32765-byte-serial control returned EDQUOT without a
splat.  With the patch, the boundary returned EINVAL and the control continued
to return EDQUOT on 3/3 fresh boots.

I will send a v2 with this observed trace and the before/after results in the
commit message.  The code change is unchanged.  I can also provide the source
reproducer privately if useful.

Thanks,
Daehyeon

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

* [PATCH v2] keys: reject descriptions that exceed the index length
  2026-08-28  5:38   ` Daehyeon Ko
@ 2026-08-28  8:07     ` Daehyeon Ko
  2026-08-28 10:12       ` Sudhakar Kuppusamy
  0 siblings, 1 reply; 5+ messages in thread
From: Daehyeon Ko @ 2026-08-28  8:07 UTC (permalink / raw)
  To: dhowells, jarkko, lukas, ignat
  Cc: paul, jmorris, serge, herbert, davem, keyrings,
	linux-security-module, linux-crypto, linux-kernel, 4ncienth

struct keyring_index_key::desc_len is a u16.  User-provided key
descriptions are limited to 4095 bytes, but a key type preparser can
generate a longer description when the caller passes NULL.

The X.509 parser forms a description from the certificate subject and
twice the raw serial length when the certificate has no Subject Key
Identifier.  A certificate with a two-byte subject and a 32766-byte serial
therefore produces a 65536-byte description.  Assigning strlen() to
desc_len wraps it to zero, after which __key_link_begin() hits:

    BUG_ON(index_key->desc_len == 0);

This was reproduced through keyctl on a v6.12.105 KASAN kernel with
panic=1 and oops=panic.  The process ran as uid and gid 1000 with no
capabilities, and the console recorded:

    CapInh: 0000000000000000
    CapPrm: 0000000000000000
    CapEff: 0000000000000000
    [   14.044055] ------------[ cut here ]------------
    [   14.044211] kernel BUG at security/keys/keyring.c:1308!
    [   14.044375] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
    [   14.044558] CPU: 0 UID: 1000 PID: 138 Comm: keyctl Not tainted 6.12.105-dirty #1
    [   14.044788] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
    [   14.045025] RIP: 0010:__key_link_begin+0x1e5/0x250
    [   14.047630] Call Trace:
    [   14.047711]  <TASK>
    [   14.047785]  __key_create_or_update+0x490/0xcd0
    [   14.047954]  ? __pfx___key_create_or_update+0x10/0x10
    [   14.048139]  ? __pfx_lookup_user_key_possessed+0x10/0x10
    [   14.048323]  key_create_or_update+0x47/0x60
    [   14.048484]  __do_sys_add_key+0x219/0x430
    [   14.048632]  ? __pfx___do_sys_add_key+0x10/0x10
    [   14.048794]  ? switch_fpu_return+0x127/0x250
    [   14.048980]  ? srso_return_thunk+0x5/0x5f
    [   14.049163]  ? arch_exit_to_user_mode_prepare.constprop.0+0x6f/0xa0
    [   14.049417]  ? srso_return_thunk+0x5/0x5f
    [   14.049582]  do_syscall_64+0x5a/0x130
    [   14.049721]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
    [   14.051772]  </TASK>
    [   14.051975] ---[ end trace 0000000000000000 ]---
    [   14.054729] Kernel panic - not syncing: Fatal exception

The same boundary was independently reproduced on 3/3 fresh v6.12.105
KASAN boots with the source reproducer.

A one-byte-short control with a 32765-byte serial parsed successfully and
returned EDQUOT without a splat, showing that the wrap boundary is causal.

Measure generated descriptions before narrowing the length and reject
values that cannot be represented.  On 3/3 fresh patched mainline KASAN
boots, the boundary returned EINVAL and the control continued to return
EDQUOT, with no BUG, Oops, KASAN report, warning or panic.

A tested source reproducer exists and is available to maintainers
privately.

Fixes: f771fde82051 ("keys: Simplify key description management")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
Changes in v2:
- Add the observed keyctl crash trace and before/after runtime results to the
  commit message.
- Clarify that the serial is used when the certificate has no Subject Key
  Identifier.
- No code changes.

 security/keys/key.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index b34a64d81d47ab..f2f472b45f4eee 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -14,6 +14,7 @@
 #include <linux/workqueue.h>
 #include <linux/random.h>
 #include <linux/err.h>
+#include <linux/limits.h>
 #include "internal.h"
 
 struct kmem_cache *key_jar;
@@ -820,6 +821,7 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
 	const struct cred *cred = current_cred();
 	struct key *keyring, *key = NULL;
 	key_ref_t key_ref;
+	size_t desc_len;
 	int ret;
 	struct key_restriction *restrict_link = NULL;
 
@@ -865,7 +867,12 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
 		if (!index_key.description)
 			goto error_free_prep;
 	}
-	index_key.desc_len = strlen(index_key.description);
+	desc_len = strlen(index_key.description);
+	if (desc_len > U16_MAX) {
+		key_ref = ERR_PTR(-EINVAL);
+		goto error_free_prep;
+	}
+	index_key.desc_len = desc_len;
 	key_set_index_key(&index_key);
 
 	ret = __key_link_lock(keyring, &index_key);

base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
-- 
2.54.0

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

* Re: [PATCH v2] keys: reject descriptions that exceed the index length
  2026-08-28  8:07     ` [PATCH v2] " Daehyeon Ko
@ 2026-08-28 10:12       ` Sudhakar Kuppusamy
  0 siblings, 0 replies; 5+ messages in thread
From: Sudhakar Kuppusamy @ 2026-08-28 10:12 UTC (permalink / raw)
  To: Daehyeon Ko
  Cc: dhowells, jarkko, lukas, ignat, paul, jmorris, serge, herbert,
	davem, keyrings, linux-security-module, linux-crypto,
	linux-kernel



> On 28 Aug 2026, at 1:37 PM, Daehyeon Ko <4ncienth@gmail.com> wrote:
> 
> struct keyring_index_key::desc_len is a u16.  User-provided key
> descriptions are limited to 4095 bytes, but a key type preparser can
> generate a longer description when the caller passes NULL.
> 
> The X.509 parser forms a description from the certificate subject and
> twice the raw serial length when the certificate has no Subject Key
> Identifier.  A certificate with a two-byte subject and a 32766-byte serial
> therefore produces a 65536-byte description.  Assigning strlen() to
> desc_len wraps it to zero, after which __key_link_begin() hits:
> 
>    BUG_ON(index_key->desc_len == 0);
> 
> This was reproduced through keyctl on a v6.12.105 KASAN kernel with
> panic=1 and oops=panic.  The process ran as uid and gid 1000 with no
> capabilities, and the console recorded:
> 
>    CapInh: 0000000000000000
>    CapPrm: 0000000000000000
>    CapEff: 0000000000000000
>    [   14.044055] ------------[ cut here ]------------
>    [   14.044211] kernel BUG at security/keys/keyring.c:1308!
>    [   14.044375] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>    [   14.044558] CPU: 0 UID: 1000 PID: 138 Comm: keyctl Not tainted 6.12.105-dirty #1
>    [   14.044788] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
>    [   14.045025] RIP: 0010:__key_link_begin+0x1e5/0x250
>    [   14.047630] Call Trace:
>    [   14.047711]  <TASK>
>    [   14.047785]  __key_create_or_update+0x490/0xcd0
>    [   14.047954]  ? __pfx___key_create_or_update+0x10/0x10
>    [   14.048139]  ? __pfx_lookup_user_key_possessed+0x10/0x10
>    [   14.048323]  key_create_or_update+0x47/0x60
>    [   14.048484]  __do_sys_add_key+0x219/0x430
>    [   14.048632]  ? __pfx___do_sys_add_key+0x10/0x10
>    [   14.048794]  ? switch_fpu_return+0x127/0x250
>    [   14.048980]  ? srso_return_thunk+0x5/0x5f
>    [   14.049163]  ? arch_exit_to_user_mode_prepare.constprop.0+0x6f/0xa0
>    [   14.049417]  ? srso_return_thunk+0x5/0x5f
>    [   14.049582]  do_syscall_64+0x5a/0x130
>    [   14.049721]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>    [   14.051772]  </TASK>
>    [   14.051975] ---[ end trace 0000000000000000 ]---
>    [   14.054729] Kernel panic - not syncing: Fatal exception
> 
> The same boundary was independently reproduced on 3/3 fresh v6.12.105
> KASAN boots with the source reproducer.
> 
> A one-byte-short control with a 32765-byte serial parsed successfully and
> returned EDQUOT without a splat, showing that the wrap boundary is causal.
> 
> Measure generated descriptions before narrowing the length and reject
> values that cannot be represented.  On 3/3 fresh patched mainline KASAN
> boots, the boundary returned EINVAL and the control continued to return
> EDQUOT, with no BUG, Oops, KASAN report, warning or panic.
> 
> A tested source reproducer exists and is available to maintainers
> privately.
> 
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>


Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>


Thanks,
Sudhakar

> ---
> Changes in v2:
> - Add the observed keyctl crash trace and before/after runtime results to the
>  commit message.
> - Clarify that the serial is used when the certificate has no Subject Key
>  Identifier.
> - No code changes.
> 
> security/keys/key.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/security/keys/key.c b/security/keys/key.c
> index b34a64d81d47ab..f2f472b45f4eee 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -14,6 +14,7 @@
> #include <linux/workqueue.h>
> #include <linux/random.h>
> #include <linux/err.h>
> +#include <linux/limits.h>
> #include "internal.h"
> 
> struct kmem_cache *key_jar;
> @@ -820,6 +821,7 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
> const struct cred *cred = current_cred();
> struct key *keyring, *key = NULL;
> key_ref_t key_ref;
> + size_t desc_len;
> int ret;
> struct key_restriction *restrict_link = NULL;
> 
> @@ -865,7 +867,12 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
> if (!index_key.description)
> goto error_free_prep;
> }
> - index_key.desc_len = strlen(index_key.description);
> + desc_len = strlen(index_key.description);
> + if (desc_len > U16_MAX) {
> + key_ref = ERR_PTR(-EINVAL);
> + goto error_free_prep;
> + }
> + index_key.desc_len = desc_len;
> key_set_index_key(&index_key);
> 
> ret = __key_link_lock(keyring, &index_key);
> 
> base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
> -- 
> 2.54.0
> 


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

end of thread, other threads:[~2026-08-28 10:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 11:30 [PATCH] keys: reject descriptions that exceed the index length Daehyeon Ko
2026-08-28  5:02 ` Jarkko Sakkinen
2026-08-28  5:38   ` Daehyeon Ko
2026-08-28  8:07     ` [PATCH v2] " Daehyeon Ko
2026-08-28 10:12       ` Sudhakar Kuppusamy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.