Re: [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Tue Oct 29, 2024 at 1:34 PM EET, David Gstir wrote:
> When sealing or unsealing a key blob we currently do not wait for
> the AEAD cipher operation to finish and simply return after submitting
> the request. If there is some load on the system we can exit before
> the cipher operation is done and the buffer we read from/write to
> is already removed from the stack. This will e.g. result in NULL
> pointer dereference errors in the DCP driver during blob creation.
>
> Fix this by waiting for the AEAD cipher operation to finish before
> resuming the seal and unseal calls.
>
> Cc: stable@vger.kernel.org # v6.10+
> Fixes: 0e28bf61a5f9 ("KEYS: trusted: dcp: fix leak of blob encryption key")
> Reported-by: Parthiban N <parthiban@linumiz.com>
> Closes: https://lore.kernel.org/keyrings/254d3bb1-6dbc-48b4-9c08-77df04baee2f@linumiz.com/
> Signed-off-by: David Gstir <david@sigma-star.at>
> ---
>  security/keys/trusted-keys/trusted_dcp.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
> index 4edc5bbbcda3..e908c53a803c 100644
> --- a/security/keys/trusted-keys/trusted_dcp.c
> +++ b/security/keys/trusted-keys/trusted_dcp.c
> @@ -133,6 +133,7 @@ static int do_aead_crypto(u8 *in, u8 *out, size_t len, u8 *key, u8 *nonce,
>  	struct scatterlist src_sg, dst_sg;
>  	struct crypto_aead *aead;
>  	int ret;
> +	DECLARE_CRYPTO_WAIT(wait);
>  
>  	aead = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
>  	if (IS_ERR(aead)) {
> @@ -163,8 +164,8 @@ static int do_aead_crypto(u8 *in, u8 *out, size_t len, u8 *key, u8 *nonce,
>  	}
>  
>  	aead_request_set_crypt(aead_req, &src_sg, &dst_sg, len, nonce);
> -	aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL,
> -				  NULL);
> +	aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_SLEEP,
> +				  crypto_req_done, &wait);
>  	aead_request_set_ad(aead_req, 0);
>  
>  	if (crypto_aead_setkey(aead, key, AES_KEYSIZE_128)) {
> @@ -174,9 +175,9 @@ static int do_aead_crypto(u8 *in, u8 *out, size_t len, u8 *key, u8 *nonce,
>  	}
>  
>  	if (do_encrypt)
> -		ret = crypto_aead_encrypt(aead_req);
> +		ret = crypto_wait_req(crypto_aead_encrypt(aead_req), &wait);
>  	else
> -		ret = crypto_aead_decrypt(aead_req);
> +		ret = crypto_wait_req(crypto_aead_decrypt(aead_req), &wait);
>  
>  free_req:
>  	aead_request_free(aead_req);

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

BR, Jarkko