* Trusted keys: DCP: Unable to handle paging request
@ 2024-09-10 15:02 Parthiban
2024-09-11 11:46 ` David Gstir
2024-10-29 11:34 ` [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation David Gstir
0 siblings, 2 replies; 5+ messages in thread
From: Parthiban @ 2024-09-10 15:02 UTC (permalink / raw)
To: david
Cc: parthiban, dhowells, linux-integrity, keyrings,
linux-security-module, linux-kernel
Dear David,
The below commit when using the stack memory for encrypt/decryption, unable to
add the key using keyctl. IMO the encrypt/decrypt request which is submitted to
the dcp driver is asynchronous and seal/unseal is returned before the completion.
crypto_wait_req with aead_request_set_callback will help?
+ keyctl add trusted kmk 'new 32' @u
[ 18.345069] 8<--- cut here ---
[ 18.348199] Unable to handle kernel paging request at virtual address e00125a0 when read
[ 18.356321] [e00125a0] *pgd=00000000
[ 18.359948] Internal error: Oops: 5 [#1] SMP ARM
[ 18.364597] Modules linked in:
[ 18.367688] CPU: 0 UID: 0 PID: 35 Comm: mxs_dcp_chan/ae Not tainted 6.11.0-rc7-00017-gbc83b4d1f086 #20
[ 18.377035] Hardware name: Freescale i.MX6 Ultralite (Device Tree)
[ 18.383235] PC is at page_address+0x8/0xf4
[ 18.387393] LR is at dcp_chan_thread_aes+0x170/0x7e8
[ 18.392412] pc : [<c0281a94>] lr : [<c06d0dc0>] psr: 800e0013
[ 18.398703] sp : e0a85f00 ip : 00000100 fp : 00000000
[ 18.403951] r10: c241c640 r9 : e0845db0 r8 : c2715490
[ 18.409201] r7 : e0c2ddbc r6 : 00000000 r5 : c2715280 r4 : e00125a0
[ 18.415754] r3 : e0c2ddbc r2 : e00125a2 r1 : c27152ed r0 : e00125a0
[ 18.422308] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 18.429476] Control: 10c5387d Table: 8270406a DAC: 00000051
[ 18.435244] Register r0 information: non-paged memory
[ 18.440334] Register r1 information: slab kmalloc-192 start c2715240 pointer offset 173 size 192
[ 18.449206] Register r2 information: non-paged memory
[ 18.454286] Register r3 information: 2-page vmalloc region starting at 0xe0c2c000 allocated at kernel_
clone+0x9c/0x340
[ 18.465051] Register r4 information: non-paged memory
[ 18.470133] Register r5 information: slab kmalloc-192 start c2715240 pointer offset 64 size 192
[ 18.478906] Register r6 information: NULL pointer
[ 18.483641] Register r7 information: 2-page vmalloc region starting at 0xe0c2c000 allocated at kernel_
clone+0x9c/0x340
[ 18.494395] Register r8 information: slab kmalloc-192 start c2715480 pointer offset 16 size 192
[ 18.503166] Register r9 information: 2-page vmalloc region starting at 0xe0844000 allocated at kernel_
clone+0x9c/0x340
[ 18.513918] Register r10 information: slab kmalloc-512 start c241c600 pointer offset 64 size 512
[ 18.522780] Register r11 information: NULL pointer
[ 18.527603] Register r12 information: non-paged memory
[ 18.532773] Process mxs_dcp_chan/ae (pid: 35, stack limit = 0x4cccc1b5)
[ 18.539428] Stack: (0xe0a85f00 to 0xe0a86000)
[ 18.543825] 5f00: 00000000 c2715280 00000000 e0c2ddbc c2715490 c06d0dc0 00000000 00000000
[ 18.552041] 5f20: c22e3480 c0901a08 00000000 c2715490 c0b57a80 00000000 c22e3480 00000000
[ 18.560256] 5f40: c2344040 e0c2ddbc 00000002 124e0e4e c2345040 e0c2ddcc 00000001 c27152c0
[ 18.568469] 5f60: c241c640 c242c080 e0845db0 c2419e80 c22e3480 c06d0c50 00000000 c242c080
[ 18.576680] 5f80: e0845db0 00000000 00000000 c0141fe8 c2419e80 c0141f10 00000000 00000000
[ 18.584890] 5fa0: 00000000 00000000 00000000 c010016c 00000000 00000000 00000000 00000000
[ 18.593101] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 18.601309] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 18.609509] Call trace:
[ 18.609537] page_address from dcp_chan_thread_aes+0x170/0x7e8
[ 18.617988] dcp_chan_thread_aes from kthread+0xd8/0xf8
[ 18.623280] kthread from ret_from_fork+0x14/0x28
[ 18.628031] Exception stack(0xe0a85fb0 to 0xe0a85ff8)
[ 18.633115] 5fa0: 00000000 00000000 00000000 00000000
[ 18.641326] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 18.649532] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 18.656188] Code: e12fff1e e12fff1e e92d41f0 e1a04000 (e5903000)
[ 18.662306] ---[ end trace 0000000000000000 ]---
[ 18.666949] note: mxs_dcp_chan/ae[35] exited with irqs disabled
git show --stat 0e28bf61a5f9ab30be3f3b4eafb8d097e39446bb
commit 0e28bf61a5f9ab30be3f3b4eafb8d097e39446bb
Author: David Gstir <david@sigma-star.at>
Date: Wed Jul 17 13:28:45 2024 +0200
KEYS: trusted: dcp: fix leak of blob encryption key
Trusted keys unseal the key blob on load, but keep the sealed payload in
the blob field so that every subsequent read (export) will simply
convert this field to hex and send it to userspace.
With DCP-based trusted keys, we decrypt the blob encryption key (BEK)
in the Kernel due hardware limitations and then decrypt the blob payload.
BEK decryption is done in-place which means that the trusted key blob
field is modified and it consequently holds the BEK in plain text.
Every subsequent read of that key thus send the plain text BEK instead
of the encrypted BEK to userspace.
This issue only occurs when importing a trusted DCP-based key and
then exporting it again. This should rarely happen as the common use cases
are to either create a new trusted key and export it, or import a key
blob and then just use it without exporting it again.
Fix this by performing BEK decryption and encryption in a dedicated
buffer. Further always wipe the plain text BEK buffer to prevent leaking
the key via uninitialized memory.
Cc: stable@vger.kernel.org # v6.10+
Fixes: 2e8a0f40a39c ("KEYS: trusted: Introduce NXP DCP-backed trusted keys")
Signed-off-by: David Gstir <david@sigma-star.at>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
security/keys/trusted-keys/trusted_dcp.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
--
Thanks,
Parthiban N
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Trusted keys: DCP: Unable to handle paging request
2024-09-10 15:02 Trusted keys: DCP: Unable to handle paging request Parthiban
@ 2024-09-11 11:46 ` David Gstir
2024-10-29 11:34 ` [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation David Gstir
1 sibling, 0 replies; 5+ messages in thread
From: David Gstir @ 2024-09-11 11:46 UTC (permalink / raw)
To: Parthiban
Cc: David Howells, linux-integrity@vger.kernel.org,
keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Parthiban,
> On 10.09.2024, at 17:02, Parthiban <parthiban@linumiz.com> wrote:
>
> Dear David,
>
> The below commit when using the stack memory for encrypt/decryption, unable to
> add the key using keyctl. IMO the encrypt/decrypt request which is submitted to
> the dcp driver is asynchronous and seal/unseal is returned before the completion.
>
> crypto_wait_req with aead_request_set_callback will help?
thanks for reporting this! Yes, it looks like this could fix it.
I’m still a bit puzzled why I never ran into this during my tests,
but I’ll prepare a fix.
Thanks,
- david
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation
2024-09-10 15:02 Trusted keys: DCP: Unable to handle paging request Parthiban
2024-09-11 11:46 ` David Gstir
@ 2024-10-29 11:34 ` David Gstir
2024-10-29 22:37 ` Jarkko Sakkinen
2024-10-29 22:38 ` Jarkko Sakkinen
1 sibling, 2 replies; 5+ messages in thread
From: David Gstir @ 2024-10-29 11:34 UTC (permalink / raw)
To: parthiban, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn
Cc: sigma star Kernel Team, linux-integrity, keyrings,
linux-security-module, linux-kernel, David Gstir, stable
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);
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation
2024-10-29 11:34 ` [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation David Gstir
@ 2024-10-29 22:37 ` Jarkko Sakkinen
2024-10-29 22:38 ` Jarkko Sakkinen
1 sibling, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2024-10-29 22:37 UTC (permalink / raw)
To: David Gstir, parthiban, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn
Cc: sigma star Kernel Team, linux-integrity, keyrings,
linux-security-module, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation
2024-10-29 11:34 ` [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation David Gstir
2024-10-29 22:37 ` Jarkko Sakkinen
@ 2024-10-29 22:38 ` Jarkko Sakkinen
1 sibling, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2024-10-29 22:38 UTC (permalink / raw)
To: David Gstir, parthiban, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn
Cc: sigma star Kernel Team, linux-integrity, keyrings,
linux-security-module, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-29 22:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-10 15:02 Trusted keys: DCP: Unable to handle paging request Parthiban
2024-09-11 11:46 ` David Gstir
2024-10-29 11:34 ` [PATCH] KEYS: trusted: dcp: fix NULL dereference in AEAD crypto operation David Gstir
2024-10-29 22:37 ` Jarkko Sakkinen
2024-10-29 22:38 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).