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