[PATCH] keys: big_key: Reserve the final quota before destroying the payload

[PATCH] keys: big_key: Reserve the final quota before destroying the payload

[Daehyeon Ko]

A big_key is charged a fixed 16 bytes of key quota, regardless of the
actual payload size.  big_key_update() currently reserves the actual new
payload length before destroying the old payload, while the subsequent
generic_key_instantiate() reserves the fixed quota length.

For an update to a payload shorter than 16 bytes, the first reservation
temporarily returns quota.  Another key operation by the same user can
consume that quota before generic_key_instantiate() tries to reserve it
again.  If that second reservation fails, the key remains instantiated but
its old big_key payload has already been destroyed.  Reading a
buffer-backed key or revoking a shmem-backed key then dereferences a NULL
payload pointer.

On a KASAN build, reading a buffer-backed key reports:

  BUG: KASAN: null-ptr-deref in big_key_read+0x110/0x310
  Read of size 64 at addr 0000000000000000
  ...
  big_key_read+0x110/0x310
  keyctl_read_key+0x2da/0x550

Revoking a shmem-backed key reports, with oops=panic:

  Oops: general protection fault
  KASAN: null-ptr-deref in range [0x68-0x6f]
  RIP: vfs_truncate+0xa2/0x730
  big_key_revoke+0xbd/0xe0
  key_revoke+0xc0/0x1a0
  Kernel panic - not syncing: Fatal exception

Reserve the final fixed quota length before destroying the old payload.
The reservation in generic_key_instantiate() then has no quota delta and
cannot fail for this reason after the destructive step.

Fixes: b6f61c314649 ("keys: Implement update for the big_key type")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
 security/keys/big_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 268f702df380e..6aff1a73c7b38 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -195,7 +195,7 @@ int big_key_update(struct key *key, struct key_preparsed_payload *prep)
 {
 	int ret;
 
-	ret = key_payload_reserve(key, prep->datalen);
+	ret = key_payload_reserve(key, prep->quotalen);
 	if (ret < 0)
 		return ret;
 
-- 
2.55.0

Re: [PATCH] keys: big_key: Reserve the final quota before destroying the payload

[sashiko-bot]

> A big_key is charged a fixed 16 bytes of key quota, regardless of the
> actual payload size.  big_key_update() currently reserves the actual new
> payload length before destroying the old payload, while the subsequent
> generic_key_instantiate() reserves the fixed quota length.
> 
> For an update to a payload shorter than 16 bytes, the first reservation
> temporarily returns quota.  Another key operation by the same user can
> consume that quota before generic_key_instantiate() tries to reserve it
> again.  If that second reservation fails, the key remains instantiated but
> its old big_key payload has already been destroyed.  Reading a
> buffer-backed key or revoking a shmem-backed key then dereferences a NULL
> payload pointer.
> 
> On a KASAN build, reading a buffer-backed key reports:
> 
> [ ... ]
> 
> Fixes: b6f61c314649 ("keys: Implement update for the big_key type")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016040004.324286-1-4ncienth@gmail.com?part=1

Re: [PATCH] keys: big_key: Reserve the final quota before destroying the payload

[Jarkko Sakkinen]

On Fri, Oct 16, 2026 at 01:00:04PM +0900, Daehyeon Ko wrote:
> A big_key is charged a fixed 16 bytes of key quota, regardless of the
> actual payload size.  big_key_update() currently reserves the actual new
> payload length before destroying the old payload, while the subsequent
> generic_key_instantiate() reserves the fixed quota length.
> 
> For an update to a payload shorter than 16 bytes, the first reservation
> temporarily returns quota.  Another key operation by the same user can
> consume that quota before generic_key_instantiate() tries to reserve it
> again.  If that second reservation fails, the key remains instantiated but
> its old big_key payload has already been destroyed.  Reading a
> buffer-backed key or revoking a shmem-backed key then dereferences a NULL
> payload pointer.

Even at the risk that I'm saying something stupid, makes me
wonder why the call exists in big_key.c? I.e. why not remove
the call instead of doing what the patch does?

If I'm now missing something out, that's cool, just correct
me.

> 
> On a KASAN build, reading a buffer-backed key reports:
> 
>   BUG: KASAN: null-ptr-deref in big_key_read+0x110/0x310
>   Read of size 64 at addr 0000000000000000
>   ...
>   big_key_read+0x110/0x310
>   keyctl_read_key+0x2da/0x550
> 
> Revoking a shmem-backed key reports, with oops=panic:
> 
>   Oops: general protection fault
>   KASAN: null-ptr-deref in range [0x68-0x6f]
>   RIP: vfs_truncate+0xa2/0x730
>   big_key_revoke+0xbd/0xe0
>   key_revoke+0xc0/0x1a0
>   Kernel panic - not syncing: Fatal exception
> 
> Reserve the final fixed quota length before destroying the old payload.
> The reservation in generic_key_instantiate() then has no quota delta and
> cannot fail for this reason after the destructive step.
> 
> Fixes: b6f61c314649 ("keys: Implement update for the big_key type")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
>  security/keys/big_key.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/keys/big_key.c b/security/keys/big_key.c
> index 268f702df380e..6aff1a73c7b38 100644
> --- a/security/keys/big_key.c
> +++ b/security/keys/big_key.c
> @@ -195,7 +195,7 @@ int big_key_update(struct key *key, struct key_preparsed_payload *prep)
>  {
>  	int ret;
>  
> -	ret = key_payload_reserve(key, prep->datalen);
> +	ret = key_payload_reserve(key, prep->quotalen);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.55.0
> 

Br, Jarkko

Re: [PATCH] keys: big_key: Reserve the final quota before destroying the payload

[Jarkko Sakkinen]

On Sat, Oct 17, 2026 at 02:33:10AM +0300, Jarkko Sakkinen wrote:
> On Fri, Oct 16, 2026 at 01:00:04PM +0900, Daehyeon Ko wrote:
> > A big_key is charged a fixed 16 bytes of key quota, regardless of the
> > actual payload size.  big_key_update() currently reserves the actual new
> > payload length before destroying the old payload, while the subsequent
> > generic_key_instantiate() reserves the fixed quota length.
> > 
> > For an update to a payload shorter than 16 bytes, the first reservation
> > temporarily returns quota.  Another key operation by the same user can
> > consume that quota before generic_key_instantiate() tries to reserve it
> > again.  If that second reservation fails, the key remains instantiated but
> > its old big_key payload has already been destroyed.  Reading a
> > buffer-backed key or revoking a shmem-backed key then dereferences a NULL
> > payload pointer.
> 
> Even at the risk that I'm saying something stupid, makes me
> wonder why the call exists in big_key.c? I.e. why not remove
> the call instead of doing what the patch does?
> 
> If I'm now missing something out, that's cool, just correct
> me.

So the way I see it this patch has the right bug and a wrong fix.

I.e. if you still want to move forward with this fix, explain
why two repeated calls are still a necessity. Then we can ensure
that the fix addressing the root cause.

Alternatively rewrite the patch so that it makes the call path have
only a single call site for key_payload_reserve().

Br. Jarkko