[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

[PATCH v2] keys: big_key: Remove the redundant quota reservation

[Daehyeon Ko]

A big_key is charged a fixed 16 bytes of key quota, regardless of its
payload size.  big_key_update() nevertheless reserves the new raw payload
length before destroying the old payload, and generic_key_instantiate()
then reserves the fixed quota length.

For an update 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() reserves it again.  If the second
reservation fails, the key remains positive after its old payload has 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

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

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

The first reservation is unnecessary.  A positive big_key already has a
16-byte reservation from successful instantiation, so the reservation in
generic_key_instantiate() has zero delta and cannot fail after the old
payload is destroyed.  A negative key has no old payload to destroy, and
generic_key_instantiate() performs the required reservation before
publishing the new payload.

Remove the redundant reservation so that the update path adjusts quota only
once and cannot expose a temporary quota gap.

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>
---
Changes since v1:
- Remove the redundant first reservation instead of changing its length,
  leaving generic_key_instantiate() as the single quota reservation on the
  update path, as suggested by Jarkko.
- Verify full-quota positive and negative updates separately.  A positive
  update keeps a zero quota delta; a negative update can still return
  -EDQUOT before publishing a payload and succeeds after quota is released.

 security/keys/big_key.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 268f702df380e..ef707f2a81a98 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -193,12 +193,6 @@ void big_key_destroy(struct key *key)
  */
 int big_key_update(struct key *key, struct key_preparsed_payload *prep)
 {
-	int ret;
-
-	ret = key_payload_reserve(key, prep->datalen);
-	if (ret < 0)
-		return ret;
-
 	if (key_is_positive(key))
 		big_key_destroy(key);
 

base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da
-- 
2.55.0

Re: [PATCH v2] keys: big_key: Remove the redundant quota reservation

[sashiko-bot]

> A big_key is charged a fixed 16 bytes of key quota, regardless of its
> payload size.  big_key_update() nevertheless reserves the new raw payload
> length before destroying the old payload, and generic_key_instantiate()
> then reserves the fixed quota length.
> 
> For an update 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() reserves it again.  If the second
> reservation fails, the key remains positive after its old payload has 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
> [ ... ]
> 
> 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.

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