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

[Jarkko Sakkinen <jarkko@kernel.org>]

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