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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <D58NF7GGPID7.1AIU8KVZVY4WC@kernel.org>
Date: Wed, 30 Oct 2024 00:38:19 +0200
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "David Gstir" <david@...ma-star.at>, <parthiban@...umiz.com>, "James
 Bottomley" <James.Bottomley@...senPartnership.com>, "Mimi Zohar"
 <zohar@...ux.ibm.com>, "David Howells" <dhowells@...hat.com>, "Paul Moore"
 <paul@...l-moore.com>, "James Morris" <jmorris@...ei.org>, "Serge E.
 Hallyn" <serge@...lyn.com>
Cc: "sigma star Kernel Team" <upstream+dcp@...ma-star.at>,
 <linux-integrity@...r.kernel.org>, <keyrings@...r.kernel.org>,
 <linux-security-module@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <stable@...r.kernel.org>
Subject: Re: [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto
 operation

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@...r.kernel.org # v6.10+
> Fixes: 0e28bf61a5f9 ("KEYS: trusted: dcp: fix leak of blob encryption key")
> Reported-by: Parthiban N <parthiban@...umiz.com>
> Closes: https://lore.kernel.org/keyrings/254d3bb1-6dbc-48b4-9c08-77df04baee2f@linumiz.com/
> Signed-off-by: David Gstir <david@...ma-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@...nel.org>

BR, Jarkko