| Message ID | 20241029113401.90539-1-david@sigma-star.at (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation | expand |
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
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
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);
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(-)