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

[Jarkko Sakkinen <jarkko@kernel.org>]

On Sun, Oct 18, 2026 at 12:21:23AM +0900, Daehyeon Ko wrote:
> 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
> 

Well, that made a difference. I've started actively try to look
for eager tactics, and in this case it turned out to be worth
it. The first solution was exactly like weighted answer that
cut the search too early :-) 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Thank you.

PS. Not applying til Monday.

Br, Jarkko