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: [email protected] # v6.10+ > Fixes: 0e28bf61a5f9 ("KEYS: trusted: dcp: fix leak of blob encryption key") > Reported-by: Parthiban N <[email protected]> > Closes: > https://lore.kernel.org/keyrings/[email protected]/ > Signed-off-by: David Gstir <[email protected]> > --- > 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 <[email protected]> BR, Jarkko
