[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